[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Dominik Adamski via Phabricator via cfe-commits]

domada added inline comments.



Comment at: clang/test/OpenMP/irbuilder_simd.cpp:15
   P pp;
-#pragma omp simd
+#pragma omp simd simdlen(3)
   for (int i = 3; i < 32; i += 5) {

Could you add separate test case instead of modifying existing test case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129149

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

[PATCH] D129160: libclang.so: Make SONAME the same as LLVM version

[Fangrui Song via Phabricator via cfe-commits]

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

LGTM. I have seen multiple reports that distributors found this behavior change 
surprising.
I agree that having this more complex symbol versioning with libclang.so 
probably doesn't improve things a lot since a libclang user more or less will 
use some LLVM API and it doesn't have stability anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129160

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

[PATCH] D129174: [C++20][Modules] Invalidate internal-linkage functions in overload sets [P1815R2 part 1]

[Iain Sandoe via Phabricator via cfe-commits]

iains created this revision.
Herald added a project: All.
iains added reviewers: urnathan, ChuanqiXu.
iains added a subscriber: clang-modules.
iains published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is an implementation of the first part of P1815 
 (and includes the testcase for
[basic.def.odr]/ p10).

If a function is in a different modular TU, and has internal-linkage, we 
invalidate its
entry in an overload set.

In [basic.lookup.argdep] p5 ex 2, we now correctly reject the use of `R::g` in 
the attempted
instantiation of 'apply' in TU #3.

Note that clang (and GCC) also rejects `f(x)` in the same TU, where the example 
currently
states it should be OK.  A clarification question has been sent to core about 
this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129174

Files:
  clang/include/clang/Sema/Overload.h
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/basic/basic.link/p10-ex2.cpp
  clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp

Index: clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
===
--- /dev/null
+++ clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
@@ -0,0 +1,68 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 M.cpp -emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -std=c++20 N.cpp -emit-module-interface -o N.pcm \
+// RUN:   -fmodule-file=M.pcm
+// RUN: %clang_cc1 -std=c++20 Q.cpp -emit-module-interface -o Q.pcm
+// RUN: %clang_cc1 -std=c++20 Q-impl.cpp -fsyntax-only -fmodule-file=Q.pcm \
+// RUN:   -fmodule-file=N.pcm -verify
+
+//--- M.cpp
+export module M;
+namespace R {
+export struct X {};
+export void f(X);
+} // namespace R
+namespace S {
+export void f(R::X, R::X);
+}
+
+//--- N.cpp
+export module N;
+import M;
+export R::X make();
+namespace R {
+static int g(X);
+}
+export template 
+void apply(T t, U u) {
+  f(t, u);
+  g(t);
+}
+
+//--- Q.cpp
+export module Q;
+
+//--- Q-impl.cpp
+module Q;
+import N;
+
+namespace S {
+struct Z {
+  template  operator T();
+};
+} // namespace S
+void test() {
+  // OK, decltype(x) is R::X in module M
+  auto x = make();
+
+  // error: R and R::f are not visible here
+  R::f(x); // expected-error {{declaration of 'R' must be imported from module 'N' before it is required}}
+  // expected-n...@n.cpp:4 {{declaration here is not visible}}
+  // expected-error@-2 {{no type named 'f' in namespace 'R'}}
+
+  // Not OK, R::f from interface of M is reachable, but not visible to lookup.
+  f(x); // expected-error {{use of undeclared identifier 'f'}}
+
+  // error: S::f in module M not considered even though S is an associated
+  // namespace
+  f(x, S::Z()); // expected-error {{use of undeclared identifier 'f'}}
+
+  // error: S::f is visible in instantiation context, but  R::g has internal
+  // linkage and cannot be used outside N.cpp
+  apply(x, S::Z()); // expected-er...@n.cpp:10 {{no matching function for call to 'g'}}
+// expected-note@-1 {{in instantiation of function template specialization 'apply' requested here}}
+}
Index: clang/test/CXX/basic/basic.link/p10-ex2.cpp
===
--- /dev/null
+++ clang/test/CXX/basic/basic.link/p10-ex2.cpp
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 M.cpp -fsyntax-only -DTEST_INTERFACE -verify
+// RUN: %clang_cc1 -std=c++20 M.cpp -emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -std=c++20 useM.cpp -fsyntax-only -fmodule-file=M.pcm -verify
+
+//--- decls.h
+int f(); // #1, attached to the global module
+int g(); // #2, attached to the global module
+
+//--- M.cpp
+module;
+#include "decls.h"
+export module M;
+export using ::f; // OK, does not declare an entity, exports #1
+#if TEST_INTERFACE
+// error: matches #2, but attached to M
+int g(); // expected-error {{declaration of 'g' in module M follows declaration in the global module}}
+// expected-note@decls.h:2 {{previous declaration is here}}
+#endif
+export int h(); // #3
+export int k(); // #4
+
+//--- useM.cpp
+import M;
+// error: matches #3
+static int h(); // expected-error {{static declaration of 'h' follows non-static declaration}}
+// expected-n...@m.cpp:10 {{previous declaration is here}}
+
+// error: matches #4
+int k(); // expected-error {{declaration of 'k' in the global module follows declaration in module M}}
+// expected-n...@m.cpp:11 {{previous declaration is here}}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -6400,6 +6400,16 @@
 return;
   }
 
+  // Functions with internal linkage are only viable in the same module.
+  if (auto *MF = Function

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Shraiysh via Phabricator via cfe-commits]

shraiysh added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596
   // Check for unsupported clauses
-  if (!S.clauses().empty()) {
-// Currently no clause is supported
-return false;
+  for (OMPClause *C : S.clauses()) {
+// Currently only simdlen clause is supported

psoni2628 wrote:
> psoni2628 wrote:
> > arnamoy10 wrote:
> > > I am just wondering whether we should have a check to make sure that we 
> > > are processing the clauses of only `simd` directive here. Because the 
> > > function takes a general `OMPExecutableDirective` as argument 
> > That's a fair point. I guess `isSupportedByOpenMPIRBuilder` could be used 
> > for other directive types other than simd, even though it's not right now.
> Would it make more sense to only guard the checking of clauses with a check 
> for `OMPSimdDirective`, or the whole thing? I believe even the code below, 
> which checks for an ordered directive, is also specifically for `simd`?
> 
> 
> Example of guarding the whole thing:
> 
> ```
>   if(dyn_cast(S)) {
> // Check for unsupported clauses
> for (OMPClause *C : S.clauses()) {
>   // Currently only simdlen clause is supported
>   if (dyn_cast(C))
> continue;
>   else
> return false;
> }
> 
> // Check if we have a statement with the ordered directive.
> // Visit the statement hierarchy to find a compound statement
> // with a ordered directive in it.
> if (const auto *CanonLoop = dyn_cast(S.getRawStmt())) {
>   if (const Stmt *SyntacticalLoop = CanonLoop->getLoopStmt()) {
> for (const Stmt *SubStmt : SyntacticalLoop->children()) {
>   if (!SubStmt)
> continue;
>   if (const CompoundStmt *CS = dyn_cast(SubStmt)) {
> for (const Stmt *CSSubStmt : CS->children()) {
>   if (!CSSubStmt)
> continue;
>   if (isa(CSSubStmt)) {
> return false;
>   }
> }
>   }
> }
>   }
> }
>   }
> ```
Can we instead have separate `isSupportedByOpenMPIRBuilder` for every directive 
to avoid bloating the function with checks and if conditions?
```
static bool isSupportedByOpenMPIRBuilder(const OMPSimdDirective &S) {...}
void CodeGenFunction::EmitOMPSimdDirective(const OMPSimdDirective &S) {...}

static bool isSupportedByOpenMPIRBuilder(const OMPOrderedDirective &S) {...}
void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) 
{...}

static bool isSupportedByOpenMPIRBuilder(const OMPTaskDirective &S) {...}
void CodeGenFunction::EmitOMPOrderedDirective(const OMPTaskDirective &S) {...}
```



Comment at: clang/test/OpenMP/irbuilder_simd.cpp:72
+// CHECK-NEXT: ![[META9]]  = distinct !{![[META9]], ![[META10:[0-9]+]], 
![[META6]]}
+// CHECK-NEXT: ![[META10]]  = !{!"llvm.loop.parallel_accesses", ![[META8]]}

nit: maybe add newline



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:1773
   OMPBuilder.applySimd(DL, CLI);
+  OMPBuilder.applySimdlen(DL, CLI, ConstantInt::get(Type::getInt32Ty(Ctx), 3));
 

Can we please add this as a new test instead of modifying the existing one?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129149

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

[PATCH] D129160: libclang.so: Make SONAME the same as LLVM version

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added a comment.

Forgot to mention it before: would be good to note this in the release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129160

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

[PATCH] D129043: [RISCV][Clang] Teach RISCVEmitter to generate BitCast for pointer operands.

[Yeting Kuo via Phabricator via cfe-commits]

fakepaper56 added inline comments.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:111
+if (I.value()->isPointer()) {
+  assert(RVVI->getIntrinsicTypes().front() == -1 &&
+ "RVVI should be vector load intrinsic.");

khchen wrote:
> I feel this logic is not clear for reader, maybe you should add comment to 
> say why the return type -1 are load intrinsics?
You are right. Maybe I could rewrite the message of assertion to "RVVI should 
be vector load intrinsic, we don't support this feature for stores now."?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129043

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

[PATCH] D129138: [clang] [docs] Update the changes of C++20 Modules in clang15

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

Perhaps we could be a little more bold about the completeness of the 
implementation (at least, w.r.t the base paper `P1103`) - we pass the relevant 
test cases.

As for the follow-on papers, I think we have more that can be added notes below:
There are some test cases to be posted to phab for some of these (so maybe 
allow me a few more days to get the list fully correct).

(it will also depend on what we can land before 26th - however some of the 
stuff below is already approved, so it's a matter of finding some time to push 
the patches and watch the bots...)

@ChuanqiXu if you think that more is needed on any of these (other than `P1815` 
which is known partial), please let me know.

(Please also add any relevant phab reviews from your side)

-

P1779R3: ABI isolation for member functions

- Paper applied to WP.

Change [dcl.inline]/7 (as edited by P1815R1):

- This is being addressed by D128328 

(although we have somewhat of a moving target since some clarifications were 
requested from core).

Change [dcl.fct.def.default]/3:

- Already handled in the constexpr/consteval code, there is no actual change 
for modular cases.

Change [class.mfct]/1:
Change [class.friend]/7:

- D129045 , including tests.

--

P1979R0: Resolution to US086

- Paper applied to WP.
- Current implementation complies: test case to be posted 
CXX/module/module.import/p7.cpp

--

P1811R0 Relaxing redefinition restrictions for re-exportation robustness

- Paper applied to WP - note there are on-going discussions in core/ext about 
exports that might affect this.

Note that there are a lot of changes here, but that the draft that includes 
them is what we are working to, so it is expected that (mostly) we will need to 
identify tests and/or queries about how to verify.

Change in 6.2 [basic.def.odr] paragraph 1:
Change in 6.2 [basic.def.odr] paragraph 12:

- no action required, these changes restore pre-P1103 behaviour (and the 
paragraph 12 changes have been subsumed in following updates).

Change in 10.5 [module.context] paragraph 7:

  test: CXX/module/module.context/p7.cpp

Change in 10.6 [module.reach] paragraph 3:

- D126694  and D128328 


Change in 15.2 [cpp.include] paragraph 7:

- D128981  provides conditional include 
translation that follows the same scheme as GCC.

Feature test macro

- already implemented.

---

[Partial] P1815R2: Translation-unit-local entities

Change [basic.def.odr]/10:

- implementation complies, example provided.

Insert before [basic.def.odr]/11

- D128328  + ongoing core/ext discussions.

Change [basic.lookup.argdep]/5:
D129174  - overload changes.  example added 
 Question to core on TU#3  f(x).

---

P2115R0: US069: Merging of multiple definitions for unnamed unscoped 
enumerations

- merged to WP, but ongoing discussions in core/ext might cause re-work

testcase to be posted to phab.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129138

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

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a reviewer: nikic.
ChuanqiXu marked 2 inline comments as done.
ChuanqiXu added a comment.

In D125291#3629276 , @nikic wrote:

> FWIW the bitcode patch has landed, so implementing the variant using a token 
> type should be possible now. I'm not sure whether it's better to start with 
> the current patch where the intrinsic is optional, or go directly to the one 
> where it is required.

I preferred to start with the current patch. Since my original idea is to fix 
the thread problems in coroutines and @jyknight said we could fix the two 
problems in one approach. But I still want to fix the thread problems in 
coroutines first.

---

A big problem I meet now is about the constant expression would merge into the 
argument automatically so the verifier would fail. Here is the example:

  C++
  union U {
int a;
float f;
constexpr U() : f(0.0) {}
  };
  static thread_local U u;
  void *p = &u;

Then it would generate following code for `u`:

  define internal void @__cxx_global_var_init() #0 section ".text.startup" {
  entry:
%0 = call %union.U* @llvm.threadlocal.address.p0s_union.Us(%union.U* 
bitcast ({ float }* @_ZL1u to %union.U*))
%1 = bitcast %union.U* %0 to i8*
store i8* %1, i8** @p, align 8
ret void
  }

Then the verifier would complain since the argument of 
`@llvm.threadlocal.address` is not theadlocal global value. And I feel this 
might not be a simple problem to fix. I feel like we could only fix it after we 
made https://discourse.llvm.org/t/rfc-remove-most-constant-expressions/63179. 
But it should be a long term goal to me.

---

So now it looks like there are two options:

1. Remove the verifier part.
2. Wait until we removed most constant expressions. In this case, I could only 
to pick up original methods to solve the thread problems in coroutines.

@jyknight @nikic  @nhaehnle @efriedma @rjmccall How do you think about this?




Comment at: llvm/docs/LangRef.rst:24415
+
+  declare ptr @llvm.threadlocal.address(ptr) nounwind readnone willreturn
+

nikic wrote:
> Don't we need to overload this intrinsic by return type, so it works with 
> different address spaces?
Done. So we could remove the limit for opaque pointers too. So now it covers 
more tests.



Comment at: llvm/docs/LangRef.rst:24420
+
+The first argument is pointer, which refers to a thread local variable.
+

nikic wrote:
> Should we enforce here (with a verifier check) that the argument is a 
> thread-local global variable? I assume we //don't// want to allow weird 
> things like `@llvm.threadlocal.address(c ? @g1 : @g2)` right? (Though I 
> guess, without thread-local globals using a token type, nothing would prevent 
> optimizations from forming this.)
I originally added assertions when creating the intrinsics. But I add a check 
in the verifier in this revision.


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

https://reviews.llvm.org/D125291

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

[PATCH] D125693: [DebugInfo] Support types, imports and static locals declared in a lexical block (3/5)

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added inline comments.



Comment at: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll:23
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 
x i8]* @__profn_foo, i32 0, i32 0), i64 12345678, i32 2, i32 0)
-  ret void
+  ret void, !dbg !17
 }

ellis wrote:
> I asked the same question in D113741, but why is this test changed?
Normally, we emit function-local entities iff a parent function has location 
information. This is done the same way for local variables, labels, parameters, 
imported entities, and, //now,// for static locals as well.
Before this change static locals behaved more like global variables and get 
emitted doesn't matter its parent function. This patch makes them handled more 
like local variables.

I believe either the call or the 'ret' (or, likely, both) had had DILocation 
attached originally, but it has been removed to simplify the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125693

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

[PATCH] D129174: [C++20][Modules] Invalidate internal-linkage functions in overload sets [P1815R2 part 1]

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/include/clang/Sema/Overload.h:800
+/// This candidate was not viable because it has internal linkage and is
+/// from a different module than the use.
+ovl_fail_module_mismatched,





Comment at: clang/lib/Sema/SemaOverload.cpp:6403
 
+  // Functions with internal linkage are only viable in the same module.
+  if (auto *MF = Function->getOwningModule()) {





Comment at: clang/lib/Sema/SemaOverload.cpp:6406
+if (Function->getFormalLinkage() <= Linkage::InternalLinkage &&
+getLangOpts().CPlusPlus20 && MF != getCurrentModule()) {
+  Candidate.Viable = false;

The current implementation may reject following two cases:
```
module;
static void foo(int); // Ignore header
...
export module A;
void bar() { foo(5); } // Should it be invalid?
```
and
```
export module A;
static void foo(int);
...
module :private;
void bar() { foo(5); } // Should it be invalid?
```

I mean in the above examples, the function of `foo(int)` is defined in the same 
TU but the call to it might be rejected.



Comment at: clang/test/CXX/basic/basic.link/p10-ex2.cpp:10-35
+//--- decls.h
+int f(); // #1, attached to the global module
+int g(); // #2, attached to the global module
+
+//--- M.cpp
+module;
+#include "decls.h"

I feel like the test is irrelevant with the revision, isn't it? If yes, I think 
we could land this as a separate NFC patch.



Comment at: 
clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp:36-41
+//--- Q.cpp
+export module Q;
+
+//--- Q-impl.cpp
+module Q;
+import N;

This looks more consistent with the example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129174

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

[PATCH] D129064: [clang-format] Avoid crash in LevelIndentTracker.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius updated this revision to Diff 442482.
curdeius marked an inline comment as done.
curdeius added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129064

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


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -609,6 +609,14 @@
  "  return a == 8 ? 32 : 16;\n"
  "}\n";
   EXPECT_EQ(Code, format(Code, 40, 0));
+
+  // https://llvm.org/PR56352
+  Style.CompactNamespaces = true;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  Code = "\n"
+ "namespace ns1 { namespace ns2 {\n"
+ "}}";
+  EXPECT_EQ(Code, format(Code, 0, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -66,7 +66,6 @@
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
-  IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -91,6 +90,7 @@
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
+assert(Line.Level < IndentForLevel.size());
 if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
 !Line.InPPDirective) {
   IndentForLevel[Line.Level] = LevelIndent;


Index: clang/unittests/Format/FormatTestSelective.cpp
===
--- clang/unittests/Format/FormatTestSelective.cpp
+++ clang/unittests/Format/FormatTestSelective.cpp
@@ -609,6 +609,14 @@
  "  return a == 8 ? 32 : 16;\n"
  "}\n";
   EXPECT_EQ(Code, format(Code, 40, 0));
+
+  // https://llvm.org/PR56352
+  Style.CompactNamespaces = true;
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  Code = "\n"
+ "namespace ns1 { namespace ns2 {\n"
+ "}}";
+  EXPECT_EQ(Code, format(Code, 0, 0));
 }
 
 } // end namespace
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -66,7 +66,6 @@
   (Style.PPIndentWidth >= 0) ? Style.PPIndentWidth : Style.IndentWidth;
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
-  IndentForLevel.resize(Line.Level + 1);
   Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
@@ -91,6 +90,7 @@
 unsigned LevelIndent = Line.First->OriginalColumn;
 if (static_cast(LevelIndent) - Offset >= 0)
   LevelIndent -= Offset;
+assert(Line.Level < IndentForLevel.size());
 if ((!Line.First->is(tok::comment) || IndentForLevel[Line.Level] == -1) &&
 !Line.InPPDirective) {
   IndentForLevel[Line.Level] = LevelIndent;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D110114: [OMPIRBuilder] Generate aggregate argument for parallel region outlined functions

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added subscribers: Meinersbur, kiranchandramohan.
kiranchandramohan added a comment.
Herald added a project: All.

If I understand this correctly, filling the aggregate struct is only happening 
in the parallel case but not for the serialized parallel version. See example 
below, the call to `@sb_..omp_par` from kmpc_fork_call in block `omp_parallel` 
has the aggregate filled in while the call to `@sb_..omp_par` in the serialized 
parallel region in block 6 does not have this. I assume that the extractor 
creates these and it will only do it at the place that it is called. Would 
copying over the instructions that fill the aggregate to the serialized 
parallel region be a reasonable solution?

CC: @Meinersbur

  define void @sb_(ptr %0, ptr %1) !dbg !3 {
%structArg = alloca { ptr }, align 8
%tid.addr = alloca i32, align 4, !dbg !7
%zero.addr = alloca i32, align 4, !dbg !7
store i32 0, ptr %tid.addr, align 4, !dbg !7
store i32 0, ptr %zero.addr, align 4, !dbg !7
%3 = load i32, ptr %0, align 4, !dbg !7
%4 = icmp ne i32 %3, 0, !dbg !7 
br label %entry, !dbg !7
  
  entry:; preds = %2
%omp_global_thread_num = call i32 @__kmpc_global_thread_num(ptr @1), !dbg !7
br i1 %4, label %5, label %6
  
  5:; preds = %entry
br label %omp_parallel
  
  omp_parallel: ; preds = %5
%gep_ = getelementptr { ptr }, ptr %structArg, i32 0, i32 0
store ptr %1, ptr %gep_, align 8
call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @1, i32 1, ptr 
@sb_..omp_par, ptr %structArg), !dbg !9
br label %omp.par.outlined.exit
  
  omp.par.outlined.exit:; preds = %omp_parallel
br label %omp.par.exit.split
  
  omp.par.exit.split:   ; preds = 
%omp.par.outlined.exit
br label %7
  
  6:; preds = %entry
call void @__kmpc_serialized_parallel(ptr @1, i32 %omp_global_thread_num)
call void @sb_..omp_par(ptr %tid.addr, ptr %zero.addr, ptr %structArg), 
!dbg !9
call void @__kmpc_end_serialized_parallel(ptr @1, i32 
%omp_global_thread_num)
br label %7
  
  7:; preds = %6, 
%omp.par.exit.split
ret void, !dbg !10
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110114

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

[PATCH] D129100: [clangd] Support external throttler for preamble builds

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!




Comment at: clang-tools-extra/clangd/ClangdServer.h:108
+/// This throttler controls which preambles may be built at a given time.
+clangd::PreambleThrottler *PreambleThrottler = nullptr;
+

why the qualification ?



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:414
+private:
+  unsigned ID;
+  PreambleThrottler *Throttler;

use PreambleThrottler::RequestID instead of unsigned?



Comment at: clang-tools-extra/clangd/TUScheduler.h:90
 
+// PreambleThrottler controls which preambles can build at any given time.
+// This can be used to limit overall concurrency, and to prioritize some

triple slashes here and inside the class



Comment at: clang-tools-extra/clangd/TUScheduler.h:106
+  // Does not block, may eventually invoke the callback to satisfy the request.
+  // If cancel() is called, the callback will not be invoked afterwards.
+  // If the callback is invoked, release() must be called afterwards.

there's no cancel anymore.



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1499
+
+S.remove(Filenames.back());
+// Now shut down the TU Scheduler.

sequencing is hard here but it'd be nice to ensure release is actually seen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129100

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

[PATCH] D129180: [clang][dataflow] Return a solution from the solver when `Constraints` are `Satisfiable`.

[weiyi via Phabricator via cfe-commits]

wyt created this revision.
Herald added subscribers: martong, tschuett, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
wyt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129180

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

Index: clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
@@ -20,6 +20,12 @@
 using namespace clang;
 using namespace dataflow;
 
+using testing::_;
+using testing::AnyOf;
+using testing::Optional;
+using testing::Pair;
+using testing::UnorderedElementsAre;
+
 class SolverTest : public ::testing::Test {
 protected:
   // Checks if the conjunction of `Vals` is satisfiable and returns the
@@ -64,6 +70,17 @@
 return conj(impl(LeftSubVal, RightSubVal), impl(RightSubVal, LeftSubVal));
   }
 
+  void expectUnsatisfiable(Solver::Result Result) {
+EXPECT_EQ(Result.getStatus(), Solver::Result::Status::Unsatisfiable);
+EXPECT_FALSE(Result.getSolution().has_value());
+  }
+
+  template 
+  void expectSatisfiable(Solver::Result Result, Matcher Solution) {
+EXPECT_EQ(Result.getStatus(), Solver::Result::Status::Satisfiable);
+EXPECT_THAT(Result.getSolution(), Optional(Solution));
+  }
+
 private:
   std::vector> Vals;
 };
@@ -72,7 +89,9 @@
   auto X = atom();
 
   // X
-  EXPECT_EQ(solve({X}), Solver::Result::Satisfiable);
+  expectSatisfiable(
+  solve({X}),
+  UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedTrue)));
 }
 
 TEST_F(SolverTest, NegatedVar) {
@@ -80,7 +99,9 @@
   auto NotX = neg(X);
 
   // !X
-  EXPECT_EQ(solve({NotX}), Solver::Result::Satisfiable);
+  expectSatisfiable(
+  solve({NotX}),
+  UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedFalse)));
 }
 
 TEST_F(SolverTest, UnitConflict) {
@@ -88,7 +109,7 @@
   auto NotX = neg(X);
 
   // X ^ !X
-  EXPECT_EQ(solve({X, NotX}), Solver::Result::Unsatisfiable);
+  expectUnsatisfiable(solve({X, NotX}));
 }
 
 TEST_F(SolverTest, DistinctVars) {
@@ -97,7 +118,10 @@
   auto NotY = neg(Y);
 
   // X ^ !Y
-  EXPECT_EQ(solve({X, NotY}), Solver::Result::Satisfiable);
+  expectSatisfiable(
+  solve({X, NotY}),
+  UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedTrue),
+   Pair(Y, Solver::Result::Assignment::AssignedFalse)));
 }
 
 TEST_F(SolverTest, DoubleNegation) {
@@ -106,7 +130,7 @@
   auto NotNotX = neg(NotX);
 
   // !!X ^ !X
-  EXPECT_EQ(solve({NotNotX, NotX}), Solver::Result::Unsatisfiable);
+  expectUnsatisfiable(solve({NotNotX, NotX}));
 }
 
 TEST_F(SolverTest, NegatedDisjunction) {
@@ -116,7 +140,7 @@
   auto NotXOrY = neg(XOrY);
 
   // !(X v Y) ^ (X v Y)
-  EXPECT_EQ(solve({NotXOrY, XOrY}), Solver::Result::Unsatisfiable);
+  expectUnsatisfiable(solve({NotXOrY, XOrY}));
 }
 
 TEST_F(SolverTest, NegatedConjunction) {
@@ -126,7 +150,7 @@
   auto NotXAndY = neg(XAndY);
 
   // !(X ^ Y) ^ (X ^ Y)
-  EXPECT_EQ(solve({NotXAndY, XAndY}), Solver::Result::Unsatisfiable);
+  expectUnsatisfiable(solve({NotXAndY, XAndY}));
 }
 
 TEST_F(SolverTest, DisjunctionSameVars) {
@@ -135,7 +159,7 @@
   auto XOrNotX = disj(X, NotX);
 
   // X v !X
-  EXPECT_EQ(solve({XOrNotX}), Solver::Result::Satisfiable);
+  expectSatisfiable(solve({XOrNotX}), _);
 }
 
 TEST_F(SolverTest, ConjunctionSameVarsConflict) {
@@ -144,7 +168,7 @@
   auto XAndNotX = conj(X, NotX);
 
   // X ^ !X
-  EXPECT_EQ(solve({XAndNotX}), Solver::Result::Unsatisfiable);
+  expectUnsatisfiable(solve({XAndNotX}));
 }
 
 TEST_F(SolverTest, PureVar) {
@@ -156,7 +180,10 @@
   auto NotXOrNotY = disj(NotX, NotY);
 
   // (!X v Y) ^ (!X v !Y)
-  EXPECT_EQ(solve({NotXOrY, NotXOrNotY}), Solver::Result::Satisfiable);
+  expectSatisfiable(
+  solve({NotXOrY, NotXOrNotY}),
+  UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedFalse),
+   Pair(Y, _)));
 }
 
 TEST_F(SolverTest, MustAssumeVarIsFalse) {
@@ -169,7 +196,10 @@
   auto NotXOrNotY = disj(NotX, NotY);
 
   // (X v Y) ^ (!X v Y) ^ (!X v !Y)
-  EXPECT_EQ(solve({XOrY, NotXOrY, NotXOrNotY}), Solver::Result::Satisfiable);
+  expectSatisfiable(
+  solve({XOrY, NotXOrY, NotXOrNotY}),
+  UnorderedElementsAre(Pair(X, Solver::Result::Assignment::AssignedFalse),
+   Pair(Y, Solver::Result::Assignment::AssignedTrue)));
 }
 
 TEST_F(SolverTest, DeepConflict) {
@@ -183,8 +213,7 @@
   auto XOrNotY = disj(X, NotY);
 
   // (X v Y) ^ (!X v Y) ^ (!X v !Y) ^ (X v !Y)
-  EXPECT_EQ(solve({XOrY, NotXOrY, NotXOrNotY, XOrNotY}),
-Solver::Resu

[PATCH] D129138: [clang] [docs] Update the changes of C++20 Modules in clang15

[Chuanqi Xu via Phabricator via cfe-commits]

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

Address comments.


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

https://reviews.llvm.org/D129138

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -466,6 +466,14 @@
   that can be used for such compatibility. The demangler now demangles
   symbols with named module attachment.
 
+- Enhanced the support for C++20 Modules, including: Partitions,
+  Reachability, Header Unit and ``extern "C++"`` semantics.
+
+- Implemented `P1103R3: Merging Modules `_.
+- Implemented `P1779R3: ABI isolation for member functions 
`_.
+- Implemented `P1874R1: Dynamic Initialization Order of Non-Local Variables in 
Modules `_.
+- Partially implemented `P1815R2: Translation-unit-local entities 
`_.
+
 - As per "Conditionally Trivial Special Member Functions" (P0848), it is
   now possible to overload destructors using concepts. Note that the rest
   of the paper about other special member functions is not yet implemented.


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -466,6 +466,14 @@
   that can be used for such compatibility. The demangler now demangles
   symbols with named module attachment.
 
+- Enhanced the support for C++20 Modules, including: Partitions,
+  Reachability, Header Unit and ``extern "C++"`` semantics.
+
+- Implemented `P1103R3: Merging Modules `_.
+- Implemented `P1779R3: ABI isolation for member functions `_.
+- Implemented `P1874R1: Dynamic Initialization Order of Non-Local Variables in Modules `_.
+- Partially implemented `P1815R2: Translation-unit-local entities `_.
+
 - As per "Conditionally Trivial Special Member Functions" (P0848), it is
   now possible to overload destructors using concepts. Note that the rest
   of the paper about other special member functions is not yet implemented.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129138: [clang] [docs] Update the changes of C++20 Modules in clang15

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D129138#3631912 , @iains wrote:

> Perhaps we could be a little more bold about the completeness of the 
> implementation (at least, w.r.t the base paper `P1103`) - we pass the 
> relevant test cases.

I added the wording like `Implemented `P1103R3`. I am not sure if it is good to 
say this. We made some progress indeed. But both of us know there are some 
FIXME remains.

> As for the follow-on papers, I think we have more that can be added notes 
> below:
> There are some test cases to be posted to phab for some of these (so maybe 
> allow me a few more days to get the list fully correct).
>
> (it will also depend on what we can land before 26th - however some of the 
> stuff below is already approved, so it's a matter of finding some time to 
> push the patches and watch the bots...)
>
> @ChuanqiXu if you think that more is needed on any of these (other than 
> `P1815` which is known partial), please let me know.
>
> (Please also add any relevant phab reviews from your side)
>
> -
>
> P1779R3: ABI isolation for member functions
>
> - Paper applied to WP.
>
> Change [dcl.inline]/7 (as edited by P1815R1):
>
> - This is being addressed by D128328 
>
> (although we have somewhat of a moving target since some clarifications were 
> requested from core).
>
> Change [dcl.fct.def.default]/3:
>
> - Already handled in the constexpr/consteval code, there is no actual change 
> for modular cases.
>
> Change [class.mfct]/1:
> Change [class.friend]/7:
>
> - D129045 , including tests.
>
> --
>
> P1979R0: Resolution to US086
>
> - Paper applied to WP.
> - Current implementation complies: test case to be posted 
> CXX/module/module.import/p7.cpp
>
> --
>
> P1811R0 Relaxing redefinition restrictions for re-exportation robustness
>
> - Paper applied to WP - note there are on-going discussions in core/ext about 
> exports that might affect this.
>
> Note that there are a lot of changes here, but that the draft that includes 
> them is what we are working to, so it is expected that (mostly) we will need 
> to identify tests and/or queries about how to verify.
>
> Change in 6.2 [basic.def.odr] paragraph 1:
> Change in 6.2 [basic.def.odr] paragraph 12:
>
> - no action required, these changes restore pre-P1103 behaviour (and the 
> paragraph 12 changes have been subsumed in following updates).
>
> Change in 10.5 [module.context] paragraph 7:
>
>   test: CXX/module/module.context/p7.cpp
>
> Change in 10.6 [module.reach] paragraph 3:
>
> - D126694  and D128328 
> 
>
> Change in 15.2 [cpp.include] paragraph 7:
>
> - D128981  provides conditional include 
> translation that follows the same scheme as GCC.
>
> Feature test macro
>
> - already implemented.
>
> ---
>
> [Partial] P1815R2: Translation-unit-local entities
>
> Change [basic.def.odr]/10:
>
> - implementation complies, example provided.
>
> Insert before [basic.def.odr]/11
>
> - D128328  + ongoing core/ext discussions.
>
> Change [basic.lookup.argdep]/5:
> D129174  - overload changes.  example added 
>  Question to core on TU#3  f(x).
>
> ---
>
> P2115R0: US069: Merging of multiple definitions for unnamed unscoped 
> enumerations
>
> - merged to WP, but ongoing discussions in core/ext might cause re-work
>
> testcase to be posted to phab.

Thanks for the summary! But I am afraid it is too fine-grained to users. It 
looks like the current ReleaseNotes don't contain phab links nor new added test 
cases. @erichkeane @aaron.ballman may you give some advice?


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

https://reviews.llvm.org/D129138

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

[PATCH] D129138: [clang] [docs] Update the changes of C++20 Modules in clang15

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

I would not expect to add all this information to the release notes, or any of 
the phab links - just single lines to say that paper numbers are implemented

- the details are just to help us track the situation up to 26th July.


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

https://reviews.llvm.org/D129138

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

[PATCH] D126266: Mark the file entry invalid, until reread. Invalidate SLocEntry cache, readd it on reread. Do not use translateFile, because it pulls in parts of the pch.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D126266#3629145 , @vsapsai wrote:

> Hmm, I'm concerned with the pieces added purely for testing. Specifically, 
> `FileEntriesToReread` and TestHelper friend functions. Need to think how else 
> we can test it.

I am not thrilled about that either. We have discussed this approach here 
https://reviews.llvm.org/D126183 and we have past experience going that route. 
I would be happy to drop this test in favor of the mentioned clang-repl test. 
Should be enough to cover the usecase.

> Do you intend to support only the error cases like
>
>   clang-repl> #include "file_with_error.h"
>   // error is printed, we edit the file and include it again:
>   clang-repl> #include "file_with_error.h"
>
> or do you want to handle re-including files? Something like
>
>   clang-repl> #include "experiments.h"
>   // edit the file and include it again:
>   clang-repl> #include "experiments.h"
>
> Asking because maybe in the error case we commit to some state too eagerly 
> and fixing that sticky eagerness is another option (just guessing, have no 
> idea if it is an actual option and if it is "better").

We want both. The error case is somewhat easier to deal with. However, in case 
we have error in the logic in `#include "experiments.h"` we want to execute and 
then re-execute the new code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126266

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

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[Jean Perier via Phabricator via cfe-commits]

jeanPerier added a comment.

Hi @pengfei, I am working on flang, and after this patch, we started to see 
some bugs in Fortran programs using REAL(2) (which is fp16 in flang). I am not 
an expert in LLVM codegen and the builtins, but I am wondering if there is not 
issue with how llvm codegen thinks `__truncsfhf2` returns its value and how the 
runtime actually does return it.

Here is an llvm IR reproducer for a bug we saw:

  define void @bug(ptr %addr, i32 %i) {
%1 = sitofp i32 %i to half
store half %1, ptr %addr, align 2
ret void
  }

After this patch the generated assembly on X86 is:

  bug:# @bug
  pushrbx
  mov rbx, rdi
  cvtsi2ssxmm0, esi
  call__truncsfhf2@PLT
  pextrw  eax, xmm0, 0
  mov word ptr [rbx], ax
  pop rbx
  ret

When running this from a C program to test integers are casted to floats, I am 
only seeing the bytes of the passed address being set to zero (regardless of 
the input). It seems to me that there is an issue around the `__truncsfhf2` 
interface. The `pextrw  eax, xmm0, 0` after the call seems to suggest LLVM 
codegen is looking for the result in xmm0 register, but it seems that 
`__truncsfhf2` is only returning it in eax.

Do you have any idea what could be the issue ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107082

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

[PATCH] D129180: [clang][dataflow] Return a solution from the solver when `Constraints` are `Satisfiable`.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:263-267
+   Possible return values are:
+  /// - `Satisfiable`  : There exists a satisfying assignment for 
`Constraints`,
+  ///the solution found by the solver will be returned.
   /// - `Unsatisfiable`: There is no satisfying assignment for `Constraints`.
+  /// - `TimedOut` : The solver gives up on finding a satisfying 
assignment.

I'd suggest to revert these edits. Vertical alignment that is not supported by 
an autoformatter isn't sustainable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129180

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

[PATCH] D129174: [C++20][Modules] Invalidate internal-linkage functions in overload sets [P1815R2 part 1]

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 2 inline comments as done.
iains added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:6406
+if (Function->getFormalLinkage() <= Linkage::InternalLinkage &&
+getLangOpts().CPlusPlus20 && MF != getCurrentModule()) {
+  Candidate.Viable = false;

ChuanqiXu wrote:
> The current implementation may reject following two cases:
> ```
> module;
> static void foo(int); // Ignore header
> ...
> export module A;
> void bar() { foo(5); } // Should it be invalid?
> ```
> and
> ```
> export module A;
> static void foo(int);
> ...
> module :private;
> void bar() { foo(5); } // Should it be invalid?
> ```
> 
> I mean in the above examples, the function of `foo(int)` is defined in the 
> same TU but the call to it might be rejected.
> The current implementation may reject following two cases:
> ```
> module;
> static void foo(int); // Ignore header
> ...
> export module A;
> void bar() { foo(5); } // Should it be invalid?
> ```
> and
> ```
> export module A;
> static void foo(int);
> ...
> module :private;
> void bar() { foo(5); } // Should it be invalid?
> ```
> 
> I mean in the above examples, the function of `foo(int)` is defined in the 
> same TU but the call to it might be rejected.





Comment at: clang/lib/Sema/SemaOverload.cpp:6406
+if (Function->getFormalLinkage() <= Linkage::InternalLinkage &&
+getLangOpts().CPlusPlus20 && MF != getCurrentModule()) {
+  Candidate.Viable = false;

iains wrote:
> ChuanqiXu wrote:
> > The current implementation may reject following two cases:
> > ```
> > module;
> > static void foo(int); // Ignore header
> > ...
> > export module A;
> > void bar() { foo(5); } // Should it be invalid?
> > ```
> > and
> > ```
> > export module A;
> > static void foo(int);
> > ...
> > module :private;
> > void bar() { foo(5); } // Should it be invalid?
> > ```
> > 
> > I mean in the above examples, the function of `foo(int)` is defined in the 
> > same TU but the call to it might be rejected.
> > The current implementation may reject following two cases:
> > ```
> > module;
> > static void foo(int); // Ignore header
> > ...
> > export module A;
> > void bar() { foo(5); } // Should it be invalid?
> > ```
> > and
> > ```
> > export module A;
> > static void foo(int);
> > ...
> > module :private;
> > void bar() { foo(5); } // Should it be invalid?
> > ```
> > 
> > I mean in the above examples, the function of `foo(int)` is defined in the 
> > same TU but the call to it might be rejected.
> 
> 
> The current implementation may reject following two cases:
> ```
> module;
> static void foo(int); // Ignore header
> ...

Actually, I have a note to check on the global module case, since we have 
special rules that allow merging of items there,


> export module A;
> void bar() { foo(5); } // Should it be invalid?
> ```
> and
> ```
> export module A;
> static void foo(int);
> ...
> module :private;
> void bar() { foo(5); } // Should it be invalid?
> ```
> 
> I mean in the above examples, the function of `foo(int)` is defined in the 
> same TU but the call to it might be rejected.

otherwise, agreed, these cases should be ok -  I guess we need a second test 
case with lookups that should succeed.




Comment at: clang/test/CXX/basic/basic.link/p10-ex2.cpp:10-35
+//--- decls.h
+int f(); // #1, attached to the global module
+int g(); // #2, attached to the global module
+
+//--- M.cpp
+module;
+#include "decls.h"

ChuanqiXu wrote:
> I feel like the test is irrelevant with the revision, isn't it? If yes, I 
> think we could land this as a separate NFC patch.
well, it is related to the paper mentioned, but yes we can make this separate.




Comment at: 
clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp:36-41
+//--- Q.cpp
+export module Q;
+
+//--- Q-impl.cpp
+module Q;
+import N;

ChuanqiXu wrote:
> This looks more consistent with the example.
This changes the example so that M is directly included in the implementation 
rather than transitively (the example does specifically use a different module 
name for the implementation)

I am not sure what you mean by "more consistent"
 (the example will fail to reject some of the lookups with this change).





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129174

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

[PATCH] D129174: [C++20][Modules] Invalidate internal-linkage functions in overload sets [P1815R2 part 1]

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 442505.
iains marked 4 inline comments as done.
iains added a comment.

address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129174

Files:
  clang/include/clang/Sema/Overload.h
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/basic/basic.link/p10-ex2.cpp
  clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp

Index: clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
===
--- /dev/null
+++ clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
@@ -0,0 +1,68 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 M.cpp -emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -std=c++20 N.cpp -emit-module-interface -o N.pcm \
+// RUN:   -fmodule-file=M.pcm
+// RUN: %clang_cc1 -std=c++20 Q.cpp -emit-module-interface -o Q.pcm
+// RUN: %clang_cc1 -std=c++20 Q-impl.cpp -fsyntax-only -fmodule-file=Q.pcm \
+// RUN:   -fmodule-file=N.pcm -verify
+
+//--- M.cpp
+export module M;
+namespace R {
+export struct X {};
+export void f(X);
+} // namespace R
+namespace S {
+export void f(R::X, R::X);
+}
+
+//--- N.cpp
+export module N;
+import M;
+export R::X make();
+namespace R {
+static int g(X);
+}
+export template 
+void apply(T t, U u) {
+  f(t, u);
+  g(t);
+}
+
+//--- Q.cpp
+export module Q;
+
+//--- Q-impl.cpp
+module Q;
+import N;
+
+namespace S {
+struct Z {
+  template  operator T();
+};
+} // namespace S
+void test() {
+  // OK, decltype(x) is R::X in module M
+  auto x = make();
+
+  // error: R and R::f are not visible here
+  R::f(x); // expected-error {{declaration of 'R' must be imported from module 'N' before it is required}}
+  // expected-n...@n.cpp:4 {{declaration here is not visible}}
+  // expected-error@-2 {{no type named 'f' in namespace 'R'}}
+
+  // Not OK, R::f from interface of M is reachable, but not visible to lookup.
+  f(x); // expected-error {{use of undeclared identifier 'f'}}
+
+  // error: S::f in module M not considered even though S is an associated
+  // namespace
+  f(x, S::Z()); // expected-error {{use of undeclared identifier 'f'}}
+
+  // error: S::f is visible in instantiation context, but  R::g has internal
+  // linkage and cannot be used outside N.cpp
+  apply(x, S::Z()); // expected-er...@n.cpp:10 {{no matching function for call to 'g'}}
+// expected-note@-1 {{in instantiation of function template specialization 'apply' requested here}}
+}
Index: clang/test/CXX/basic/basic.link/p10-ex2.cpp
===
--- /dev/null
+++ clang/test/CXX/basic/basic.link/p10-ex2.cpp
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 M.cpp -fsyntax-only -DTEST_INTERFACE -verify
+// RUN: %clang_cc1 -std=c++20 M.cpp -emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -std=c++20 useM.cpp -fsyntax-only -fmodule-file=M.pcm -verify
+
+//--- decls.h
+int f(); // #1, attached to the global module
+int g(); // #2, attached to the global module
+
+//--- M.cpp
+module;
+#include "decls.h"
+export module M;
+export using ::f; // OK, does not declare an entity, exports #1
+#if TEST_INTERFACE
+// error: matches #2, but attached to M
+int g(); // expected-error {{declaration of 'g' in module M follows declaration in the global module}}
+// expected-note@decls.h:2 {{previous declaration is here}}
+#endif
+export int h(); // #3
+export int k(); // #4
+
+//--- useM.cpp
+import M;
+// error: matches #3
+static int h(); // expected-error {{static declaration of 'h' follows non-static declaration}}
+// expected-n...@m.cpp:10 {{previous declaration is here}}
+
+// error: matches #4
+int k(); // expected-error {{declaration of 'k' in the global module follows declaration in module M}}
+// expected-n...@m.cpp:11 {{previous declaration is here}}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -6400,6 +6400,17 @@
 return;
   }
 
+  // Functions with internal linkage are only viable in the same module unit.
+  if (auto *MF = Function->getOwningModule()) {
+if (Function->getFormalLinkage() <= Linkage::InternalLinkage &&
+getLangOpts().CPlusPlusModules &&
+MF->getTopLevelModule() != getCurrentModule()->getTopLevelModule()) {
+  Candidate.Viable = false;
+  Candidate.FailureKind = ovl_fail_module_mismatched;
+  return;
+}
+  }
+
   if (Function->isMultiVersion() && Function->hasAttr() &&
   !Function->getAttr()->isDefaultVersion()) {
 Candidate.Viable = false;
Index: clang/include/clang/Sema/Overload.h
===
--- clang/include/clang/Sema

[PATCH] D129174: [C++20][Modules] Invalidate internal-linkage functions in overload sets [P1815R2 part 1]

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

TODO: second test-case for cases that should succeed, split out basic-link / 
p10-ex2 test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129174

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

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

Hi @jeanPerier , yes, you are right. This patch changes the calling conversion 
of fp16 from GPRs to XMMs. So you need to update the runtime. If you are using 
compiler-rt, you could simply re-build it with trunk code, or at least after 
rGabeeae57 
. If you 
are using your own runtime, you can solve the problem through the way in 
https://github.com/llvm/llvm-project/issues/56156


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107082

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

[PATCH] D129138: [clang] [docs] Update the changes of C++20 Modules in clang15

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:472-476
+- Implemented `P1103R3: Merging Modules `_.
+- Implemented `P1779R3: ABI isolation for member functions 
`_.
+- Implemented `P1874R1: Dynamic Initialization Order of Non-Local Variables in 
Modules `_.
+- Partially implemented `P1815R2: Translation-unit-local entities 
`_.
+

This should probably also be reflected in 
https://clang.llvm.org/cxx_status.html / 
https://github.com/llvm/llvm-project/blob/main/clang/www/cxx_status.html ...?


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

https://reviews.llvm.org/D129138

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

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Prabhdeep Soni via Phabricator via cfe-commits]

psoni2628 added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596
   // Check for unsupported clauses
-  if (!S.clauses().empty()) {
-// Currently no clause is supported
-return false;
+  for (OMPClause *C : S.clauses()) {
+// Currently only simdlen clause is supported

shraiysh wrote:
> psoni2628 wrote:
> > psoni2628 wrote:
> > > arnamoy10 wrote:
> > > > I am just wondering whether we should have a check to make sure that we 
> > > > are processing the clauses of only `simd` directive here. Because the 
> > > > function takes a general `OMPExecutableDirective` as argument 
> > > That's a fair point. I guess `isSupportedByOpenMPIRBuilder` could be used 
> > > for other directive types other than simd, even though it's not right now.
> > Would it make more sense to only guard the checking of clauses with a check 
> > for `OMPSimdDirective`, or the whole thing? I believe even the code below, 
> > which checks for an ordered directive, is also specifically for `simd`?
> > 
> > 
> > Example of guarding the whole thing:
> > 
> > ```
> >   if(dyn_cast(S)) {
> > // Check for unsupported clauses
> > for (OMPClause *C : S.clauses()) {
> >   // Currently only simdlen clause is supported
> >   if (dyn_cast(C))
> > continue;
> >   else
> > return false;
> > }
> > 
> > // Check if we have a statement with the ordered directive.
> > // Visit the statement hierarchy to find a compound statement
> > // with a ordered directive in it.
> > if (const auto *CanonLoop = dyn_cast(S.getRawStmt())) 
> > {
> >   if (const Stmt *SyntacticalLoop = CanonLoop->getLoopStmt()) {
> > for (const Stmt *SubStmt : SyntacticalLoop->children()) {
> >   if (!SubStmt)
> > continue;
> >   if (const CompoundStmt *CS = dyn_cast(SubStmt)) {
> > for (const Stmt *CSSubStmt : CS->children()) {
> >   if (!CSSubStmt)
> > continue;
> >   if (isa(CSSubStmt)) {
> > return false;
> >   }
> > }
> >   }
> > }
> >   }
> > }
> >   }
> > ```
> Can we instead have separate `isSupportedByOpenMPIRBuilder` for every 
> directive to avoid bloating the function with checks and if conditions?
> ```
> static bool isSupportedByOpenMPIRBuilder(const OMPSimdDirective &S) {...}
> void CodeGenFunction::EmitOMPSimdDirective(const OMPSimdDirective &S) {...}
> 
> static bool isSupportedByOpenMPIRBuilder(const OMPOrderedDirective &S) {...}
> void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) 
> {...}
> 
> static bool isSupportedByOpenMPIRBuilder(const OMPTaskDirective &S) {...}
> void CodeGenFunction::EmitOMPOrderedDirective(const OMPTaskDirective &S) {...}
> ```
It was decided in D114379 to use `OMPExecutableDirective` in order to allow 
this function to be reused for constructs such as `for simd`. Do you wish to 
undo this now, and instead specialize the function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129149

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

[PATCH] D126956: [tbaa] Handle base classes in struct tbaa

[Jeroen Dobbelaere via Phabricator via cfe-commits]

jeroen.dobbelaere accepted this revision.
jeroen.dobbelaere 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/D126956/new/

https://reviews.llvm.org/D126956

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

[clang] 5b3247b - [tbaa] Handle base classes in struct tbaa

[Jeroen Dobbelaere via cfe-commits]

Author: Bruno De Fraine
Date: 2022-07-06T14:37:59+02:00
New Revision: 5b3247bf9f715cc1b399af1e17540b3a3ce9cdec

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

LOG: [tbaa] Handle base classes in struct tbaa

This is a fix for the miscompilation reported in 
https://github.com/llvm/llvm-project/issues/55384

Not adding a new test case since existing test cases already cover base classes 
(including new-struct-path tbaa).

Reviewed By: jeroen.dobbelaere

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

Added: 


Modified: 
clang/lib/CodeGen/CodeGenTBAA.cpp
clang/test/CodeGen/tbaa-class.cpp
clang/unittests/CodeGen/TBAAMetadataTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenTBAA.cpp 
b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 95763d8e18b70..0cb63fbbe9e5c 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -335,7 +335,42 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const 
Type *Ty) {
   if (auto *TTy = dyn_cast(Ty)) {
 const RecordDecl *RD = TTy->getDecl()->getDefinition();
 const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
-SmallVector Fields;
+using TBAAStructField = llvm::MDBuilder::TBAAStructField;
+SmallVector Fields;
+if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
+  // Handle C++ base classes. Non-virtual bases can treated a a kind of
+  // field. Virtual bases are more complex and omitted, but avoid an
+  // incomplete view for NewStructPathTBAA.
+  if (CodeGenOpts.NewStructPathTBAA && CXXRD->getNumVBases() != 0)
+return BaseTypeMetadataCache[Ty] = nullptr;
+  for (const CXXBaseSpecifier &B : CXXRD->bases()) {
+if (B.isVirtual())
+  continue;
+QualType BaseQTy = B.getType();
+const CXXRecordDecl *BaseRD = BaseQTy->getAsCXXRecordDecl();
+if (BaseRD->isEmpty())
+  continue;
+llvm::MDNode *TypeNode = isValidBaseType(BaseQTy)
+ ? getBaseTypeInfo(BaseQTy)
+ : getTypeInfo(BaseQTy);
+if (!TypeNode)
+  return BaseTypeMetadataCache[Ty] = nullptr;
+uint64_t Offset = Layout.getBaseClassOffset(BaseRD).getQuantity();
+uint64_t Size =
+Context.getASTRecordLayout(BaseRD).getDataSize().getQuantity();
+Fields.push_back(
+llvm::MDBuilder::TBAAStructField(Offset, Size, TypeNode));
+  }
+  // The order in which base class subobjects are allocated is unspecified,
+  // so may 
diff er from declaration order. In particular, Itanium ABI will
+  // allocate a primary base first.
+  // Since we exclude empty subobjects, the objects are not overlapping and
+  // their offsets are unique.
+  llvm::sort(Fields,
+ [](const TBAAStructField &A, const TBAAStructField &B) {
+   return A.Offset < B.Offset;
+ });
+}
 for (FieldDecl *Field : RD->fields()) {
   if (Field->isZeroSize(Context) || Field->isUnnamedBitfield())
 continue;

diff  --git a/clang/test/CodeGen/tbaa-class.cpp 
b/clang/test/CodeGen/tbaa-class.cpp
index 7f413a6f323c2..8e485f7c1cb8f 100644
--- a/clang/test/CodeGen/tbaa-class.cpp
+++ b/clang/test/CodeGen/tbaa-class.cpp
@@ -51,6 +51,25 @@ class StructS2 : public StructS
uint32_t f32_2;
 };
 
+class StructT {
+public:
+  uint32_t f32_2;
+  void foo();
+};
+class StructM1 : public StructS, public StructT {
+public:
+  uint16_t f16_2;
+};
+class StructDyn {
+public:
+  uint32_t f32_2;
+  virtual void foo();
+};
+class StructM2 : public StructS, public StructDyn {
+public:
+  uint16_t f16_2;
+};
+
 uint32_t g(uint32_t *s, StructA *A, uint64_t count) {
 // CHECK-LABEL: define{{.*}} i32 @_Z1g
 // CHECK: store i32 1, i32* %{{.*}}, align 4, !tbaa [[TAG_i32:!.*]]
@@ -199,6 +218,30 @@ uint32_t g12(StructC *C, StructD *D, uint64_t count) {
   return b1->a.f32;
 }
 
+uint32_t g13(StructM1 *M, StructS *S) {
+  // CHECK-LABEL: define{{.*}} i32 @_Z3g13
+  // CHECK: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+  // CHECK: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+  // PATH-LABEL: define{{.*}} i32 @_Z3g13
+  // PATH: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_S_f16]]
+  // PATH: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_M1_f16_2:!.*]]
+  S->f16 = 1;
+  M->f16_2 = 4;
+  return S->f16;
+}
+
+uint32_t g14(StructM2 *M, StructS *S) {
+  // CHECK-LABEL: define{{.*}} i32 @_Z3g14
+  // CHECK: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+  // CHECK: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+  // PATH-LABEL: define{{.*}} i32 @_Z3g14
+  // PATH: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_S_f16]]
+  // PATH: store i16 4, i16* %{{.*}},

[PATCH] D126956: [tbaa] Handle base classes in struct tbaa

[Jeroen Dobbelaere via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b3247bf9f71: [tbaa] Handle base classes in struct tbaa 
(authored by brunodf, committed by jeroen.dobbelaere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126956

Files:
  clang/lib/CodeGen/CodeGenTBAA.cpp
  clang/test/CodeGen/tbaa-class.cpp
  clang/unittests/CodeGen/TBAAMetadataTest.cpp

Index: clang/unittests/CodeGen/TBAAMetadataTest.cpp
===
--- clang/unittests/CodeGen/TBAAMetadataTest.cpp
+++ clang/unittests/CodeGen/TBAAMetadataTest.cpp
@@ -968,13 +968,10 @@
   MConstInt(0)),
 MConstInt(0));
 
-  auto ClassDerived = MMTuple(
-MMString("_ZTS7Derived"),
-MMTuple(
-  MMString("short"),
-  OmnipotentCharCXX,
-  MConstInt(0)),
-MConstInt(4));
+  auto ClassDerived =
+  MMTuple(MMString("_ZTS7Derived"), ClassBase, MConstInt(0),
+  MMTuple(MMString("short"), OmnipotentCharCXX, MConstInt(0)),
+  MConstInt(4));
 
   const Instruction *I = match(BB,
   MInstruction(Instruction::Store,
@@ -1047,13 +1044,10 @@
   MConstInt(0)),
 MConstInt(Compiler.PtrSize));
 
-  auto ClassDerived = MMTuple(
-MMString("_ZTS7Derived"),
-MMTuple(
-  MMString("short"),
-  OmnipotentCharCXX,
-  MConstInt(0)),
-MConstInt(Compiler.PtrSize + 4));
+  auto ClassDerived =
+  MMTuple(MMString("_ZTS7Derived"), ClassBase, MConstInt(0),
+  MMTuple(MMString("short"), OmnipotentCharCXX, MConstInt(0)),
+  MConstInt(Compiler.PtrSize + 4));
 
   const Instruction *I = match(BB,
   MInstruction(Instruction::Store,
Index: clang/test/CodeGen/tbaa-class.cpp
===
--- clang/test/CodeGen/tbaa-class.cpp
+++ clang/test/CodeGen/tbaa-class.cpp
@@ -51,6 +51,25 @@
uint32_t f32_2;
 };
 
+class StructT {
+public:
+  uint32_t f32_2;
+  void foo();
+};
+class StructM1 : public StructS, public StructT {
+public:
+  uint16_t f16_2;
+};
+class StructDyn {
+public:
+  uint32_t f32_2;
+  virtual void foo();
+};
+class StructM2 : public StructS, public StructDyn {
+public:
+  uint16_t f16_2;
+};
+
 uint32_t g(uint32_t *s, StructA *A, uint64_t count) {
 // CHECK-LABEL: define{{.*}} i32 @_Z1g
 // CHECK: store i32 1, i32* %{{.*}}, align 4, !tbaa [[TAG_i32:!.*]]
@@ -199,6 +218,30 @@
   return b1->a.f32;
 }
 
+uint32_t g13(StructM1 *M, StructS *S) {
+  // CHECK-LABEL: define{{.*}} i32 @_Z3g13
+  // CHECK: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+  // CHECK: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+  // PATH-LABEL: define{{.*}} i32 @_Z3g13
+  // PATH: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_S_f16]]
+  // PATH: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_M1_f16_2:!.*]]
+  S->f16 = 1;
+  M->f16_2 = 4;
+  return S->f16;
+}
+
+uint32_t g14(StructM2 *M, StructS *S) {
+  // CHECK-LABEL: define{{.*}} i32 @_Z3g14
+  // CHECK: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+  // CHECK: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_i16]]
+  // PATH-LABEL: define{{.*}} i32 @_Z3g14
+  // PATH: store i16 1, i16* %{{.*}}, align 4, !tbaa [[TAG_S_f16]]
+  // PATH: store i16 4, i16* %{{.*}}, align 4, !tbaa [[TAG_M2_f16_2:!.*]]
+  S->f16 = 1;
+  M->f16_2 = 4;
+  return S->f16;
+}
+
 // CHECK: [[TYPE_char:!.*]] = !{!"omnipotent char", [[TAG_cxx_tbaa:!.*]],
 // CHECK: [[TAG_cxx_tbaa]] = !{!"Simple C++ TBAA"}
 // CHECK: [[TAG_i32]] = !{[[TYPE_i32:!.*]], [[TYPE_i32]], i64 0}
@@ -222,11 +265,17 @@
 // OLD-PATH: [[TYPE_S]] = !{!"_ZTS7StructS", [[TYPE_SHORT]], i64 0, [[TYPE_INT]], i64 4}
 // OLD-PATH: [[TAG_S_f16]] = !{[[TYPE_S]], [[TYPE_SHORT]], i64 0}
 // OLD-PATH: [[TAG_S2_f32_2]] = !{[[TYPE_S2:!.*]], [[TYPE_INT]], i64 12}
-// OLD-PATH: [[TYPE_S2]] = !{!"_ZTS8StructS2", [[TYPE_SHORT]], i64 8, [[TYPE_INT]], i64 12}
+// OLD-PATH: [[TYPE_S2]] = !{!"_ZTS8StructS2", [[TYPE_S]], i64 0, [[TYPE_SHORT]], i64 8, [[TYPE_INT]], i64 12}
 // OLD-PATH: [[TAG_C_b_a_f32]] = !{[[TYPE_C:!.*]], [[TYPE_INT]], i64 12}
 // OLD-PATH: [[TYPE_C]] = !{!"_ZTS7StructC", [[TYPE_SHORT]], i64 0, [[TYPE_B]], i64 4, [[TYPE_INT]], i64 28}
 // OLD-PATH: [[TAG_D_b_a_f32]] = !{[[TYPE_D:!.*]], [[TYPE_INT]], i64 12}
 // OLD-PATH: [[TYPE_D]] = !{!"_ZTS7StructD", [[TYPE_SHORT]], i64 0, [[TYPE_B]], i64 4, [[TYPE_INT]], i64 28, [[TYPE_CHAR]], i64 32}
+// OLD-PATH: [[TAG_M1_f16_2]] = !{[[TYPE_M1:!.*]], [[TYPE_SHORT]], i64 12}
+// OLD-PATH: [[TYPE_M1]] = !{!"_ZTS8StructM1", [[TYPE_S]], i64 0, [[TYPE_T:!.*]], i64 8, [[TYPE_SHORT]], i64 12}
+// OLD_PATH: [[TYPE_T]] = !{!"_ZTS7StructT", [[TYPE_INT]], i64 0}
+// OLD-PATH: [[TAG_M2_f16_2]] = !{[[TYPE_M2:!.*]], [[TYPE_SHORT]], i64 20}
+// OLD-PATH: [[TYPE_M2]] = !{!"_ZTS8StructM2", [[TYPE_DYN:!.*]], i64 0, [[TYPE_S]], i64 12, [[TYPE_SHORT]], i64 20}
+// OLD_PATH: [[TYPE_DYN]] = !{!"_ZTS9StructDyn", [[TYPE_INT]], i64 8}
 

[PATCH] D129100: [clangd] Support external throttler for preamble builds

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/ClangdServer.h:108
+/// This throttler controls which preambles may be built at a given time.
+clangd::PreambleThrottler *PreambleThrottler = nullptr;
+

kadircet wrote:
> why the qualification ?
same name is used for the type and the member name, an error in at least some 
compilers (I forget what the standard says).
I can't think of another good name



Comment at: clang-tools-extra/clangd/TUScheduler.h:114
+  // If it fails (e.g. races), resources are acquired and must be released.
+  virtual CancelFn acquire(llvm::StringRef Filename, AcquireCallback) = 0;
+

kadircet wrote:
> can we make the first parameter a struct instead? i feel like we might end up 
> adding more parameters describing the reason for the request here.
My hypothesis is that *all* other parameters will be provided separately on 
unrelated codepaths (Throttler->setFoo(Filename, Foo)) and the name here is a 
join key rather than one parameter among many.

I might be wrong about this but would rather hold off on generalizing until 
we're sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129100

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

[PATCH] D129180: [clang][dataflow] Return a solution from the solver when `Constraints` are `Satisfiable`.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/Solver.h:45
+enum class Assignment : int8_t {
+  Unassigned = -1,
+  AssignedFalse = 0,

A solution consists of true/false assignments for all variables. Having an 
`Unassigned` option seems confusing. I suggest defining only two values in this 
enum and translating `WatchedLiteralsSolver`'s internal representation to 
`Solution` right before returning it, in `buildValAssignment`.



Comment at: clang/include/clang/Analysis/FlowSensitive/Solver.h:50-57
+static Result Unsatisfiable() { return Result(Status::Unsatisfiable, {}); }
+
+static Result TimedOut() { return Result(Status::TimedOut, {}); }
+
+static Result
+Satisfiable(llvm::DenseMap Solution) {
+  return Result(Status::Satisfiable, std::move(Solution));

Let's document these members.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129180

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

[PATCH] D128204: [clangd] Add fix-it for inserting IWYU pragma: keep

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

> A specific example i encountered is clang/Tooling/DiagnosticsYaml.h Which 
> defines template specializations for inputting/outputting yaml io. That file 
> must be included if you ever want to emit diagnostics as YAML, but the 
> typical use case is to just use the operator<< on a yaml stream. This 
> function internally will use those specializations, but as clangd doesn't 
> look into the function calls and expand all those instantiations, they are 
> treated as being unused(There's many bugs already related to this) and as 
> such the header is marked as being unused.

i see makes sense. this falls in the "adl through templates" bucket, which is 
annoying and doesn't seem to be as rare in the wild. so we probably need to 
figure something out here.
as mentioned above i've hesitations for having this automatically available but 
it's a state we want to get at eventually. i guess there's not much point in 
delaying that, so LGTM (after comments already raised are addressed), sorry for 
the late reply and thanks for the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128204

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

[clang-tools-extra] ed0e20d - [clangd] Support external throttler for preamble builds

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-07-06T14:58:24+02:00
New Revision: ed0e20d5e8c5d6c679d2cead674654541fa10e1b

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

LOG: [clangd] Support external throttler for preamble builds

Building preambles is the most resource-intensive thing clangd does, driving
peak RAM and sustained CPU usage.

In a hosted environment where multiple clangd instances are packed into the same
container, it's useful to be able to limit the *aggregate* resource peaks.

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 4d9dbc773..f28b322c0bbdd 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -166,6 +166,7 @@ ClangdServer::Options::operator TUScheduler::Options() 
const {
   Opts.StorePreamblesInMemory = StorePreamblesInMemory;
   Opts.UpdateDebounce = UpdateDebounce;
   Opts.ContextProvider = ContextProvider;
+  Opts.PreambleThrottler = PreambleThrottler;
   return Opts;
 }
 

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index e73454901cff0..dacb7b74af99d 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -104,6 +104,9 @@ class ClangdServer {
 /// Cached preambles are potentially large. If false, store them on disk.
 bool StorePreamblesInMemory = true;
 
+/// This throttler controls which preambles may be built at a given time.
+clangd::PreambleThrottler *PreambleThrottler = nullptr;
+
 /// If true, ClangdServer builds a dynamic in-memory index for symbols in
 /// opened files and uses the index to augment code completion results.
 bool BuildDynamicSymbolIndex = false;

diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp 
b/clang-tools-extra/clangd/TUScheduler.cpp
index 49d850ad96a09..a112b666e9ab2 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -381,6 +381,41 @@ class SynchronizedTUStatus {
   ParsingCallbacks &Callbacks;
 };
 
+// An attempt to acquire resources for a task using PreambleThrottler.
+// Initially it is unsatisfied, it (hopefully) becomes satisfied later but may
+// be destroyed before then. Destruction releases all resources.
+class PreambleThrottlerRequest {
+public:
+  // The condition variable is signalled when the request is satisfied.
+  PreambleThrottlerRequest(llvm::StringRef Filename,
+   PreambleThrottler *Throttler,
+   std::condition_variable &CV)
+  : Throttler(Throttler), Satisfied(Throttler == nullptr) {
+// If there is no throttler, this dummy request is always satisfied.
+if (!Throttler)
+  return;
+ID = Throttler->acquire(Filename, [&] {
+  Satisfied.store(true, std::memory_order_release);
+  CV.notify_all();
+});
+  }
+
+  bool satisfied() const { return Satisfied.load(std::memory_order_acquire); }
+
+  // When the request is destroyed:
+  //  - if resources are not yet obtained, stop trying to get them.
+  //  - if resources were obtained, release them.
+  ~PreambleThrottlerRequest() {
+if (Throttler)
+  Throttler->release(ID);
+  }
+
+private:
+  PreambleThrottler::RequestID ID;
+  PreambleThrottler *Throttler;
+  std::atomic Satisfied = {false};
+};
+
 /// Responsible for building preambles. Whenever the thread is idle and the
 /// preamble is outdated, it starts to build a fresh preamble from the latest
 /// inputs. If RunSync is true, preambles are built synchronously in update()
@@ -389,12 +424,13 @@ class PreambleThread {
 public:
   PreambleThread(llvm::StringRef FileName, ParsingCallbacks &Callbacks,
  bool StorePreambleInMemory, bool RunSync,
- SynchronizedTUStatus &Status,
+ PreambleThrottler *Throttler, SynchronizedTUStatus &Status,
  TUScheduler::HeaderIncluderCache &HeaderIncluders,
  ASTWorker &AW)
   : FileName(FileName), Callbacks(Callbacks),
-StoreInMemory(StorePreambleInMemory), RunSync(RunSync), Status(Status),
-ASTPeer(AW), HeaderIncluders(HeaderIncluders) {}
+StoreInMemory(StorePreambleInMemory), RunSync(RunSync),
+Throttler(Throttler), Status(Status), ASTPeer(AW),
+HeaderIncluders(HeaderIncluders) {}
 
   /// It isn't guaranteed that each requested version will be built. If ther

[PATCH] D129100: [clangd] Support external throttler for preamble builds

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rGed0e20d5e8c5: [clangd] Support external throttler for 
preamble builds (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D129100?vs=442319&id=442539#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129100

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1372,6 +1372,150 @@
 CheckNoFileActionsSeesLastActiveFile(LastActive);
   }
 }
+
+TEST_F(TUSchedulerTests, PreambleThrottle) {
+  const int NumRequests = 4;
+  // Silly throttler that waits for 4 requests, and services them in reverse.
+  // Doesn't honor cancellation but records it.
+  struct : public PreambleThrottler {
+std::mutex Mu;
+std::vector Acquires;
+std::vector Releases;
+llvm::DenseMap Callbacks;
+// If set, the notification is signalled after acquiring the specified ID.
+llvm::Optional> Notify;
+
+RequestID acquire(llvm::StringRef Filename, Callback CB) override {
+  RequestID ID;
+  Callback Invoke;
+  {
+std::lock_guard Lock(Mu);
+ID = Acquires.size();
+Acquires.emplace_back(Filename);
+// If we're full, satisfy this request immediately.
+if (Acquires.size() == NumRequests) {
+  Invoke = std::move(CB);
+} else {
+  Callbacks.try_emplace(ID, std::move(CB));
+}
+  }
+  if (Invoke)
+Invoke();
+  if (Notify && ID == Notify->first) {
+Notify->second->notify();
+Notify.reset();
+  }
+  return ID;
+}
+
+void release(RequestID ID) override {
+  Releases.push_back(ID);
+  Callback SatisfyNext;
+  {
+std::lock_guard Lock(Mu);
+if (ID > 0)
+  SatisfyNext = std::move(Callbacks[ID - 1]);
+  }
+  if (SatisfyNext)
+SatisfyNext();
+}
+
+void reset() {
+  Acquires.clear();
+  Releases.clear();
+  Callbacks.clear();
+}
+  } Throttler;
+
+  struct CaptureBuiltFilenames : public ParsingCallbacks {
+std::vector &Filenames;
+CaptureBuiltFilenames(std::vector &Filenames)
+: Filenames(Filenames) {}
+void onPreambleAST(PathRef Path, llvm::StringRef Version,
+   const CompilerInvocation &CI, ASTContext &Ctx,
+   Preprocessor &PP, const CanonicalIncludes &) override {
+  // Deliberately no synchronization.
+  // The PreambleThrottler should serialize these calls, if not then tsan
+  // will find a bug here.
+  Filenames.emplace_back(Path);
+}
+  };
+
+  auto Opts = optsForTest();
+  Opts.AsyncThreadsCount = 2 * NumRequests; // throttler is the bottleneck
+  Opts.PreambleThrottler = &Throttler;
+
+  std::vector Filenames;
+
+  {
+std::vector BuiltFilenames;
+TUScheduler S(CDB, Opts,
+  std::make_unique(BuiltFilenames));
+for (unsigned I = 0; I < NumRequests; ++I) {
+  auto Path = testPath(std::to_string(I) + ".cc");
+  Filenames.push_back(Path);
+  S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes);
+}
+ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+
+// The throttler saw all files, and we built them.
+EXPECT_THAT(Throttler.Acquires,
+testing::UnorderedElementsAreArray(Filenames));
+EXPECT_THAT(BuiltFilenames,
+testing::UnorderedElementsAreArray(Filenames));
+// We built the files in reverse order that the throttler saw them.
+EXPECT_THAT(BuiltFilenames,
+testing::ElementsAreArray(Throttler.Acquires.rbegin(),
+  Throttler.Acquires.rend()));
+// Resources for each file were correctly released.
+EXPECT_THAT(Throttler.Releases, ElementsAre(3, 2, 1, 0));
+  }
+
+  Throttler.reset();
+
+  // This time, enqueue 2 files, then cancel one of them while still waiting.
+  // Finally shut down the server. Observe that everything gets cleaned up.
+  Notification AfterAcquire2;
+  Notification AfterFinishA;
+  Throttler.Notify = {1, &AfterAcquire2};
+  std::vector BuiltFilenames;
+  auto A = testPath("a.cc");
+  auto B = testPath("b.cc");
+  Filenames = {A, B};
+  {
+TUScheduler S(CDB, Opts,
+  std::make_unique(BuiltFilenames));
+updateWithCallback(S, A, getInputs(A, ""), WantDiagnostics::Yes,
+   [&] { AfterFin

[PATCH] D129158: [pseudo] Define recovery strategy as grammar extension.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:54
+  return {
+  {(ExtensionID)Extension::Brackets, recoverBrackets},
+  };

hokein wrote:
> btw, it is annoying to write an explicit ExtensionID<=>Extension conversion. 
> To avoid that, what do you think of making the Symbol&Extension as an `enum` 
> rather than `enum class`, and we still keep the qualified name usage 
> `Extension::Brackets`.
I don't find it particularly annoying FWIW.
I think I'd rather deal with this (which only appears in these tables, which 
are regular) than implicit conversion and also polluting the namespace (even if 
you only *use* Extension::Brackets, cxx::Brackets still exists, ends up in code 
completion, can conflict with other symbols etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129158

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

[clang-tools-extra] 7d8e274 - [pseudo] Define recovery strategy as grammar extension.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-07-06T15:03:38+02:00
New Revision: 7d8e2742d958468b900532f1e44610b9b0b29f5e

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

LOG: [pseudo] Define recovery strategy as grammar extension.

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

Added: 


Modified: 
clang-tools-extra/pseudo/include/clang-pseudo/Language.h
clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRGraph.h
clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
clang-tools-extra/pseudo/lib/GLR.cpp
clang-tools-extra/pseudo/lib/cli/CLI.cpp
clang-tools-extra/pseudo/lib/cxx/CXX.cpp
clang-tools-extra/pseudo/lib/cxx/cxx.bnf
clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
clang-tools-extra/pseudo/lib/grammar/LRGraph.cpp
clang-tools-extra/pseudo/test/cxx/empty-member-spec.cpp
clang-tools-extra/pseudo/test/cxx/recovery-init-list.cpp
clang-tools-extra/pseudo/unittests/GLRTest.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/Language.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/Language.h
index 8c2fdfaa2852c..24267c429a965 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/Language.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/Language.h
@@ -9,6 +9,7 @@
 #ifndef CLANG_PSEUDO_GRAMMAR_LANGUAGE_H
 #define CLANG_PSEUDO_GRAMMAR_LANGUAGE_H
 
+#include "clang-pseudo/Token.h"
 #include "clang-pseudo/grammar/Grammar.h"
 #include "clang-pseudo/grammar/LRTable.h"
 
@@ -28,13 +29,25 @@ class LRTable;
 using RuleGuard = llvm::function_ref RHS, const TokenStream &)>;
 
+// A recovery strategy determines a region of code to skip when parsing fails.
+//
+// For example, given `class-def := CLASS IDENT { body [recover=Brackets] }`,
+// if parsing fails while attempting to parse `body`, we may skip up to the
+// matching `}` and assume everything between was a `body`.
+//
+// The provided index is the token where the skipped region begins.
+// Returns the (excluded) end of the range, or Token::Invalid for no recovery.
+using RecoveryStrategy =
+llvm::function_ref;
+
 // Specify a language that can be parsed by the pseduoparser.
 struct Language {
   Grammar G;
   LRTable Table;
 
-  // Binding "guard" extension id to a piece of C++ code.
+  // Binding extension ids to corresponding implementations.
   llvm::DenseMap Guards;
+  llvm::DenseMap RecoveryStrategies;
 
   // FIXME: add clang::LangOptions.
   // FIXME: add default start symbols.

diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
index e2957cc2ec71e..ab11a84ebf295 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
@@ -85,9 +85,6 @@ inline tok::TokenKind symbolToToken(SymbolID SID) {
 inline constexpr SymbolID tokenSymbol(tok::TokenKind TK) {
   return TokenFlag | static_cast(TK);
 }
-// Error recovery strategies.
-// FIXME: these should be provided as extensions instead.
-enum class RecoveryStrategy : uint8_t { None, Braces };
 
 // An extension is a piece of native code specific to a grammar that modifies
 // the behavior of annotated rules. One ExtensionID is assigned for each unique
@@ -133,7 +130,7 @@ struct Rule {
   // everything between braces.
   // For now, only a single strategy at a single point is possible.
   uint8_t RecoveryIndex = -1;
-  RecoveryStrategy Recovery = RecoveryStrategy::None;
+  ExtensionID Recovery = 0;
 
   llvm::ArrayRef seq() const {
 return llvm::ArrayRef(Sequence, Size);

diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRGraph.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRGraph.h
index f5c2cc7a16232..dd9e87c2c172b 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRGraph.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRGraph.h
@@ -144,7 +144,7 @@ class LRGraph {
   // has a Recovery { Src = S, Strategy=braces, Result=stmt-seq }.
   struct Recovery {
 StateID Src; // The state we are in when encountering the error.
-RecoveryStrategy Strategy; // Heuristic choosing the tokens to match.
+ExtensionID Strategy;  // Heuristic choosing the tokens to match.
 SymbolID Result;   // The symbol that is produced.
   };
 

diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
index aac80ad53ffd0..890a5125a1ef4 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
+++ b/clang-tools-extra/pseudo/in

[PATCH] D129158: [pseudo] Define recovery strategy as grammar extension.

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d8e2742d958: [pseudo] Define recovery strategy as grammar 
extension. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D129158?vs=442383&id=442541#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129158

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/Language.h
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRGraph.h
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/cli/CLI.cpp
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
  clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
  clang-tools-extra/pseudo/lib/grammar/LRGraph.cpp
  clang-tools-extra/pseudo/test/cxx/empty-member-spec.cpp
  clang-tools-extra/pseudo/test/cxx/recovery-init-list.cpp
  clang-tools-extra/pseudo/unittests/GLRTest.cpp

Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -48,6 +48,17 @@
testing::UnorderedElementsAreArray(Parents));
 }
 
+Token::Index recoverBraces(Token::Index Begin, const TokenStream &Code) {
+  EXPECT_GT(Begin, 0u);
+  const Token &Left = Code.tokens()[Begin - 1];
+  EXPECT_EQ(Left.Kind, tok::l_brace);
+  if (const auto* Right = Left.pair()) {
+EXPECT_EQ(Right->Kind, tok::r_brace);
+return Code.index(*Right);
+  }
+  return Token::Invalid;
+}
+
 class GLRTest : public ::testing::Test {
 public:
   void build(llvm::StringRef GrammarBNF) {
@@ -375,11 +386,12 @@
   //0--1({)--2(?)
   //  After recovering a `word` at state 1:
   //0--3(word)  // 3 is goto(1, word)
-  buildGrammar({"word"}, {});
+  buildGrammar({"word", "top"}, {"top := { word [recover=Braces] }"});
   LRTable::Builder B(TestLang.G);
   B.Transition[{StateID{1}, id("word")}] = StateID{3};
-  B.Recoveries.push_back({StateID{1}, {RecoveryStrategy::Braces, id("word")}});
+  B.Recoveries.push_back({StateID{1}, {extensionID("Braces"), id("word")}});
   TestLang.Table = std::move(B).build();
+  TestLang.RecoveryStrategies.try_emplace(extensionID("Braces"), recoverBraces);
 
   auto *LBrace = &Arena.createTerminal(tok::l_brace, 0);
   auto *Question1 = &Arena.createTerminal(tok::question, 1);
@@ -418,11 +430,12 @@
   //0--1({)--1({)--1({)
   //  After recovering a `block` at inside the second braces:
   //0--1({)--2(body)  // 2 is goto(1, body)
-  buildGrammar({"body"}, {});
+  buildGrammar({"body", "top"}, {"top := { body [recover=Braces] }"});
   LRTable::Builder B(TestLang.G);
   B.Transition[{StateID{1}, id("body")}] = StateID{2};
-  B.Recoveries.push_back({StateID{1}, {RecoveryStrategy::Braces, id("body")}});
+  B.Recoveries.push_back({StateID{1}, {extensionID("Braces"), id("body")}});
   TestLang.Table = std::move(B).build();
+  TestLang.RecoveryStrategies.try_emplace(extensionID("Braces"), recoverBraces);
 
   clang::LangOptions LOptions;
   // Innermost brace is unmatched, to test fallback to next brace.
@@ -455,13 +468,17 @@
   //  After recovering either `word` or `number` inside the braces:
   //0--1({)--2(word)   // 2 is goto(1, word)
   //  └--3(number) // 3 is goto(1, number)
-  buildGrammar({"number", "word"}, {});
+  buildGrammar({"number", "word", "top"},
+   {
+   "top := { number [recover=Braces] }",
+   "top := { word [recover=Braces] }",
+   });
   LRTable::Builder B(TestLang.G);
   B.Transition[{StateID{1}, id("number")}] = StateID{2};
   B.Transition[{StateID{1}, id("word")}] = StateID{3};
-  B.Recoveries.push_back(
-  {StateID{1}, {RecoveryStrategy::Braces, id("number")}});
-  B.Recoveries.push_back({StateID{1}, {RecoveryStrategy::Braces, id("word")}});
+  B.Recoveries.push_back({StateID{1}, {extensionID("Braces"), id("number")}});
+  B.Recoveries.push_back({StateID{1}, {extensionID("Braces"), id("word")}});
+  TestLang.RecoveryStrategies.try_emplace(extensionID("Braces"), recoverBraces);
   TestLang.Table = std::move(B).build();
   auto *LBrace = &Arena.createTerminal(tok::l_brace, 0);
   const auto *Root = GSStack.addNode(0, nullptr, {});
@@ -560,11 +577,12 @@
   build(R"bnf(
 _ := block
 
-block := { block }
-block := { numbers }
+block := { block [recover=Braces] }
+block := { numbers [recover=Braces] }
 numbers := NUMERIC_CONSTANT NUMERIC_CONSTANT
   )bnf");
   TestLang.Table = LRTable::buildSLR(TestLang.G);
+  TestLang.RecoveryStrategies.try_emplace(extensionID("Braces"), recoverBraces);
   clang::

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

[Erich Keane via Phabricator via cfe-commits]

erichkeane added subscribers: aaron.ballman, erichkeane.
erichkeane added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:305
 
+- Some type traits builtins like ``__has_trivial_assign`` have been documented
+  as deprecated for a while, because their semantics don't mix well with 
post-C++11 type





Comment at: clang/docs/ReleaseNotes.rst:306
+- Some type traits builtins like ``__has_trivial_assign`` have been documented
+  as deprecated for a while, because their semantics don't mix well with 
post-C++11 type
+  traits. Clang now emits deprecation warnings for them under the flag 
``-Wdeprecated-has-builtins``.





Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5561
+def warn_deprecated_has_builtins : Warning<
+  "the %0 compiler builtin is deprecated from C++11 onwards. Use %1 instead.">,
+  InGroup;

cor3ntin wrote:
> Should we say something like "and will be removed in the future"?
> 
> "%0 is deprecated from C++11 onward. Use %1 instead." might be sufficient
> 
> 
Diagnostics arent sentences. Also, we wouldn't say "C++11 onward", we can just 
say C++11. I might suggest:

`builtin %0 is deprecated in C++11, use %1 instead`

BUT @aaron.ballman always does great at this level of bikeshedding.




Comment at: clang/lib/Sema/SemaExprCXX.cpp:5454
 
+  IssueBuiltinDeprecationWarningIfNeeded(*this, Kind, KWLoc);
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129170

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

[PATCH] D129198: [clang] Correct the macOS version that supports aligned allocation

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added a reviewer: ahatanak.
Herald added a project: All.
ldionne requested review of this revision.
Herald added projects: clang, libc++.
Herald added subscribers: libcxx-commits, cfe-commits.
Herald added a reviewer: libc++.

After checking the libc++abi.dylib shipped in macOS 10.13, I can confirm
that it contains the align_val_t variants of operator new and operator
delete. However, the libc++abi.dylib shipped on macOS 10.12 does not.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129198

Files:
  clang/include/clang/Basic/AlignedAllocation.h
  clang/test/Driver/unavailable_aligned_allocation.cpp
  clang/test/SemaCXX/unavailable_aligned_allocation.cpp
  
libcxx/test/libcxx/language.support/support.dynamic/aligned_alloc_availability.verify.cpp
  libcxx/test/libcxx/memory/aligned_allocation_macro.compile.pass.cpp
  
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t.pass.cpp
  
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t_nothrow.pass.cpp
  
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t_nothrow_replace.pass.cpp
  
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t.pass.cpp
  
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow.pass.cpp
  
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow_replace.pass.cpp

Index: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow_replace.pass.cpp
===
--- libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow_replace.pass.cpp
+++ libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow_replace.pass.cpp
@@ -11,9 +11,9 @@
 
 // XFAIL: LIBCXX-AIX-FIXME
 
-// Aligned allocation was not provided before macosx10.14 and as a result we
-// get availability errors when the deployment target is older than macosx10.14.
-// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13}}
+// Aligned allocation was not provided before macosx10.13 and as a result we
+// get availability errors when the deployment target is older than macosx10.13.
+// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12}}
 
 // Libcxx when built for z/OS doesn't contain the aligned allocation functions,
 // nor does the dynamic library shipped with z/OS.
Index: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow.pass.cpp
===
--- libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow.pass.cpp
+++ libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t_nothrow.pass.cpp
@@ -8,9 +8,9 @@
 
 // UNSUPPORTED: c++03, c++11, c++14
 
-// Aligned allocation was not provided before macosx10.14 and as a result we
-// get availability errors when the deployment target is older than macosx10.14.
-// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13}}
+// Aligned allocation was not provided before macosx10.13 and as a result we
+// get availability errors when the deployment target is older than macosx10.13.
+// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12}}
 
 // asan and msan will not call the new handler.
 // UNSUPPORTED: sanitizer-new-delete
Index: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t.pass.cpp
===
--- libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t.pass.cpp
+++ libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.single/new_align_val_t.pass.cpp
@@ -8,9 +8,9 @@
 
 // UNSUPPORTED: c++03, c++11, c++14
 
-// Aligned allocation was not provided before macosx10.14 and as a result we
-// get availability errors when the deployment target is older than macosx10.14.
-// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13}}
+// Aligned allocation was not provided before macosx10.13 and as a result we
+// get availability errors when the deployment target is older than macosx10.13.
+// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12}}
 
 // asan and msan will not call the new handler.
 // UNSUPPORTED: sanitizer-new-delete
Index: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_align_val_t_nothrow_replace.pass.cpp
===
--- libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new

[clang] 923b56e - [NFC] Add a TODO comment to apply nounwind attribute in all GPU modes.

[Alexey Bader via cfe-commits]

Author: Alexey Bader
Date: 2022-07-06T06:20:09-07:00
New Revision: 923b56e7ca96e03cedcb0e3a5df5c05e8e975a38

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

LOG: [NFC] Add a TODO comment to apply nounwind attribute in all GPU modes.

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 4e26c35c6342..104a30dd6b25 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1931,6 +1931,9 @@ void 
CodeGenModule::getDefaultFunctionAttributes(StringRef Name,
 FuncAttrs.addAttribute(llvm::Attribute::Convergent);
   }
 
+  // TODO: NoUnwind attribute should be added for other GPU modes OpenCL, HIP,
+  // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
+  // code.
   if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice) {
 // Exceptions aren't supported in CUDA device code.
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);



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

[clang-tools-extra] 511a7ee - [clangd] Fix tests after ed0e20d5e8c5d6c679d2c

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-07-06T15:22:05+02:00
New Revision: 511a7eef937ebc1e25efc90bf41f12b52ad1dff3

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

LOG: [clangd] Fix tests after ed0e20d5e8c5d6c679d2c

Added: 


Modified: 
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp 
b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index 39ab1638efa8..995ad992d51b 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1413,7 +1413,7 @@ TEST_F(TUSchedulerTests, PreambleThrottle) {
   Callback SatisfyNext;
   {
 std::lock_guard Lock(Mu);
-if (ID > 0)
+if (ID > 0 && Acquires.size() == NumRequests)
   SatisfyNext = std::move(Callbacks[ID - 1]);
   }
   if (SatisfyNext)



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

[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

[Louis Dionne via Phabricator via cfe-commits]

ldionne updated this revision to Diff 442549.
ldionne marked 2 inline comments as done.
ldionne added a comment.

Rebase and address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125683

Files:
  clang/cmake/caches/Android.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  compiler-rt/cmake/Modules/AddCompilerRT.cmake
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxx/cmake/caches/MinGW.cmake
  libcxx/docs/BuildingLibcxx.rst
  libcxx/docs/ReleaseNotes.rst
  libcxx/src/CMakeLists.txt
  libcxx/test/CMakeLists.txt
  libcxx/test/configs/legacy.cfg.in
  libcxx/utils/ci/run-buildbot
  llvm/docs/HowToBuildWindowsItaniumPrograms.rst

Index: llvm/docs/HowToBuildWindowsItaniumPrograms.rst
===
--- llvm/docs/HowToBuildWindowsItaniumPrograms.rst
+++ llvm/docs/HowToBuildWindowsItaniumPrograms.rst
@@ -124,7 +124,6 @@
 * ``-DLIBCXXABI_ENABLE_SHARED=OFF``
 * ``-DLIBCXXABI_ENABLE_STATIC=ON``
 * ``-DLIBCXX_ENABLE_SHARED=ON'``
-* ``-DLIBCXX_ENABLE_STATIC_ABI_LIBRARY=ON``
 
 To break the symbol dependency between libc++abi and libc++ we
 build libc++abi as a static library and then statically link it
@@ -157,8 +156,7 @@
 
 Windows Itanium does not offer a POSIX-like layer over WIN32.
 
-* ``-DLIBCXX_ENABLE_STATIC_ABI_LIBRARY=ON``
-* ``-DLIBCXX_CXX_ABI=libcxxabi``
+* ``-DLIBCXX_CXX_ABI=libcxxabi-objects``
 * ``-DLIBCXX_CXX_ABI_INCLUDE_PATHS=/include``
 * ``-DLIBCXX_CXX_ABI_LIBRARY_PATH=/lib``
 
Index: libcxx/utils/ci/run-buildbot
===
--- libcxx/utils/ci/run-buildbot
+++ libcxx/utils/ci/run-buildbot
@@ -93,7 +93,6 @@
 function generate-cmake() {
 generate-cmake-base \
   -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
-  -DLIBCXX_CXX_ABI=libcxxabi \
   "${@}"
 }
 
@@ -477,8 +476,7 @@
   -GNinja -DCMAKE_MAKE_PROGRAM="${NINJA}" \
   -DCMAKE_BUILD_TYPE=RelWithDebInfo \
   -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
-  -DLLVM_LIT_ARGS="-sv --show-unsupported --xunit-xml-output test-results.xml --timeout=1500" \
-  -DLIBCXX_CXX_ABI=libcxxabi
+  -DLLVM_LIT_ARGS="-sv --show-unsupported --xunit-xml-output test-results.xml --timeout=1500"
 check-runtimes
 ;;
 aarch64)
Index: libcxx/test/configs/legacy.cfg.in
===
--- libcxx/test/configs/legacy.cfg.in
+++ libcxx/test/configs/legacy.cfg.in
@@ -14,7 +14,7 @@
 config.cxx_library_root = "@LIBCXX_LIBRARY_DIR@"
 config.abi_library_root = "@LIBCXX_CXX_ABI_LIBRARY_PATH@"
 config.enable_shared= @LIBCXX_LINK_TESTS_WITH_SHARED_LIBCXX@
-config.cxx_abi  = "@LIBCXX_CXXABI_FOR_TESTS@"
+config.cxx_abi  = "@LIBCXX_CXX_ABI@"
 config.configuration_variant= "@LIBCXX_LIT_VARIANT@"
 config.host_triple  = "@LLVM_HOST_TRIPLE@"
 config.sysroot  = "@CMAKE_SYSROOT@"
Index: libcxx/test/CMakeLists.txt
===
--- libcxx/test/CMakeLists.txt
+++ libcxx/test/CMakeLists.txt
@@ -14,14 +14,6 @@
 set(LIBCXX_TEST_COMPILER_FLAGS "" CACHE STRING
 "Additonal linker flags to pass when compiling the tests")
 
-# The tests shouldn't link to any ABI library when it has been linked into
-# libc++ statically or via a linker script.
-if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY OR LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
-  set(LIBCXX_CXXABI_FOR_TESTS "none")
-else()
-  set(LIBCXX_CXXABI_FOR_TESTS "${LIBCXX_CXX_ABI}")
-endif()
-
 # The tests shouldn't link to libunwind if we have a linker script which
 # already does so.
 if (LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
Index: libcxx/src/CMakeLists.txt
===
--- libcxx/src/CMakeLists.txt
+++ libcxx/src/CMakeLists.txt
@@ -204,7 +204,7 @@
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
   target_include_directories(cxx_shared PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
-  target_link_libraries(cxx_shared PUBLIC cxx-headers
+  target_link_libraries(cxx_shared PUBLIC cxx-headers libcxx-abi-shared
PRIVATE ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES
@@ -232,19 +232,9 @@
 endif()
   endif()
 
-  # Link against libc++abi
-  if (LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY)
-target_link_libraries(cxx_shared PRIVATE libcxx-abi-shared-objects)
-  else()
-target_link_libraries(cxx_shared PUBLIC libcxx-abi-shared)
-  endif()
-
   # Maybe re-export symbols from libc++abi
-  # In particular, we don't re-export the symbols if libc++abi is merged statically
-  # into libc++ because in that case there's no dylib to re-export from.
   if (APPLE AND

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Shraiysh via Phabricator via cfe-commits]

shraiysh added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596
   // Check for unsupported clauses
-  if (!S.clauses().empty()) {
-// Currently no clause is supported
-return false;
+  for (OMPClause *C : S.clauses()) {
+// Currently only simdlen clause is supported

psoni2628 wrote:
> shraiysh wrote:
> > psoni2628 wrote:
> > > psoni2628 wrote:
> > > > arnamoy10 wrote:
> > > > > I am just wondering whether we should have a check to make sure that 
> > > > > we are processing the clauses of only `simd` directive here. Because 
> > > > > the function takes a general `OMPExecutableDirective` as argument 
> > > > That's a fair point. I guess `isSupportedByOpenMPIRBuilder` could be 
> > > > used for other directive types other than simd, even though it's not 
> > > > right now.
> > > Would it make more sense to only guard the checking of clauses with a 
> > > check for `OMPSimdDirective`, or the whole thing? I believe even the code 
> > > below, which checks for an ordered directive, is also specifically for 
> > > `simd`?
> > > 
> > > 
> > > Example of guarding the whole thing:
> > > 
> > > ```
> > >   if(dyn_cast(S)) {
> > > // Check for unsupported clauses
> > > for (OMPClause *C : S.clauses()) {
> > >   // Currently only simdlen clause is supported
> > >   if (dyn_cast(C))
> > > continue;
> > >   else
> > > return false;
> > > }
> > > 
> > > // Check if we have a statement with the ordered directive.
> > > // Visit the statement hierarchy to find a compound statement
> > > // with a ordered directive in it.
> > > if (const auto *CanonLoop = 
> > > dyn_cast(S.getRawStmt())) {
> > >   if (const Stmt *SyntacticalLoop = CanonLoop->getLoopStmt()) {
> > > for (const Stmt *SubStmt : SyntacticalLoop->children()) {
> > >   if (!SubStmt)
> > > continue;
> > >   if (const CompoundStmt *CS = dyn_cast(SubStmt)) {
> > > for (const Stmt *CSSubStmt : CS->children()) {
> > >   if (!CSSubStmt)
> > > continue;
> > >   if (isa(CSSubStmt)) {
> > > return false;
> > >   }
> > > }
> > >   }
> > > }
> > >   }
> > > }
> > >   }
> > > ```
> > Can we instead have separate `isSupportedByOpenMPIRBuilder` for every 
> > directive to avoid bloating the function with checks and if conditions?
> > ```
> > static bool isSupportedByOpenMPIRBuilder(const OMPSimdDirective &S) {...}
> > void CodeGenFunction::EmitOMPSimdDirective(const OMPSimdDirective &S) {...}
> > 
> > static bool isSupportedByOpenMPIRBuilder(const OMPOrderedDirective &S) {...}
> > void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective &S) 
> > {...}
> > 
> > static bool isSupportedByOpenMPIRBuilder(const OMPTaskDirective &S) {...}
> > void CodeGenFunction::EmitOMPOrderedDirective(const OMPTaskDirective &S) 
> > {...}
> > ```
> It was decided in D114379 to use `OMPExecutableDirective` in order to allow 
> this function to be reused for constructs such as `for simd`. Do you wish to 
> undo this now, and instead specialize the function?
I wasn't aware of the earlier discussion about this. However it seems like it 
was suggested to try either splitting the function or merging it with slight 
preference for merging (I didn't understand how that would help constructs like 
`for simd` because I haven't worked with `for simd` much and will take others' 
word for it).

The suggested code change in previous reply - checking of clauses with a check 
for OMPSimdDirective - this seems like it would eventually become a huge 
function because of such checks. 

@Meinersbur it would be great if you could please advise on this - if keeping 
the checks for all executable directives in the same function will be helpful 
in the long run - allowing us to reuse checks or should we split it? A third 
alternative would be to have both functions, one specialized for SIMD and one 
for ExecutableConstruct, where the latter might handle combined constructs like 
`for simd` etc. Would that arrangement work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129149

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

[PATCH] D124159: [SimplifyCFG] Thread branches on same condition in more cases (PR54980)

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

@nathanchance Thanks for the report. I've applied the obvious fix in 
https://github.com/llvm/llvm-project/commit/20962c1240691d25b21ce425313c81eed0b1b358.
 However, I think that this (and similar limitations relating to critical edge 
splitting) might actually be unnecessary since 
https://github.com/llvm/llvm-project/commit/7a7bba289521e6d4da199565cf72dc9eaed3d671.
 callbr has stricter semantics than indirectbr, and apparently updating 
successors of a callbr //is// supposed to be possible (because the 
blockaddresses used by it must be directly used in the call arguments and thus 
can be updated, while in the indirectbr case they may be passed indirectly).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124159

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

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Prabhdeep Soni via Phabricator via cfe-commits]

psoni2628 updated this revision to Diff 442555.
psoni2628 marked 3 inline comments as done.
psoni2628 added a comment.

- Create new tests instead of modifying existing ones
- Specialize `isSupportedByOpenMPIRBuilder` for `OMPSimdDirective`


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

https://reviews.llvm.org/D129149

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/irbuilder_simd.cpp
  clang/test/OpenMP/irbuilder_simdlen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  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
@@ -1794,6 +1794,39 @@
   }));
 }
 
+TEST_F(OpenMPIRBuilderTest, ApplySimdlen) {
+  OpenMPIRBuilder OMPBuilder(*M);
+
+  CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
+
+  // Simd-ize the loop.
+  OMPBuilder.applySimd(DL, CLI);
+  OMPBuilder.applySimdlen(DL, CLI, ConstantInt::get(Type::getInt32Ty(Ctx), 3));
+
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+
+  PassBuilder PB;
+  FunctionAnalysisManager FAM;
+  PB.registerFunctionAnalyses(FAM);
+  LoopInfo &LI = FAM.getResult(*F);
+
+  const std::vector &TopLvl = LI.getTopLevelLoops();
+  EXPECT_EQ(TopLvl.size(), 1u);
+
+  Loop *L = TopLvl.front();
+  EXPECT_TRUE(findStringMetadataForLoop(L, "llvm.loop.parallel_accesses"));
+  EXPECT_TRUE(getBooleanLoopAttribute(L, "llvm.loop.vectorize.enable"));
+  EXPECT_EQ(getIntLoopAttribute(L, "llvm.loop.vectorize.width"), 3);
+
+  // Check for llvm.access.group metadata attached to the printf
+  // function in the loop body.
+  BasicBlock *LoopBody = CLI->getBody();
+  EXPECT_TRUE(any_of(*LoopBody, [](Instruction &I) {
+return I.getMetadata("llvm.access.group") != nullptr;
+  }));
+}
+
 TEST_F(OpenMPIRBuilderTest, UnrollLoopFull) {
   OpenMPIRBuilder OMPBuilder(*M);
 
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2881,6 +2881,15 @@
  BoolConst})});
 }
 
+void OpenMPIRBuilder::applySimdlen(DebugLoc, CanonicalLoopInfo *CanonicalLoop,
+   llvm::ConstantInt *Simdlen) {
+  LLVMContext &Ctx = Builder.getContext();
+  addLoopMetadata(
+  CanonicalLoop,
+  MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.width"),
+ConstantAsMetadata::get(Simdlen)}));
+}
+
 /// Create the TargetMachine object to query the backend for optimization
 /// preferences.
 ///
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -603,6 +603,14 @@
   /// \param Loop The loop to simd-ize.
   void applySimd(DebugLoc DL, CanonicalLoopInfo *Loop);
 
+  /// Add metadata for simdlen to a simd loop.
+  ///
+  /// \param DL  Debug location for instructions added by unrolling.
+  /// \param LoopThe simd loop.
+  /// \param Simdlen The Simdlen length to apply to the simd loop.
+  void applySimdlen(DebugLoc DL, CanonicalLoopInfo *Loop,
+llvm::ConstantInt *Simdlen);
+
   /// Generator for '#omp flush'
   ///
   /// \param Loc The location where the flush directive was encountered
Index: clang/test/OpenMP/irbuilder_simdlen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/irbuilder_simdlen.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-enable-irbuilder -verify -fopenmp -fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+struct S {
+  int a, b;
+};
+
+struct P {
+  int a, b;
+};
+
+void simple(float *a, float *b, int *c) {
+  S s, *p;
+  P pp;
+#pragma omp simd simdlen(3)
+  for (int i = 3; i < 32; i += 5) {
+// llvm.access.group test
+// CHECK: %[[A_ADDR:.+]] = alloca float*, align 8
+// CHECK: %[[B_ADDR:.+]] = alloca float*, align 8
+// CHECK: %[[S:.+]] = alloca %struct.S, align 4
+// CHECK: %[[P:.+]] = alloca %struct.S*, align 8
+// CHECK: %[[I:.+]] = alloca i32, align 4
+// CHECK: %[[TMP3:.+]] = load float*, float** %[[B_ADDR:.+]], align 8, !llvm.access.group ![[META3:[0-9]+]]
+// CHECK-NEXT: %[[TMP4:.+]] = load i32, i32* %[[I:.+]], align 4, !llvm.access.group ![[META3:[0-9]+]]
+// CHECK-NEXT: %[[IDXPROM:.+]] = sext i32 %[[TMP4:.+]] to i64
+// CHECK-NEXT: %[[ARRAYIDX:.+]] = getelementptr inbounds float, float* %[[TMP3:.+]], i64 %[[IDXPROM:.+]]
+// CHECK-NEXT: %[[TMP5:.+]] = load float, float* %[[ARRAYIDX:.+]], align 4, !llvm.access

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Prabhdeep Soni via Phabricator via cfe-commits]

psoni2628 added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2596
   // Check for unsupported clauses
-  if (!S.clauses().empty()) {
-// Currently no clause is supported
-return false;
+  for (OMPClause *C : S.clauses()) {
+// Currently only simdlen clause is supported

shraiysh wrote:
> psoni2628 wrote:
> > shraiysh wrote:
> > > psoni2628 wrote:
> > > > psoni2628 wrote:
> > > > > arnamoy10 wrote:
> > > > > > I am just wondering whether we should have a check to make sure 
> > > > > > that we are processing the clauses of only `simd` directive here. 
> > > > > > Because the function takes a general `OMPExecutableDirective` as 
> > > > > > argument 
> > > > > That's a fair point. I guess `isSupportedByOpenMPIRBuilder` could be 
> > > > > used for other directive types other than simd, even though it's not 
> > > > > right now.
> > > > Would it make more sense to only guard the checking of clauses with a 
> > > > check for `OMPSimdDirective`, or the whole thing? I believe even the 
> > > > code below, which checks for an ordered directive, is also specifically 
> > > > for `simd`?
> > > > 
> > > > 
> > > > Example of guarding the whole thing:
> > > > 
> > > > ```
> > > >   if(dyn_cast(S)) {
> > > > // Check for unsupported clauses
> > > > for (OMPClause *C : S.clauses()) {
> > > >   // Currently only simdlen clause is supported
> > > >   if (dyn_cast(C))
> > > > continue;
> > > >   else
> > > > return false;
> > > > }
> > > > 
> > > > // Check if we have a statement with the ordered directive.
> > > > // Visit the statement hierarchy to find a compound statement
> > > > // with a ordered directive in it.
> > > > if (const auto *CanonLoop = 
> > > > dyn_cast(S.getRawStmt())) {
> > > >   if (const Stmt *SyntacticalLoop = CanonLoop->getLoopStmt()) {
> > > > for (const Stmt *SubStmt : SyntacticalLoop->children()) {
> > > >   if (!SubStmt)
> > > > continue;
> > > >   if (const CompoundStmt *CS = dyn_cast(SubStmt)) 
> > > > {
> > > > for (const Stmt *CSSubStmt : CS->children()) {
> > > >   if (!CSSubStmt)
> > > > continue;
> > > >   if (isa(CSSubStmt)) {
> > > > return false;
> > > >   }
> > > > }
> > > >   }
> > > > }
> > > >   }
> > > > }
> > > >   }
> > > > ```
> > > Can we instead have separate `isSupportedByOpenMPIRBuilder` for every 
> > > directive to avoid bloating the function with checks and if conditions?
> > > ```
> > > static bool isSupportedByOpenMPIRBuilder(const OMPSimdDirective &S) {...}
> > > void CodeGenFunction::EmitOMPSimdDirective(const OMPSimdDirective &S) 
> > > {...}
> > > 
> > > static bool isSupportedByOpenMPIRBuilder(const OMPOrderedDirective &S) 
> > > {...}
> > > void CodeGenFunction::EmitOMPOrderedDirective(const OMPOrderedDirective 
> > > &S) {...}
> > > 
> > > static bool isSupportedByOpenMPIRBuilder(const OMPTaskDirective &S) {...}
> > > void CodeGenFunction::EmitOMPOrderedDirective(const OMPTaskDirective &S) 
> > > {...}
> > > ```
> > It was decided in D114379 to use `OMPExecutableDirective` in order to allow 
> > this function to be reused for constructs such as `for simd`. Do you wish 
> > to undo this now, and instead specialize the function?
> I wasn't aware of the earlier discussion about this. However it seems like it 
> was suggested to try either splitting the function or merging it with slight 
> preference for merging (I didn't understand how that would help constructs 
> like `for simd` because I haven't worked with `for simd` much and will take 
> others' word for it).
> 
> The suggested code change in previous reply - checking of clauses with a 
> check for OMPSimdDirective - this seems like it would eventually become a 
> huge function because of such checks. 
> 
> @Meinersbur it would be great if you could please advise on this - if keeping 
> the checks for all executable directives in the same function will be helpful 
> in the long run - allowing us to reuse checks or should we split it? A third 
> alternative would be to have both functions, one specialized for SIMD and one 
> for ExecutableConstruct, where the latter might handle combined constructs 
> like `for simd` etc. Would that arrangement work?
I agree with you that having a general function for `OMPExecutableDirective` 
will probably result in a very large, bloated function. I have tentatively just 
specialized the function for `OMPSimdDirective`.


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

https://reviews.llvm.org/D129149

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

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:190
 : DiagGroup<"deprecated-dynamic-exception-spec">;
+def DeprecatedHasBuiltins : DiagGroup<"deprecated-has-builtins">;
 def DeprecatedImplementations :DiagGroup<"deprecated-implementations">;

I wonder if we want to rename this to `deprecated-builtins` so it applies to 
any builtin we elect to deprecate, not just ones that start with `has`. WDYT?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5561
+def warn_deprecated_has_builtins : Warning<
+  "the %0 compiler builtin is deprecated from C++11 onwards. Use %1 instead.">,
+  InGroup;

erichkeane wrote:
> cor3ntin wrote:
> > Should we say something like "and will be removed in the future"?
> > 
> > "%0 is deprecated from C++11 onward. Use %1 instead." might be sufficient
> > 
> > 
> Diagnostics arent sentences. Also, we wouldn't say "C++11 onward", we can 
> just say C++11. I might suggest:
> 
> `builtin %0 is deprecated in C++11, use %1 instead`
> 
> BUT @aaron.ballman always does great at this level of bikeshedding.
> 
I think we might want to rename this to `warn_deprecated_builtin` and make it 
slightly more general. I think we want to drop the mention of C++11 because the 
documentation says these are deprecated (not deprecated in a specific version) 
and the replacement APIs are all available pre C++11 anyway (at least in 
Clang). How about: `builtin %0 is deprecated; use %1 instead`?



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5400-5401
+SourceLocation KWLoc) {
+  if (!S.getLangOpts().CPlusPlus11)
+return;
+

I think we should always warn on these, not just in C++11.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5426-5428
+case UTT_HasTrivialDefaultConstructor:
+  Replacement = TT_IsTriviallyConstructible;
+  break;

This one is not documented as being deprecated (or documented at all). I think 
it's reasonable to deprecate it, but it should probably be documented in the 
language extensions page.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5433
+// FIXME: GCC don't implement __is_trivially_destructible: Warning for this
+//   might be too noisy for now.
+// case UTT_HasTrivialDestructor:

I'd like to know how plausible that "might" is -- Clang flags it as being 
deprecated, so it seems very reasonable to diagnose it as such. These 
diagnostics won't be triggered from system headers by default anyway, so I'm 
not certain if this will be too noisy in practice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129170

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

[PATCH] D108469: Improve handling of static assert messages.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: llvm/lib/Support/Unicode.cpp:272
+/// Unicode code points of the Cf category are considered
+/// fornatting characters.
+bool isFormatting(int UCS) {

tahonermann wrote:
> 
Thanks! I made a commit to fix it :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108469

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

[PATCH] D129202: [clang] Add a fixit for warn-self-assign if LHS is a field with the same name as parameter on RHS

[Nathan James via Phabricator via cfe-commits]

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

Add a fix-it for the common case of setters/constructors using parameters with 
the same name as fields

  struct A{
int X;
A(int X) { /*this->*/X = X; }
void setX(int X) { /*this->*/X = X;
  };


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129202

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-self-assign-builtin.cpp


Index: clang/test/SemaCXX/warn-self-assign-builtin.cpp
===
--- clang/test/SemaCXX/warn-self-assign-builtin.cpp
+++ clang/test/SemaCXX/warn-self-assign-builtin.cpp
@@ -65,3 +65,23 @@
   g();
   g();
 }
+
+struct Foo {
+  int A;
+  void setA(int A) {
+A = A; // expected-warning{{explicitly assigning}} expected-note {{did you 
mean to assign to member 'A'}}
+  }
+
+  void setThroughLambda() {
+[](int A) {
+  // To fix here we would need to insert an explicit capture 'this'
+  A = A; // expected-warning{{explicitly assigning}}
+}(5);
+
+[this](int A) {
+  this->A = 0;
+  // This fix would be possible by just adding this-> as above, but 
currently unsupported.
+  A = A; // expected-warning{{explicitly assigning}}
+}(5);
+  }
+};
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -54,6 +54,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/TypeSize.h"
 
@@ -14634,6 +14635,35 @@
   : diag::warn_self_assignment_overloaded)
   << LHSDeclRef->getType() << LHSExpr->getSourceRange()
   << RHSExpr->getSourceRange();
+
+  // Explore the case for adding 'this->' to the LHS, very common for setters
+  // struct A { int X;
+  // -void setX(int X) { X = X; }
+  // +void setX(int X) { this->X = X; }
+  // };
+
+  // Only consider parameter's for this fix.
+  if (!llvm::isa(RHSDecl))
+return;
+  if (auto *Method =
+  llvm::dyn_cast_or_null(S.getCurFunctionDecl(true))) {
+
+CXXRecordDecl *Parent = Method->getParent();
+// In theory this is fixable if the lambda explicitly captures this, but
+// that's added complexity that's rarely going to be used.
+if (Parent->isLambda())
+  return;
+
+// Only check the fields declared in Parent, without traversing base 
classes
+auto Field = llvm::find_if(
+Parent->fields(), [Name(RHSDecl->getDeclName())](const FieldDecl *F) {
+  return F->getDeclName() == Name;
+});
+if (Field != Parent->fields().end())
+  S.Diag(LHSDeclRef->getBeginLoc(), diag::note_self_assign_insert_this)
+  << *Field
+  << FixItHint::CreateInsertion(LHSDeclRef->getBeginLoc(), "this->");
+  }
 }
 
 /// Check if a bitwise-& is performed on an Objective-C pointer.  This
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6613,6 +6613,8 @@
 def warn_self_move : Warning<
   "explicitly moving variable of type %0 to itself">,
   InGroup, DefaultIgnore;
+def note_self_assign_insert_this : Note<
+  "did you mean to assign to member %0">;
 
 def err_builtin_move_forward_unsupported : Error<
   "unsupported signature for %q0">;


Index: clang/test/SemaCXX/warn-self-assign-builtin.cpp
===
--- clang/test/SemaCXX/warn-self-assign-builtin.cpp
+++ clang/test/SemaCXX/warn-self-assign-builtin.cpp
@@ -65,3 +65,23 @@
   g();
   g();
 }
+
+struct Foo {
+  int A;
+  void setA(int A) {
+A = A; // expected-warning{{explicitly assigning}} expected-note {{did you mean to assign to member 'A'}}
+  }
+
+  void setThroughLambda() {
+[](int A) {
+  // To fix here we would need to insert an explicit capture 'this'
+  A = A; // expected-warning{{explicitly assigning}}
+}(5);
+
+[this](int A) {
+  this->A = 0;
+  // This fix would be possible by just adding this-> as above, but currently unsupported.
+  A = A; // expected-warning{{explicitly assigning}}
+}(5);
+  }
+};
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -54,6 +54,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/SaveAndRes

[PATCH] D129202: [clang] Add a fixit for warn-self-assign if LHS is a field with the same name as parameter on RHS

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 442559.
njames93 added a comment.

Remove unnecessary extra include.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129202

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-self-assign-builtin.cpp


Index: clang/test/SemaCXX/warn-self-assign-builtin.cpp
===
--- clang/test/SemaCXX/warn-self-assign-builtin.cpp
+++ clang/test/SemaCXX/warn-self-assign-builtin.cpp
@@ -65,3 +65,23 @@
   g();
   g();
 }
+
+struct Foo {
+  int A;
+  void setA(int A) {
+A = A; // expected-warning{{explicitly assigning}} expected-note {{did you 
mean to assign to member 'A'}}
+  }
+
+  void setThroughLambda() {
+[](int A) {
+  // To fix here we would need to insert an explicit capture 'this'
+  A = A; // expected-warning{{explicitly assigning}}
+}(5);
+
+[this](int A) {
+  this->A = 0;
+  // This fix would be possible by just adding this-> as above, but 
currently unsupported.
+  A = A; // expected-warning{{explicitly assigning}}
+}(5);
+  }
+};
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14634,6 +14634,35 @@
   : diag::warn_self_assignment_overloaded)
   << LHSDeclRef->getType() << LHSExpr->getSourceRange()
   << RHSExpr->getSourceRange();
+
+  // Explore the case for adding 'this->' to the LHS, very common for setters
+  // struct A { int X;
+  // -void setX(int X) { X = X; }
+  // +void setX(int X) { this->X = X; }
+  // };
+
+  // Only consider parameter's for this fix.
+  if (!llvm::isa(RHSDecl))
+return;
+  if (auto *Method =
+  llvm::dyn_cast_or_null(S.getCurFunctionDecl(true))) {
+
+CXXRecordDecl *Parent = Method->getParent();
+// In theory this is fixable if the lambda explicitly captures this, but
+// that's added complexity that's rarely going to be used.
+if (Parent->isLambda())
+  return;
+
+// Only check the fields declared in Parent, without traversing base 
classes
+auto Field = llvm::find_if(
+Parent->fields(), [Name(RHSDecl->getDeclName())](const FieldDecl *F) {
+  return F->getDeclName() == Name;
+});
+if (Field != Parent->fields().end())
+  S.Diag(LHSDeclRef->getBeginLoc(), diag::note_self_assign_insert_this)
+  << *Field
+  << FixItHint::CreateInsertion(LHSDeclRef->getBeginLoc(), "this->");
+  }
 }
 
 /// Check if a bitwise-& is performed on an Objective-C pointer.  This
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6613,6 +6613,8 @@
 def warn_self_move : Warning<
   "explicitly moving variable of type %0 to itself">,
   InGroup, DefaultIgnore;
+def note_self_assign_insert_this : Note<
+  "did you mean to assign to member %0">;
 
 def err_builtin_move_forward_unsupported : Error<
   "unsupported signature for %q0">;


Index: clang/test/SemaCXX/warn-self-assign-builtin.cpp
===
--- clang/test/SemaCXX/warn-self-assign-builtin.cpp
+++ clang/test/SemaCXX/warn-self-assign-builtin.cpp
@@ -65,3 +65,23 @@
   g();
   g();
 }
+
+struct Foo {
+  int A;
+  void setA(int A) {
+A = A; // expected-warning{{explicitly assigning}} expected-note {{did you mean to assign to member 'A'}}
+  }
+
+  void setThroughLambda() {
+[](int A) {
+  // To fix here we would need to insert an explicit capture 'this'
+  A = A; // expected-warning{{explicitly assigning}}
+}(5);
+
+[this](int A) {
+  this->A = 0;
+  // This fix would be possible by just adding this-> as above, but currently unsupported.
+  A = A; // expected-warning{{explicitly assigning}}
+}(5);
+  }
+};
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14634,6 +14634,35 @@
   : diag::warn_self_assignment_overloaded)
   << LHSDeclRef->getType() << LHSExpr->getSourceRange()
   << RHSExpr->getSourceRange();
+
+  // Explore the case for adding 'this->' to the LHS, very common for setters
+  // struct A { int X;
+  // -void setX(int X) { X = X; }
+  // +void setX(int X) { this->X = X; }
+  // };
+
+  // Only consider parameter's for this fix.
+  if (!llvm::isa(RHSDecl))
+return;
+  if (auto *Method =
+  llvm::dyn_cast_or_null(S.getCurFunctionDecl(true))) {
+
+CXXRecordDecl *Parent = Method->getParent();
+// In theory this is fixable if the lambda explicitly captures this, but
+// that's added 

[clang] 08e4fe6 - [X86] Add RDPRU instruction

[Paul Robinson via cfe-commits]

Author: Paul Robinson
Date: 2022-07-06T07:17:47-07:00
New Revision: 08e4fe6c61967d5c6c16ef7a4cc63d51c4992b55

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

LOG: [X86] Add RDPRU instruction

Add support for the RDPRU instruction on Zen2 processors.

User-facing features:

- Clang option -m[no-]rdpru to enable/disable the feature
- Support is implicit for znver2/znver3 processors
- Preprocessor symbol __RDPRU__ to indicate support
- Header rdpruintrin.h to define intrinsics
- "rdpru" mnemonic supported for assembler code

Internal features:

- Clang builtin __builtin_ia32_rdpru
- IR intrinsic @llvm.x86.rdpru

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

Added: 
clang/lib/Headers/rdpruintrin.h
clang/test/CodeGen/rdpru-builtins.c
llvm/test/CodeGen/X86/rdpru.ll
llvm/test/MC/X86/RDPRU.s

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/BuiltinsX86.def
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/X86.cpp
clang/lib/Basic/Targets/X86.h
clang/lib/Headers/CMakeLists.txt
clang/lib/Headers/x86intrin.h
clang/test/Driver/x86-target-features.c
clang/test/Preprocessor/x86_target_features.c
llvm/docs/ReleaseNotes.rst
llvm/include/llvm/IR/IntrinsicsX86.td
llvm/include/llvm/Support/X86TargetParser.def
llvm/lib/Support/X86TargetParser.cpp
llvm/lib/Target/X86/X86.td
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86InstrInfo.td
llvm/lib/Target/X86/X86InstrSystem.td
llvm/lib/Target/X86/X86IntrinsicsInfo.h
llvm/test/MC/Disassembler/X86/x86-32.txt
llvm/test/MC/Disassembler/X86/x86-64.txt

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 98153fd6e68d..0f542e08b841 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -527,6 +527,9 @@ X86 Support in Clang
 - Support for the ``_Float16`` type has been added for all targets with SSE2.
   When AVX512-FP16 is not available, arithmetic on ``_Float16`` is emulated
   using ``float``.
+- Added the ``-m[no-]rdpru`` flag to enable/disable the RDPRU instruction
+  provided by AMD Zen2 and later processors. Defined intrinsics for using
+  this instruction (see rdpruintrin.h).
 
 DWARF Support in Clang
 --

diff  --git a/clang/include/clang/Basic/BuiltinsX86.def 
b/clang/include/clang/Basic/BuiltinsX86.def
index 3e5c376f9bc1..6bf35c340c2d 100644
--- a/clang/include/clang/Basic/BuiltinsX86.def
+++ b/clang/include/clang/Basic/BuiltinsX86.def
@@ -825,6 +825,7 @@ BUILTIN(__rdtsc, "UOi", "")
 BUILTIN(__builtin_ia32_rdtscp, "UOiUi*", "")
 
 TARGET_BUILTIN(__builtin_ia32_rdpid, "Ui", "n", "rdpid")
+TARGET_BUILTIN(__builtin_ia32_rdpru, "ULLii", "n", "rdpru")
 
 // PKU
 TARGET_BUILTIN(__builtin_ia32_rdpkru, "Ui", "n", "pku")

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index f4fe08aa1a5b..8ae9145a271a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4570,6 +4570,8 @@ def mptwrite : Flag<["-"], "mptwrite">, 
Group;
 def mno_ptwrite : Flag<["-"], "mno-ptwrite">, Group;
 def mrdpid : Flag<["-"], "mrdpid">, Group;
 def mno_rdpid : Flag<["-"], "mno-rdpid">, Group;
+def mrdpru : Flag<["-"], "mrdpru">, Group;
+def mno_rdpru : Flag<["-"], "mno-rdpru">, Group;
 def mrdrnd : Flag<["-"], "mrdrnd">, Group;
 def mno_rdrnd : Flag<["-"], "mno-rdrnd">, Group;
 def mrtm : Flag<["-"], "mrtm">, Group;

diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 06988830eaed..69afdf8a3584 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -297,6 +297,8 @@ bool 
X86TargetInfo::handleTargetFeatures(std::vector &Features,
   HasCLDEMOTE = true;
 } else if (Feature == "+rdpid") {
   HasRDPID = true;
+} else if (Feature == "+rdpru") {
+  HasRDPRU = true;
 } else if (Feature == "+kl") {
   HasKL = true;
 } else if (Feature == "+widekl") {
@@ -743,6 +745,8 @@ void X86TargetInfo::getTargetDefines(const LangOptions 
&Opts,
 Builder.defineMacro("__WIDEKL__");
   if (HasRDPID)
 Builder.defineMacro("__RDPID__");
+  if (HasRDPRU)
+Builder.defineMacro("__RDPRU__");
   if (HasCLDEMOTE)
 Builder.defineMacro("__CLDEMOTE__");
   if (HasWAITPKG)
@@ -926,6 +930,7 @@ bool X86TargetInfo::isValidFeatureName(StringRef Name) 
const {
   .Case("prfchw", true)
   .Case("ptwrite", true)
   .Case("rdpid", true)
+  .Case("rdpru", true)
   .Case("rdrnd", true)
   .Case("rdseed", true)
   .Case("rtm", true)
@@ -1021,6 +1026,7 @@ bool X86TargetInfo::hasFeature(StringRef Feature) const {
   .Case("prfchw", HasPRFCHW)
   .Case("ptwrite", HasPTWRITE)
   .

[PATCH] D128934: [X86] Add RDPRU instruction

[Paul Robinson 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 rG08e4fe6c6196: [X86] Add RDPRU instruction (authored by 
probinson).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D128934?vs=442377&id=442562#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128934

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/BuiltinsX86.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/rdpruintrin.h
  clang/lib/Headers/x86intrin.h
  clang/test/CodeGen/rdpru-builtins.c
  clang/test/Driver/x86-target-features.c
  clang/test/Preprocessor/x86_target_features.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86InstrSystem.td
  llvm/lib/Target/X86/X86IntrinsicsInfo.h
  llvm/test/CodeGen/X86/rdpru.ll
  llvm/test/MC/Disassembler/X86/x86-32.txt
  llvm/test/MC/Disassembler/X86/x86-64.txt
  llvm/test/MC/X86/RDPRU.s

Index: llvm/test/MC/X86/RDPRU.s
===
--- /dev/null
+++ llvm/test/MC/X86/RDPRU.s
@@ -0,0 +1,17 @@
+/// Encoding and disassembly of rdpru.
+
+// RUN: llvm-mc -triple i686-- --show-encoding %s |\
+// RUN:   FileCheck %s --check-prefixes=CHECK,ENCODING
+
+// RUN: llvm-mc -triple i686-- -filetype=obj %s |\
+// RUN:   llvm-objdump -d - | FileCheck %s
+
+// RUN: llvm-mc -triple x86_64-- --show-encoding %s |\
+// RUN:   FileCheck %s --check-prefixes=CHECK,ENCODING
+
+// RUN: llvm-mc -triple x86_64-- -filetype=obj %s |\
+// RUN:   llvm-objdump -d - | FileCheck %s
+
+// CHECK: rdpru
+// ENCODING:  encoding: [0x0f,0x01,0xfd]
+rdpru
Index: llvm/test/MC/Disassembler/X86/x86-64.txt
===
--- llvm/test/MC/Disassembler/X86/x86-64.txt
+++ llvm/test/MC/Disassembler/X86/x86-64.txt
@@ -758,3 +758,6 @@
 
 # CHECK: senduipi %r13
 0xf3,0x41,0x0f,0xc7,0xf5
+
+# CHECK: rdpru
+0x0f,0x01,0xfd
Index: llvm/test/MC/Disassembler/X86/x86-32.txt
===
--- llvm/test/MC/Disassembler/X86/x86-32.txt
+++ llvm/test/MC/Disassembler/X86/x86-32.txt
@@ -1015,3 +1015,6 @@
 
 # CHECK: hreset $1
 0xf3 0x0f 0x3a 0xf0 0xc0 0x01
+
+# CHECK: rdpru
+0x0f,0x01,0xfd
Index: llvm/test/CodeGen/X86/rdpru.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/rdpru.ll
@@ -0,0 +1,85 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=i686-- -mattr=+rdpru | FileCheck %s --check-prefix=X86
+; RUN: llc < %s -mtriple=i686-- -mattr=+rdpru -fast-isel | FileCheck %s --check-prefix=X86
+; RUN: llc < %s -mtriple=x86_64-- -mattr=+rdpru | FileCheck %s --check-prefix=X64
+; RUN: llc < %s -mtriple=x86_64-- -mattr=+rdpru -fast-isel | FileCheck %s --check-prefix=X64
+; RUN: llc < %s -mtriple=x86_64-- -mcpu=znver2 | FileCheck %s --check-prefix=X64
+; RUN: llc < %s -mtriple=x86_64-- -mcpu=znver3 -fast-isel | FileCheck %s --check-prefix=X64
+
+define void @rdpru_asm() {
+; X86-LABEL: rdpru_asm:
+; X86:   # %bb.0: # %entry
+; X86-NEXT:#APP
+; X86-NEXT:rdpru
+; X86-NEXT:#NO_APP
+; X86-NEXT:retl
+;
+; X64-LABEL: rdpru_asm:
+; X64:   # %bb.0: # %entry
+; X64-NEXT:#APP
+; X64-NEXT:rdpru
+; X64-NEXT:#NO_APP
+; X64-NEXT:retq
+entry:
+  call void asm sideeffect "rdpru", "~{dirflag},~{fpsr},~{flags}"()
+  ret void
+}
+
+define i64 @rdpru_param(i32 %regid) local_unnamed_addr {
+; X86-LABEL: rdpru_param:
+; X86:   # %bb.0: # %entry
+; X86-NEXT:movl {{[0-9]+}}(%esp), %ecx
+; X86-NEXT:rdpru
+; X86-NEXT:retl
+;
+; X64-LABEL: rdpru_param:
+; X64:   # %bb.0: # %entry
+; X64-NEXT:movl %edi, %ecx
+; X64-NEXT:rdpru
+; X64-NEXT:shlq $32, %rdx
+; X64-NEXT:orq %rdx, %rax
+; X64-NEXT:retq
+entry:
+  %0 = tail call i64 @llvm.x86.rdpru(i32 %regid)
+  ret i64 %0
+}
+
+define i64 @rdpru_mperf() local_unnamed_addr {
+; X86-LABEL: rdpru_mperf:
+; X86:   # %bb.0: # %entry
+; X86-NEXT:xorl %ecx, %ecx
+; X86-NEXT:rdpru
+; X86-NEXT:retl
+;
+; X64-LABEL: rdpru_mperf:
+; X64:   # %bb.0: # %entry
+; X64-NEXT:xorl %ecx, %ecx
+; X64-NEXT:rdpru
+; X64-NEXT:shlq $32, %rdx
+; X64-NEXT:orq %rdx, %rax
+; X64-NEXT:retq
+entry:
+  %0 = tail call i64 @llvm.x86.rdpru(i32 0)
+  ret i64 %0
+}
+
+define i64 @rdpru_aperf() local_unnamed_addr {
+; X86-LABEL: rdpru_aperf:
+; X86:   # %bb.0: # %entry
+; X86-NEXT:movl $1

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[Jean Perier via Phabricator via cfe-commits]

jeanPerier added a comment.

In D107082#3632301 , @pengfei wrote:

> Hi @jeanPerier , yes, you are right. This patch changes the calling 
> conversion of fp16 from GPRs to XMMs. So you need to update the runtime. If 
> you are using compiler-rt, you could simply re-build it with trunk code, or 
> at least after rGabeeae57 
> . If you 
> are using your own runtime, you can solve the problem through the way in 
> https://github.com/llvm/llvm-project/issues/56156

Thanks for the quick reply.  I was using a compiler-rt from the trunk source 
but not building it with a clang compiler compiled from the trunk. I did not 
know the version of clang used to compiled compiler-rt mattered that much. 
Using clang from the trunk (or at least after the commit you mentionnned) 
solved my problem. Thanks !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107082

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

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

Nit: Also add to the summary that this patch uses the simdlen support in 
OpenMPIRBuilder when it is enabled in Clang.




Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2600
+  continue;
+else
+  return false;

Nit: Else after return/continue is discouraged.
https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return


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

https://reviews.llvm.org/D129149

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

[PATCH] D128745: [Sema] fix trailing parameter pack handling for function template partial ordering

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: erichkeane, clang-language-wg.
aaron.ballman added a comment.

Thank you for working on this! Can you please add more details to the patch 
summary about the changes?




Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5184
 
-  // FIXME: This mimics what GCC implements, but doesn't match up with the
-  // proposed resolution for core issue 692. This area needs to be sorted out,

aaron.ballman wrote:
> We tend not to use top-level const on locals in the project as a matter of 
> style.
Does GCC still implement it this way?

One concern I have is that this will be an ABI breaking change, and I'm not 
certain how disruptive it will be. If GCC already made that change, it may be 
reasonable for us to also break it without having to add ABI tags or something 
special. But if these changes diverge us from GCC, that may require some extra 
effort to mitigate.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5184-5185
 
-  // FIXME: This mimics what GCC implements, but doesn't match up with the
-  // proposed resolution for core issue 692. This area needs to be sorted out,
-  // but for now we attempt to maintain compatibility.
+  const unsigned NumParams1 = FT1->getTemplatedDecl()->getNumParams();
+  const unsigned NumParams2 = FT2->getTemplatedDecl()->getNumParams();
+

We tend not to use top-level const on locals in the project as a matter of 
style.



Comment at: clang/test/CXX/drs/dr6xx.cpp:1084
 
 namespace dr692 { // dr692: no
   namespace temp_func_order_example2 {

This DR is also about partial class specializations -- are changes not needed 
in `Sema::getMoreSpecializedPartialSpecialization()` as well?

It'd be helpful to add the examples from the DR here to make sure they're 
handled properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128745

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

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.
Herald added a subscriber: mattd.

reverse ping. Are there outstanding issues with this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Prabhdeep Soni via Phabricator via cfe-commits]

psoni2628 added a comment.

In D129149#3632813 , 
@kiranchandramohan wrote:

> Nit: Also add to the summary that this patch uses the simdlen support in 
> OpenMPIRBuilder when it is enabled in Clang.

I just wanted to clarify that below is what you mean?

  This patch adds OMPIRBuilder support for the simdlen clause for the
  simd directive. It uses the simdlen support in OpenMPIRBuilder when
  it is enabled in clang.


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

https://reviews.llvm.org/D129149

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

[PATCH] D127042: [Clang][OpenMP] Enable floating-point operation for `atomic compare` series

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 442585.
tianshilei1992 added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127042

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_compare_codegen.cpp
  clang/test/OpenMP/atomic_messages.c
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp

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

[clang] b5b6d3a - [Debugify] Port verify-debuginfo-preserve to NewPM

[Djordje Todorovic via cfe-commits]

Author: Nikola Tesic
Date: 2022-07-06T17:07:20+02:00
New Revision: b5b6d3a41b4eba23b604f37942b892a382caae57

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

LOG: [Debugify] Port verify-debuginfo-preserve to NewPM

Debugify in OriginalDebugInfo mode, introduced with D82545,
runs only with legacy PassManager.

This patch enables this utility for the NewPM.

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

Added: 
llvm/test/DebugInfo/verify-di-preserve.ll

Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/include/llvm/Transforms/Utils/Debugify.h
llvm/lib/Transforms/Utils/Debugify.cpp
llvm/test/DebugInfo/debugify-original-no-dbg-info.ll
llvm/test/Transforms/Util/Debugify/loc-only-original-mode.ll
llvm/tools/opt/NewPMDriver.cpp
llvm/tools/opt/NewPMDriver.h
llvm/tools/opt/opt.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index eb40e446057f9..7c4e35634e5dc 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -788,6 +788,18 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   SI.registerCallbacks(PIC, &FAM);
   PassBuilder PB(TM.get(), PTO, PGOOpt, &PIC);
 
+  // Enable verify-debuginfo-preserve-each for new PM.
+  DebugifyEachInstrumentation Debugify;
+  DebugInfoPerPass DebugInfoBeforePass;
+  if (CodeGenOpts.EnableDIPreservationVerify) {
+Debugify.setDebugifyMode(DebugifyMode::OriginalDebugInfo);
+Debugify.setDebugInfoBeforePass(DebugInfoBeforePass);
+
+if (!CodeGenOpts.DIBugsReportFilePath.empty())
+  Debugify.setOrigDIVerifyBugsReportFilePath(
+  CodeGenOpts.DIBugsReportFilePath);
+Debugify.registerCallbacks(PIC);
+  }
   // Attempt to load pass plugins and register their callbacks with PB.
   for (auto &PluginFN : CodeGenOpts.PassPlugins) {
 auto PassPlugin = PassPlugin::Load(PluginFN);

diff  --git a/llvm/include/llvm/Transforms/Utils/Debugify.h 
b/llvm/include/llvm/Transforms/Utils/Debugify.h
index 405bbb8e0be8a..24b9eeab6ee45 100644
--- a/llvm/include/llvm/Transforms/Utils/Debugify.h
+++ b/llvm/include/llvm/Transforms/Utils/Debugify.h
@@ -101,7 +101,18 @@ llvm::FunctionPass *createDebugifyFunctionPass(
 llvm::StringRef NameOfWrappedPass = "",
 DebugInfoPerPass *DebugInfoBeforePass = nullptr);
 
-struct NewPMDebugifyPass : public llvm::PassInfoMixin {
+class NewPMDebugifyPass : public llvm::PassInfoMixin {
+  llvm::StringRef NameOfWrappedPass;
+  DebugInfoPerPass *DebugInfoBeforePass = nullptr;
+  enum DebugifyMode Mode = DebugifyMode::NoDebugify;
+public:
+  NewPMDebugifyPass(
+  enum DebugifyMode Mode = DebugifyMode::SyntheticDebugInfo,
+  llvm::StringRef NameOfWrappedPass = "",
+  DebugInfoPerPass *DebugInfoBeforePass = nullptr)
+  : NameOfWrappedPass(NameOfWrappedPass),
+DebugInfoBeforePass(DebugInfoBeforePass), Mode(Mode) {}
+
   llvm::PreservedAnalyses run(llvm::Module &M, llvm::ModuleAnalysisManager 
&AM);
 };
 
@@ -148,18 +159,65 @@ llvm::FunctionPass *createCheckDebugifyFunctionPass(
 DebugInfoPerPass *DebugInfoBeforePass = nullptr,
 llvm::StringRef OrigDIVerifyBugsReportFilePath = "");
 
-struct NewPMCheckDebugifyPass
+class NewPMCheckDebugifyPass
 : public llvm::PassInfoMixin {
+  llvm::StringRef NameOfWrappedPass;
+  llvm::StringRef OrigDIVerifyBugsReportFilePath;
+  DebugifyStatsMap *StatsMap;
+  DebugInfoPerPass *DebugInfoBeforePass;
+  enum DebugifyMode Mode;
+  bool Strip;
+public:
+  NewPMCheckDebugifyPass(
+  bool Strip = false, llvm::StringRef NameOfWrappedPass = "",
+  DebugifyStatsMap *StatsMap = nullptr,
+  enum DebugifyMode Mode = DebugifyMode::SyntheticDebugInfo,
+  DebugInfoPerPass *DebugInfoBeforePass = nullptr,
+  llvm::StringRef OrigDIVerifyBugsReportFilePath = "")
+  : NameOfWrappedPass(NameOfWrappedPass),
+OrigDIVerifyBugsReportFilePath(OrigDIVerifyBugsReportFilePath),
+StatsMap(StatsMap), DebugInfoBeforePass(DebugInfoBeforePass), 
Mode(Mode),
+Strip(Strip) {}
+
   llvm::PreservedAnalyses run(llvm::Module &M, llvm::ModuleAnalysisManager 
&AM);
 };
 
 namespace llvm {
 void exportDebugifyStats(StringRef Path, const DebugifyStatsMap &Map);
 
-struct DebugifyEachInstrumentation {
-  DebugifyStatsMap StatsMap;
+class DebugifyEachInstrumentation {
+  llvm::StringRef OrigDIVerifyBugsReportFilePath = "";
+  DebugInfoPerPass *DebugInfoBeforePass = nullptr;
+  enum DebugifyMode Mode = DebugifyMode::NoDebugify;
+  DebugifyStatsMap *DIStatsMap = nullptr;
+
+public:
 
   void registerCallbacks(PassInstrumentationCallbacks &PIC);
+  // Used within DebugifyMode::SyntheticDebugInfo mode.
+  void setDIStatsMap(DebugifyStatsMap &StatMap) { DIStatsMap = &StatMap; }
+  const DebugifyStatsMap &getDebugifyStatsMap(

[PATCH] D115351: [Debugify] Port verify-debuginfo-preserve to NewPM

[Djordje Todorovic 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 rGb5b6d3a41b4e: [Debugify] Port verify-debuginfo-preserve to 
NewPM (authored by ntesic, committed by djtodoro).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115351

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Transforms/Utils/Debugify.h
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/test/DebugInfo/debugify-original-no-dbg-info.ll
  llvm/test/DebugInfo/verify-di-preserve.ll
  llvm/test/Transforms/Util/Debugify/loc-only-original-mode.ll
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/NewPMDriver.h
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -206,18 +206,6 @@
 cl::desc("Start the pipeline with collecting and end it with checking of "
  "debug info preservation."));
 
-static cl::opt VerifyEachDebugInfoPreserve(
-"verify-each-debuginfo-preserve",
-cl::desc("Start each pass with collecting and end it with checking of "
- "debug info preservation."));
-
-static cl::opt
-VerifyDIPreserveExport("verify-di-preserve-export",
-   cl::desc("Export debug info preservation failures into "
-"specified (JSON) file (should be abs path as we use"
-" append mode to insert new JSON objects)"),
-   cl::value_desc("filename"), cl::init(""));
-
 static cl::opt
 PrintBreakpoints("print-breakpoints-for-testing",
  cl::desc("Print select breakpoints location for testing"));
@@ -823,7 +811,8 @@
ThinLinkOut.get(), RemarksFile.get(), Pipeline,
Passes, PluginList, OK, VK, PreserveAssemblyUseListOrder,
PreserveBitcodeUseListOrder, EmitSummaryIndex,
-   EmitModuleHash, EnableDebugify)
+   EmitModuleHash, EnableDebugify,
+   VerifyDebugInfoPreserve)
? 0
: 1;
   }
Index: llvm/tools/opt/NewPMDriver.h
===
--- llvm/tools/opt/NewPMDriver.h
+++ llvm/tools/opt/NewPMDriver.h
@@ -33,6 +33,9 @@
 extern cl::opt DebugifyEach;
 extern cl::opt DebugifyExport;
 
+extern cl::opt VerifyEachDebugInfoPreserve;
+extern cl::opt VerifyDIPreserveExport;
+
 namespace opt_tool {
 enum OutputKind {
   OK_NoOutput,
@@ -74,7 +77,7 @@
  bool ShouldPreserveAssemblyUseListOrder,
  bool ShouldPreserveBitcodeUseListOrder,
  bool EmitSummaryIndex, bool EmitModuleHash,
- bool EnableDebugify);
+ bool EnableDebugify, bool VerifyDIPreserve);
 } // namespace llvm
 
 #endif
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -49,6 +49,19 @@
 DebugifyExport("debugify-export",
cl::desc("Export per-pass debugify statistics to this file"),
cl::value_desc("filename"));
+
+cl::opt VerifyEachDebugInfoPreserve(
+"verify-each-debuginfo-preserve",
+cl::desc("Start each pass with collecting and end it with checking of "
+ "debug info preservation."));
+
+cl::opt
+VerifyDIPreserveExport("verify-di-preserve-export",
+   cl::desc("Export debug info preservation failures into "
+"specified (JSON) file (should be abs path as we use"
+" append mode to insert new JSON objects)"),
+   cl::value_desc("filename"), cl::init(""));
+
 } // namespace llvm
 
 enum class DebugLogging { None, Normal, Verbose, Quiet };
@@ -280,7 +293,7 @@
bool ShouldPreserveAssemblyUseListOrder,
bool ShouldPreserveBitcodeUseListOrder,
bool EmitSummaryIndex, bool EmitModuleHash,
-   bool EnableDebugify) {
+   bool EnableDebugify, bool VerifyDIPreserve) {
   bool VerifyEachPass = VK == VK_VerifyEachPass;
 
   Optional P;
@@ -337,8 +350,19 @@
   PrintPassOpts);
   SI.registerCallbacks(PIC, &FAM);
   DebugifyEachInstrumentation Debugify;
-  if (DebugifyEach)
+  DebugifyStatsMap DIStatsMap;
+  DebugInfoPerPass DebugInfoBeforePass;
+  if (DebugifyEach) {
+Debugify.setDIStatsMap(DIStatsMap);
+Debugify.setDebugifyMode(DebugifyMode::SyntheticDebugInfo);
+Debugify.registerCallbacks(PIC);
+  } else if (VerifyEachDebugInfoPreserve) {
+Debugi

[PATCH] D127042: [Clang][OpenMP] Enable floating-point operation for `atomic compare` series

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 442588.
tianshilei1992 added a comment.

remove unrelated changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127042

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_compare_codegen.cpp
  clang/test/OpenMP/atomic_messages.c
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

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

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

In D129149#3632861 , @psoni2628 wrote:

> In D129149#3632813 , 
> @kiranchandramohan wrote:
>
>> Nit: Also add to the summary that this patch uses the simdlen support in 
>> OpenMPIRBuilder when it is enabled in Clang.
>
> I just wanted to clarify that below is what you mean?
>
>   This patch adds OMPIRBuilder support for the simdlen clause for the
>   simd directive. It uses the simdlen support in OpenMPIRBuilder when
>   it is enabled in clang.

Yes. Thanks.

Maybe you can also add that simdlen is lowered by the OpenMPIRBuilder by 
generating the loop.vectorize.width metadata.


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

https://reviews.llvm.org/D129149

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

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

Thanks for confirming it! I don't have much experience in compiler-rt. But I 
think the version of clang matters much to compiler-rt particular in ABI 
changing cases like this :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107082

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

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 442592.
serge-sans-paille added a comment.
Herald added a reviewer: NoQ.

Do *not* try to syndicate code across different location. Alhtough that would 
be benefitial to the project, it also makes this patch too complex and 
controversial because it would imply regression on some behaviors.

Instead, focus on compatibility with existing tests, and refine existing 
behavior wrt. bound size.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/CodeGen/bounds-checking-fam.c
  clang/test/CodeGen/bounds-checking-fam.cpp
  clang/test/CodeGen/object-size-flex-array.c
  clang/test/CodeGenObjC/ubsan-array-bounds.m
  clang/test/Sema/array-bounds-ptr-arith.c
  clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
  clang/test/SemaCXX/array-bounds.cpp

Index: clang/test/SemaCXX/array-bounds.cpp
===
--- clang/test/SemaCXX/array-bounds.cpp
+++ clang/test/SemaCXX/array-bounds.cpp
@@ -208,12 +208,14 @@
 
 namespace metaprogramming {
 #define ONE 1
-  struct foo { char c[ONE]; }; // expected-note {{declared here}}
+struct foo {
+  char c[ONE];
+};// expected-note {{declared here}}
   template  struct bar { char c[N]; }; // expected-note {{declared here}}
 
   char test(foo *F, bar<1> *B) {
 return F->c[3] + // expected-warning {{array index 3 is past the end of the array (which contains 1 element)}}
-   B->c[3]; // expected-warning {{array index 3 is past the end of the array (which contains 1 element)}}
+   B->c[3];  // expected-warning {{array index 3 is past the end of the array (which contains 1 element)}}
   }
 }
 
Index: clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -verify -fstrict-flex-arrays=3 %s
+
+// We cannot know for sure the size of a flexible array.
+void test() {
+  struct {
+int f;
+int a[];
+  } s2;
+  s2.a[2] = 0; // no-warning
+}
+
+// Under -fstrict-flex-arrays `a` is not a flexible array.
+void test1() {
+  struct {
+int f;
+int a[1]; // expected-note {{declared here}}
+  } s2;
+  s2.a[2] = 0; // expected-warning 1 {{array index 2 is past the end of the array (which contains 1 element)}}
+}
Index: clang/test/Sema/array-bounds-ptr-arith.c
===
--- clang/test/Sema/array-bounds-ptr-arith.c
+++ clang/test/Sema/array-bounds-ptr-arith.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -verify -Warray-bounds-pointer-arithmetic %s
+// RUN: %clang_cc1 -verify=expected -Warray-bounds-pointer-arithmetic %s
+// RUN: %clang_cc1 -verify=expected -Warray-bounds-pointer-arithmetic %s -fstrict-flex-arrays=0
+// RUN: %clang_cc1 -verify=expected,strict -Warray-bounds-pointer-arithmetic %s -fstrict-flex-arrays=3
 
 // Test case from PR10615
 struct ext2_super_block{
@@ -14,7 +16,9 @@
 }
 
 // Test case reduced from PR11594
-struct S { int n; };
+struct S {
+  int n;
+};
 void pr11594(struct S *s) {
   int a[10];
   int *p = a - s->n;
@@ -26,26 +30,28 @@
 struct RDar11387038 {};
 typedef struct RDar11387038 RDar11387038Array[1];
 struct RDar11387038_Table {
-  RDar11387038Array z;
+  RDar11387038Array z; // strict-note {{array 'z' declared here}}
 };
-typedef struct RDar11387038_Table * TPtr;
+typedef struct RDar11387038_Table *TPtr;
 typedef TPtr *TabHandle;
-struct RDar11387038_B { TabHandle x; };
+struct RDar11387038_B {
+  TabHandle x;
+};
 typedef struct RDar11387038_B RDar11387038_B;
 
 void radar11387038(void) {
   RDar11387038_B *pRDar11387038_B;
-  struct RDar11387038* y = &(*pRDar11387038_B->x)->z[4];
+  struct RDar11387038 *y = &(*pRDar11387038_B->x)->z[4]; // strict-warning {{array index 4 is past the end of the array (which contains 1 element)}}
 }
 
-void pr51682 (void) {
-  int arr [1];
+void pr51682(void) {
+  int arr[1];
   switch (0) {
-case 0:
-  break;
-case 1:
-  asm goto (""::"r"(arr[42] >> 1)::failed); // no-warning
-  break;
+  case 0:
+break;
+  case 1:
+asm goto("" ::"r"(arr[42] >> 1)::failed);
+break;
   }
 failed:;
 }
Index: clang/test/CodeGenObjC/ubsan-array-bounds.m
===
--- clang/test/CodeGenObjC/ubsan-array-bounds.m
+++ clang/test/Code

[PATCH] D129211: [Clang][Doc] Update the release note for clang

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: jdoerfert, ABataev.
Herald added a project: All.
tianshilei1992 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Add the support for `atomic compare` and `atomic compare capture` in the
release note of clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129211

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -511,6 +511,8 @@
 
 OpenMP Support in Clang
 ---
+* Added the support for ``atomic compare`` and ``atomic compare capture``
+  (``-fopenmp-version=51`` is required).
 
 ...
 
@@ -576,7 +578,7 @@
 
 - Added ``forEachTemplateArgument`` matcher which creates a match every
   time a ``templateArgument`` matches the matcher supplied to it.
-  
+
 - Added ``objcStringLiteral`` matcher which matches ObjectiveC String
   literal expressions.
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -511,6 +511,8 @@
 
 OpenMP Support in Clang
 ---
+* Added the support for ``atomic compare`` and ``atomic compare capture``
+  (``-fopenmp-version=51`` is required).
 
 ...
 
@@ -576,7 +578,7 @@
 
 - Added ``forEachTemplateArgument`` matcher which creates a match every
   time a ``templateArgument`` matches the matcher supplied to it.
-  
+
 - Added ``objcStringLiteral`` matcher which matches ObjectiveC String
   literal expressions.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Prabhdeep Soni via Phabricator via cfe-commits]

psoni2628 updated this revision to Diff 442595.
psoni2628 added a comment.

- Remove discouraged else after return


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

https://reviews.llvm.org/D129149

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/irbuilder_simd.cpp
  clang/test/OpenMP/irbuilder_simdlen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  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
@@ -1794,6 +1794,39 @@
   }));
 }
 
+TEST_F(OpenMPIRBuilderTest, ApplySimdlen) {
+  OpenMPIRBuilder OMPBuilder(*M);
+
+  CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
+
+  // Simd-ize the loop.
+  OMPBuilder.applySimd(DL, CLI);
+  OMPBuilder.applySimdlen(DL, CLI, ConstantInt::get(Type::getInt32Ty(Ctx), 3));
+
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+
+  PassBuilder PB;
+  FunctionAnalysisManager FAM;
+  PB.registerFunctionAnalyses(FAM);
+  LoopInfo &LI = FAM.getResult(*F);
+
+  const std::vector &TopLvl = LI.getTopLevelLoops();
+  EXPECT_EQ(TopLvl.size(), 1u);
+
+  Loop *L = TopLvl.front();
+  EXPECT_TRUE(findStringMetadataForLoop(L, "llvm.loop.parallel_accesses"));
+  EXPECT_TRUE(getBooleanLoopAttribute(L, "llvm.loop.vectorize.enable"));
+  EXPECT_EQ(getIntLoopAttribute(L, "llvm.loop.vectorize.width"), 3);
+
+  // Check for llvm.access.group metadata attached to the printf
+  // function in the loop body.
+  BasicBlock *LoopBody = CLI->getBody();
+  EXPECT_TRUE(any_of(*LoopBody, [](Instruction &I) {
+return I.getMetadata("llvm.access.group") != nullptr;
+  }));
+}
+
 TEST_F(OpenMPIRBuilderTest, UnrollLoopFull) {
   OpenMPIRBuilder OMPBuilder(*M);
 
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2881,6 +2881,15 @@
  BoolConst})});
 }
 
+void OpenMPIRBuilder::applySimdlen(DebugLoc, CanonicalLoopInfo *CanonicalLoop,
+   llvm::ConstantInt *Simdlen) {
+  LLVMContext &Ctx = Builder.getContext();
+  addLoopMetadata(
+  CanonicalLoop,
+  MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.width"),
+ConstantAsMetadata::get(Simdlen)}));
+}
+
 /// Create the TargetMachine object to query the backend for optimization
 /// preferences.
 ///
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -603,6 +603,14 @@
   /// \param Loop The loop to simd-ize.
   void applySimd(DebugLoc DL, CanonicalLoopInfo *Loop);
 
+  /// Add metadata for simdlen to a simd loop.
+  ///
+  /// \param DL  Debug location for instructions added by unrolling.
+  /// \param LoopThe simd loop.
+  /// \param Simdlen The Simdlen length to apply to the simd loop.
+  void applySimdlen(DebugLoc DL, CanonicalLoopInfo *Loop,
+llvm::ConstantInt *Simdlen);
+
   /// Generator for '#omp flush'
   ///
   /// \param Loc The location where the flush directive was encountered
Index: clang/test/OpenMP/irbuilder_simdlen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/irbuilder_simdlen.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-enable-irbuilder -verify -fopenmp -fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+struct S {
+  int a, b;
+};
+
+struct P {
+  int a, b;
+};
+
+void simple(float *a, float *b, int *c) {
+  S s, *p;
+  P pp;
+#pragma omp simd simdlen(3)
+  for (int i = 3; i < 32; i += 5) {
+// llvm.access.group test
+// CHECK: %[[A_ADDR:.+]] = alloca float*, align 8
+// CHECK: %[[B_ADDR:.+]] = alloca float*, align 8
+// CHECK: %[[S:.+]] = alloca %struct.S, align 4
+// CHECK: %[[P:.+]] = alloca %struct.S*, align 8
+// CHECK: %[[I:.+]] = alloca i32, align 4
+// CHECK: %[[TMP3:.+]] = load float*, float** %[[B_ADDR:.+]], align 8, !llvm.access.group ![[META3:[0-9]+]]
+// CHECK-NEXT: %[[TMP4:.+]] = load i32, i32* %[[I:.+]], align 4, !llvm.access.group ![[META3:[0-9]+]]
+// CHECK-NEXT: %[[IDXPROM:.+]] = sext i32 %[[TMP4:.+]] to i64
+// CHECK-NEXT: %[[ARRAYIDX:.+]] = getelementptr inbounds float, float* %[[TMP3:.+]], i64 %[[IDXPROM:.+]]
+// CHECK-NEXT: %[[TMP5:.+]] = load float, float* %[[ARRAYIDX:.+]], align 4, !llvm.access.group ![[META3:[0-9]+]]
+// CHECK-NEXT: %[[A2:.+]] = getelementptr inbounds %struct.S, %struct.S* %[[S:.+]], i32 0, i32 0

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

[Aaron Ballman via Phabricator via cfe-commits]

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

The changes here LGTM, thank you for this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128059

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

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Generally, this is fine. Some nits. The only reason not to accept this right 
now is the test. Why manual check lines?
Wrt. the function signature. As soon as we have more then SIMD directives to 
check we refactor things. Keep it simple
until you need the functionality.




Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2598
+// Currently only simdlen clause is supported
+if (!dyn_cast(C))
+  return false;





Comment at: clang/test/OpenMP/irbuilder_simdlen.cpp:1
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-enable-irbuilder -verify 
-fopenmp -fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm 
%s -o - | FileCheck %s
+// expected-no-diagnostics

The check lines look auto-generated and then modified by hand. Why is that?



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:608
+  ///
+  /// \param DL  Debug location for instructions added by unrolling.
+  /// \param LoopThe simd loop.

No debug location needed. You also copied the comment that makes little sene.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2885
+void OpenMPIRBuilder::applySimdlen(DebugLoc, CanonicalLoopInfo *CanonicalLoop,
+   llvm::ConstantInt *Simdlen) {
+  LLVMContext &Ctx = Builder.getContext();

Also above, no llvm::


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

https://reviews.llvm.org/D129149

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

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Prabhdeep Soni via Phabricator via cfe-commits]

psoni2628 added inline comments.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2885
+void OpenMPIRBuilder::applySimdlen(DebugLoc, CanonicalLoopInfo *CanonicalLoop,
+   llvm::ConstantInt *Simdlen) {
+  LLVMContext &Ctx = Builder.getContext();

jdoerfert wrote:
> Also above, no llvm::
It isn't used in the original `applySimd` either. Should I remove it in both 
places?


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

https://reviews.llvm.org/D129149

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

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Prabhdeep Soni via Phabricator via cfe-commits]

psoni2628 added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:608
+  ///
+  /// \param DL  Debug location for instructions added by unrolling.
+  /// \param LoopThe simd loop.

jdoerfert wrote:
> No debug location needed. You also copied the comment that makes little sene.
It isn't used in the original applySimd either. Should I remove it in both 
places?



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2885
+void OpenMPIRBuilder::applySimdlen(DebugLoc, CanonicalLoopInfo *CanonicalLoop,
+   llvm::ConstantInt *Simdlen) {
+  LLVMContext &Ctx = Builder.getContext();

psoni2628 wrote:
> jdoerfert wrote:
> > Also above, no llvm::
> It isn't used in the original `applySimd` either. Should I remove it in both 
> places?
Wrong comment to reply to. Please ignore this.


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

https://reviews.llvm.org/D129149

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

[clang] e3dc568 - [Clang] Add a warning on invalid UTF-8 in comments.

[Corentin Jabot via cfe-commits]

Author: Corentin Jabot
Date: 2022-07-06T17:59:44+02:00
New Revision: e3dc56805f1029dd5959e4c69196a287961afb8d

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

LOG: [Clang] Add a warning on invalid UTF-8 in comments.

Introduce an off-by default `-Winvalid-utf8` warning
that detects invalid UTF-8 code units sequences in comments.

Invalid UTF-8 in other places is already diagnosed,
as that cannot appear in identifiers and other grammar constructs.

The warning is off by default as its likely to be somewhat disruptive
otherwise.

This warning allows clang to conform to the yet-to be approved WG21
"P2295R5 Support for UTF-8 as a portable source file encoding"
paper.

Reviewed By: aaron.ballman, #clang-language-wg

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

Added: 
clang/test/Lexer/comment-invalid-utf8.c

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/lib/Lex/Lexer.cpp
llvm/include/llvm/Support/ConvertUTF.h
llvm/lib/Support/ConvertUTF.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0f542e08b841c..c50e1b89649cf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -279,6 +279,8 @@ Improvements to Clang's diagnostics
   unevaluated operands of a ``typeid`` expression, as they are now
   modeled correctly in the CFG. This fixes
   `Issue 21668 `_.
+- Added ``-Winvalid-utf8`` which diagnoses invalid UTF-8 code unit sequences in
+  comments.
 
 Non-comprehensive list of changes in this release
 -
@@ -576,7 +578,7 @@ AST Matchers
 
 - Added ``forEachTemplateArgument`` matcher which creates a match every
   time a ``templateArgument`` matches the matcher supplied to it.
-  
+
 - Added ``objcStringLiteral`` matcher which matches ObjectiveC String
   literal expressions.
 

diff  --git a/clang/include/clang/Basic/DiagnosticLexKinds.td 
b/clang/include/clang/Basic/DiagnosticLexKinds.td
index ac86076140c58..38ee022e5f04c 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -113,6 +113,8 @@ def warn_four_char_character_literal : Warning<
 // Unicode and UCNs
 def err_invalid_utf8 : Error<
   "source file is not valid UTF-8">;
+def warn_invalid_utf8_in_comment : Extension<
+  "invalid UTF-8 in comment">, InGroup>;
 def err_character_not_allowed : Error<
   "unexpected character ">;
 def err_character_not_allowed_identifier : Error<

diff  --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 6820057642bea..351e518c7ed37 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -2392,13 +2392,37 @@ bool Lexer::SkipLineComment(Token &Result, const char 
*CurPtr,
   //
   // This loop terminates with CurPtr pointing at the newline (or end of 
buffer)
   // character that ends the line comment.
+
+  // C++23 [lex.phases] p1
+  // Diagnose invalid UTF-8 if the corresponding warning is enabled, emitting a
+  // diagnostic only once per entire ill-formed subsequence to avoid
+  // emiting to many diagnostics (see http://unicode.org/review/pr-121.html).
+  bool UnicodeDecodingAlreadyDiagnosed = false;
+
   char C;
   while (true) {
 C = *CurPtr;
 // Skip over characters in the fast loop.
-while (C != 0 &&// Potentially EOF.
-   C != '\n' && C != '\r')  // Newline or DOS-style newline.
+while (isASCII(C) && C != 0 &&   // Potentially EOF.
+   C != '\n' && C != '\r') { // Newline or DOS-style newline.
   C = *++CurPtr;
+  UnicodeDecodingAlreadyDiagnosed = false;
+}
+
+if (!isASCII(C)) {
+  unsigned Length = llvm::getUTF8SequenceSize(
+  (const llvm::UTF8 *)CurPtr, (const llvm::UTF8 *)BufferEnd);
+  if (Length == 0) {
+if (!UnicodeDecodingAlreadyDiagnosed && !isLexingRawMode())
+  Diag(CurPtr, diag::warn_invalid_utf8_in_comment);
+UnicodeDecodingAlreadyDiagnosed = true;
+++CurPtr;
+  } else {
+UnicodeDecodingAlreadyDiagnosed = false;
+CurPtr += Length;
+  }
+  continue;
+}
 
 const char *NextLine = CurPtr;
 if (C != 0) {
@@ -2665,6 +2689,12 @@ bool Lexer::SkipBlockComment(Token &Result, const char 
*CurPtr,
   if (C == '/')
 C = *CurPtr++;
 
+  // C++23 [lex.phases] p1
+  // Diagnose invalid UTF-8 if the corresponding warning is enabled, emitting a
+  // diagnostic only once per entire ill-formed subsequence to avoid
+  // emiting to many diagnostics (see http://unicode.org/review/pr-121.html).
+  bool UnicodeDecodingAlreadyDiagnosed = false;
+
   while (true) {
 // Skip over all non-interesting characters until we fin

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

[Corentin Jabot 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 rGe3dc56805f10: [Clang] Add a warning on invalid UTF-8 in 
comments. (authored by cor3ntin).

Changed prior to commit:
  https://reviews.llvm.org/D128059?vs=441748&id=442601#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128059

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/Lexer.cpp
  clang/test/Lexer/comment-invalid-utf8.c
  llvm/include/llvm/Support/ConvertUTF.h
  llvm/lib/Support/ConvertUTF.cpp

Index: llvm/lib/Support/ConvertUTF.cpp
===
--- llvm/lib/Support/ConvertUTF.cpp
+++ llvm/lib/Support/ConvertUTF.cpp
@@ -417,6 +417,16 @@
 return isLegalUTF8(source, length);
 }
 
+/*
+ * Exported function to return the size of the first utf-8 code unit sequence,
+ * Or 0 if the sequence is not valid;
+ */
+unsigned getUTF8SequenceSize(const UTF8 *source, const UTF8 *sourceEnd) {
+  int length = trailingBytesForUTF8[*source] + 1;
+  return (length > sourceEnd - source && isLegalUTF8(source, length)) ? length
+  : 0;
+}
+
 /* - */
 
 static unsigned
Index: llvm/include/llvm/Support/ConvertUTF.h
===
--- llvm/include/llvm/Support/ConvertUTF.h
+++ llvm/include/llvm/Support/ConvertUTF.h
@@ -181,6 +181,8 @@
 
 Boolean isLegalUTF8String(const UTF8 **source, const UTF8 *sourceEnd);
 
+unsigned getUTF8SequenceSize(const UTF8 *source, const UTF8 *sourceEnd);
+
 unsigned getNumBytesForUTF8(UTF8 firstByte);
 
 /*/
Index: clang/test/Lexer/comment-invalid-utf8.c
===
--- /dev/null
+++ clang/test/Lexer/comment-invalid-utf8.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fsyntax-only %s -Winvalid-utf8 -verify=expected
+// RUN: %clang_cc1 -fsyntax-only %s -verify=nowarn
+// nowarn-no-diagnostics
+
+// This file is purposefully encoded as windows-1252
+// be careful when modifying.
+
+//€
+// expected-warning@-1 {{invalid UTF-8 in comment}}
+
+// € ‚ƒ„…†‡ˆ‰ Š ‹ Œ Ž
+// expected-warning@-1 6{{invalid UTF-8 in comment}}
+
+/*€*/
+// expected-warning@-1 {{invalid UTF-8 in comment}}
+
+/*€ ‚ƒ„…†‡ˆ‰ Š ‹ Œ Ž*/
+// expected-warning@-1 6{{invalid UTF-8 in comment}}
+
+/*
+€
+*/
+// expected-warning@-2 {{invalid UTF-8 in comment}}
+
+// abcd
+// €abcd
+// expected-warning@-1 {{invalid UTF-8 in comment}}
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -2392,13 +2392,37 @@
   //
   // This loop terminates with CurPtr pointing at the newline (or end of buffer)
   // character that ends the line comment.
+
+  // C++23 [lex.phases] p1
+  // Diagnose invalid UTF-8 if the corresponding warning is enabled, emitting a
+  // diagnostic only once per entire ill-formed subsequence to avoid
+  // emiting to many diagnostics (see http://unicode.org/review/pr-121.html).
+  bool UnicodeDecodingAlreadyDiagnosed = false;
+
   char C;
   while (true) {
 C = *CurPtr;
 // Skip over characters in the fast loop.
-while (C != 0 &&// Potentially EOF.
-   C != '\n' && C != '\r')  // Newline or DOS-style newline.
+while (isASCII(C) && C != 0 &&   // Potentially EOF.
+   C != '\n' && C != '\r') { // Newline or DOS-style newline.
   C = *++CurPtr;
+  UnicodeDecodingAlreadyDiagnosed = false;
+}
+
+if (!isASCII(C)) {
+  unsigned Length = llvm::getUTF8SequenceSize(
+  (const llvm::UTF8 *)CurPtr, (const llvm::UTF8 *)BufferEnd);
+  if (Length == 0) {
+if (!UnicodeDecodingAlreadyDiagnosed && !isLexingRawMode())
+  Diag(CurPtr, diag::warn_invalid_utf8_in_comment);
+UnicodeDecodingAlreadyDiagnosed = true;
+++CurPtr;
+  } else {
+UnicodeDecodingAlreadyDiagnosed = false;
+CurPtr += Length;
+  }
+  continue;
+}
 
 const char *NextLine = CurPtr;
 if (C != 0) {
@@ -2665,6 +2689,12 @@
   if (C == '/')
 C = *CurPtr++;
 
+  // C++23 [lex.phases] p1
+  // Diagnose invalid UTF-8 if the corresponding warning is enabled, emitting a
+  // diagnostic only once per entire ill-formed subsequence to avoid
+  // emiting to many diagnostics (see http://unicode.org/review/pr-121.html).
+  bool UnicodeDecodingAlreadyDiagnosed = false;
+
   while (true) {
 // Skip over all non-interesting characters until we find end of buffer or a
 // (probably ending) '/' character.
@@ -2673,14 +2703,24 @@
 // doesn't check for '\

[PATCH] D129100: [clangd] Support external throttler for preamble builds

[Andres Freund via Phabricator via cfe-commits]

anarazel added a comment.

This caused llvm builds to fail for me (using clang-14, debug, debian unstable, 
lld as linker):

  FAILED: tools/clang/tools/extra/clangd/unittests/ClangdTests 
  : && /usr/bin/clang++-14 -gz=zlib -fPIC -fno-semantic-interposition 
-fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -fdiagnostics-color -fno-common -Woverloaded-virtual 
-Wno-nested-anon-types -g 
-Wl,-rpath,/home/andres/build/llvm-project/master/debug-clang/install/lib 
-Wl,--gdb-index -fuse-ld=lld -Wl,--gdb-index -Wl,--color-diagnostics 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/AddUsing.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/AnnotateHighlightings.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/DumpAST.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/DefineInline.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/DefineOutline.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/ExpandAutoType.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/ExpandMacro.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/ExtractFunction.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/ExtractVariable.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/MemberwiseConstructor.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/ObjCLocalizeStringLiteral.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/ObjCMemberwiseInitializer.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/PopulateSwitch.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/RawStringLiteral.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/RemoveUsingNamespace.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/SpecialMembers.cpp.o
 
tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/SwapIfBranches.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/Annotations.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ASTTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ASTSignalsTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/BackgroundIndexTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CallHierarchyTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CanonicalIncludesTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ClangdTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ClangdLSPServerTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CodeCompleteTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CodeCompletionStringsTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CollectMacrosTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CompileCommandsTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CompilerTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ConfigCompileTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ConfigProviderTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ConfigYAMLTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/DecisionForestTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/DexTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/DiagnosticsTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/DraftStoreTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/DumpASTTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/ExpectedTypeTest.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/FeatureModulesTests.cpp.o
 
tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/FileDistanceTests.cpp.o
 
tools/clang/tools/e

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:608
+  ///
+  /// \param DL  Debug location for instructions added by unrolling.
+  /// \param LoopThe simd loop.

psoni2628 wrote:
> jdoerfert wrote:
> > No debug location needed. You also copied the comment that makes little 
> > sene.
> It isn't used in the original applySimd either. Should I remove it in both 
> places?
Yes.


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

https://reviews.llvm.org/D129149

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

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

As I commented on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836#c32,

> It doesn't make sense to have a mode in which `int array[0]` is accepted but 
> is not a flex array.
> Either that should be a compilation error (as the standard specifies), or it 
> should be a flex array. Accepting it as an extension but having it do the 
> wrong thing is not useful or helpful.
> Note that Clang has a dedicated warning flag for zero-length arrays: 
> -Wzero-length-array, so anyone who wants to prohibit them may use 
> -Werror=zero-length-array. It would be helpful for GCC could follow suit 
> there.

The -fstrict-flex-arrays=3 option should be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864

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

[clang-tools-extra] e7fa272 - [clangd] Fix missing key function in PreambleThrottler

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-07-06T18:27:09+02:00
New Revision: e7fa272bc6a6a504c2899bb7cf66029678d97890

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

LOG: [clangd] Fix missing key function in PreambleThrottler

Added: 


Modified: 
clang-tools-extra/clangd/TUScheduler.h

Removed: 




diff  --git a/clang-tools-extra/clangd/TUScheduler.h 
b/clang-tools-extra/clangd/TUScheduler.h
index 2817f1343185d..d616814a5edb9 100644
--- a/clang-tools-extra/clangd/TUScheduler.h
+++ b/clang-tools-extra/clangd/TUScheduler.h
@@ -104,7 +104,7 @@ class PreambleThrottler {
   ///
   /// Does not block, may eventually invoke the callback to satisfy the 
request.
   /// If the callback is invoked, release() must be called afterwards.
-  virtual RequestID acquire(llvm::StringRef Filename, Callback);
+  virtual RequestID acquire(llvm::StringRef Filename, Callback) = 0;
   /// Abandons the request/releases any resources that have been acquired.
   ///
   /// Must be called exactly once after acquire().



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

[PATCH] D129100: [clangd] Support external throttler for preamble builds

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D129100#3633073 , @anarazel wrote:

> This caused llvm builds to fail for me (using clang-14, debug, debian 
> unstable, lld as linker):

Thanks, I have a very similar setup but the build passed locally, I'll never 
understand linkers...

I think I've fixed this in e7fa272bc6a6a504c2899bb7cf66029678d97890 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129100

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

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

[Paul Scoropan via Phabricator via cfe-commits]

pscoro updated this revision to Diff 442615.
pscoro added a comment.

trying to localize calng-format again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129016

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc-stackprotect.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll

Index: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
@@ -0,0 +1,77 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux \
+; RUN:   --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=LINUX
+
+declare void @llvm.ppc.kill.canary()
+define dso_local void @test_kill_canary() {
+; AIX-LABEL: test_kill_canary:
+; AIX:   # %bb.0: # %entry
+; AIX-NEXT:blr
+;
+; LINUX-LABEL: test_kill_canary:
+; LINUX:   # %bb.0: # %entry
+; LINUX-NEXT:blr
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
+
+attributes #0 = { sspreq }
+; Function Attrs: sspreq
+define dso_local void @test_kill_canary_ssp() #0 {
+; AIX-LABEL: test_kill_canary_ssp:
+; AIX:   # %bb.0: # %entry
+; AIX-NEXT:mflr r0
+; AIX-NEXT:std r0, 16(r1)
+; AIX-NEXT:stdu r1, -128(r1)
+; AIX-NEXT:ld r3, L..C0(r2) # @__ssp_canary_word
+; AIX-NEXT:ld r4, 0(r3)
+; AIX-NEXT:std r4, 120(r1)
+; AIX-NEXT:ld r4, 0(r3)
+; AIX-NEXT:xori r4, r4, 65535
+; AIX-NEXT:xoris r4, r4, 65535
+; AIX-NEXT:std r4, 0(r3)
+; AIX-NEXT:ld r3, 0(r3)
+; AIX-NEXT:ld r4, 120(r1)
+; AIX-NEXT:cmpld r3, r4
+; AIX-NEXT:bne cr0, L..BB1_2
+; AIX-NEXT:  # %bb.1: # %entry
+; AIX-NEXT:addi r1, r1, 128
+; AIX-NEXT:ld r0, 16(r1)
+; AIX-NEXT:mtlr r0
+; AIX-NEXT:blr
+; AIX-NEXT:  L..BB1_2: # %entry
+; AIX-NEXT:bl .__stack_chk_fail[PR]
+; AIX-NEXT:nop
+;
+; LINUX-LABEL: test_kill_canary_ssp:
+; LINUX:   # %bb.0: # %entry
+; LINUX-NEXT:mflr r0
+; LINUX-NEXT:std r0, 16(r1)
+; LINUX-NEXT:stdu r1, -128(r1)
+; LINUX-NEXT:.cfi_def_cfa_offset 128
+; LINUX-NEXT:.cfi_offset lr, 16
+; LINUX-NEXT:ld r3, -28688(r13)
+; LINUX-NEXT:std r3, 120(r1)
+; LINUX-NEXT:ld r3, -28688(r13)
+; LINUX-NEXT:xori r3, r3, 65535
+; LINUX-NEXT:xoris r3, r3, 65535
+; LINUX-NEXT:std r3, 120(r1)
+; LINUX-NEXT:ld r3, 120(r1)
+; LINUX-NEXT:ld r4, -28688(r13)
+; LINUX-NEXT:cmpld r4, r3
+; LINUX-NEXT:bne cr0, .LBB1_2
+; LINUX-NEXT:  # %bb.1: # %entry
+; LINUX-NEXT:addi r1, r1, 128
+; LINUX-NEXT:ld r0, 16(r1)
+; LINUX-NEXT:mtlr r0
+; LINUX-NEXT:blr
+; LINUX-NEXT:  .LBB1_2: # %entry
+; LINUX-NEXT:bl __stack_chk_fail
+; LINUX-NEXT:nop
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
Index: llvm/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -11127,6 +11127,72 @@
 }
 break;
   }
+  case Intrinsic::ppc_kill_canary: {
+MachineFunction &MF = DAG.getMachineFunction();
+const Module *M = MF.getMMI().getModule();
+
+/* If SafeStack or !StackProtector, kill_canary not supported */
+if (MF.getFunction().hasFnAttribute(Attribute::SafeStack) ||
+!MF.getFunction().hasStackProtectorFnAttr()) {
+  DAG.ReplaceAllUsesOfValueWith(
+  SDValue(Op.getNode(), 0),
+  Op->getOperand(0)); // prepare node for deletion
+  break;
+}
+
+EVT VT = DAG.getTargetLoweringInfo().getPointerTy(DAG.getDataLayout());
+
+SDValue Load;
+SDValue Store;
+
+const uint64_t XORWord =
+0x; // XORing with 0b111...111 will never
+// result in the original word
+
+if (useLoadStackGuardNode()) { // linux uses LOAD_STACK_GUARD node instead
+   // of having a canary word global value
+  MachineSDNode *LSG =
+  DAG.getMachineNode(PPC::LOAD_STACK_GUARD, DL, VT, Op->getOperand(0));
+  Load = SDValue(LSG, 0);
+
+  /* frame index used to determine stack guard location if
+   * LOAD_STACK_GUARD is used */
+  MachineFrameInfo &MFI = MF.getFrameInfo();
+  int SPI = MFI.getStackProtectorIndex(); // should return -1
+  PPCFunctionInfo *FuncInfo = MF.getInfo();
+  SDValue FIN = DAG.getFrameIndex(FuncInfo->getVarArgsFrameIndex(), VT);
+
+  // XOR canary word and store back
+  Store = DAG.getStore(
+  Op->getOperand(0), DL,
+  

Re: [PATCH] D129100: [clangd] Support external throttler for preamble builds

[Andres Freund via cfe-commits]

Hi,

On 2022-07-06 16:28:14 +, Sam McCall via Phabricator wrote:
> sammccall added a comment.
> 
> In D129100#3633073 , @anarazel 
> wrote:
> 
> > This caused llvm builds to fail for me (using clang-14, debug, debian 
> > unstable, lld as linker):
> 
> Thanks, I have a very similar setup but the build passed locally, I'll never 
> understand linkers...

Might depend on optimization level (if references are discovered to be dead,
there's no undefined reference)?


> I think I've fixed this in e7fa272bc6a6a504c2899bb7cf66029678d97890 
> 

Looks good now, thanks!

Greetings,

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

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:2644
+
+Control which arrays are considered as flexible arrays members. 
+can be 1 (array of size 0, 1 and undefined are considered), 2 (array of size 0

Docs should also mention what the default -fno-strict-flex-arrays means -- that 
ALL sizes of trailing arrays are considered flexible array members. (I'm amazed 
that's the rule, and I never knew it. I always thought the special casing for 
FAMs was restricted to sizes 0 and 1!)

Also, since apparently different parts of the compiler have been (and will now 
continue to) use different default behaviors, may want to document that as 
well. I'm sure I don't know what the rules actually are intended to be here. 
E.g. that a macro-expansion of the size arg disables the special-behavior for 
[1] is extremely surprising!



Comment at: clang/lib/Sema/SemaChecking.cpp:15804
+
+  // FIXME: we should also allow Size = 0 here per the definition of
+  // StrictFlexArraysLevel, but that's backward incompatible with previous 
clang

Presumably the size-zero/unsized cases are already being taken care of 
elsewhere in the code? I find it hard to believe we are currently emitting 
diagnostics for size-0 FAM which we don't emit for size-1 FAM?



Comment at: clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp:9
+  } s2;
+  s2.a[2] = 0; // no-warning
+}

Except we actually _do_ know the bounds of the full-object and ought to be able 
to warn on this code anyhow...

Better to have the test function accept a pointer, so that's not a conflating 
issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864

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

[PATCH] D127042: [Clang][OpenMP] Enable floating-point operation for `atomic compare` series

[Shilei Tian 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 rG83837a61981c: [Clang][OpenMP] Enable floating-point 
operation for `atomic compare` series (authored by tianshilei1992).

Changed prior to commit:
  https://reviews.llvm.org/D127042?vs=442588&id=442618#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127042

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_compare_codegen.cpp
  clang/test/OpenMP/atomic_messages.c
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

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

[PATCH] D129218: [CMake][Fuchsia] Install static libuwind

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added a reviewer: abrachet.
Herald added a subscriber: mgorny.
Herald added a project: All.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This can now be used with -static-libgcc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129218

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -183,7 +183,6 @@
 set(RUNTIMES_${target}_COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_HIDE_SYMBOLS ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_STATIC_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -183,7 +183,6 @@
 set(RUNTIMES_${target}_COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_HIDE_SYMBOLS ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_STATIC_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 442623.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Update code to match how typedefs behave
- remove leftover test from previous version
- Add test for C++20 modules, rewrite original test with split-file
- use isReachable, do not call makeMergedDefinitionVisible


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128921

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Modules/merge-concepts-cxx-modules.cpp
  clang/test/Modules/merge-concepts.cpp

Index: clang/test/Modules/merge-concepts.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-concepts.cpp
@@ -0,0 +1,83 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -fmodules -fmodule-name=library \
+// RUN: -emit-module %t/modules.map \
+// RUN: -o %t/module.pcm
+//
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -fmodules -fmodule-file=%t/module.pcm  \
+// RUN: -fmodule-map-file=%t/modules.map \
+// RUN: -fsyntax-only -verify %t/use.cpp
+//
+//--- use.cpp
+
+#include "concepts.h"
+#include "conflicting.h"
+#include "format.h"
+
+template  void foo()
+  requires same_as
+{}
+ConflictingConcept auto x = 10; // expected-error {{reference to 'ConflictingConcept' is ambiguous}}
+// expected-note@* 2 {{candidate}}
+
+//--- modules.map
+module "library" {
+	export *
+	module "concepts" {
+		export *
+		header "concepts.h"
+	}
+	module "format" {
+		export *
+		header "format.h"
+	}
+	module "conflicting" {
+		export *
+		header "conflicting.h"
+	}
+}
+
+//--- concepts.h
+#ifndef SAMEAS_CONCEPTS_H_
+#define SAMEAS_CONCEPTS_H_
+
+#include "same_as.h"
+
+template 
+concept ConflictingConcept = true;
+
+#endif // SAMEAS_CONCEPTS_H
+
+//--- same_as.h
+#ifndef SAME_AS_H
+#define SAME_AS_H
+
+template 
+concept same_as = __is_same(T, U);
+
+#endif // SAME_AS_H
+
+//--- format.h
+#ifndef FORMAT_H
+#define FORMAT_H
+
+#include "concepts.h"
+#include "same_as.h"
+
+template  void foo()
+  requires same_as
+{}
+
+#endif // FORMAT_H
+
+//--- conflicting.h
+#ifndef CONFLICTING_H
+#define CONFLICTING_H
+
+template 
+concept ConflictingConcept = true;
+
+#endif // CONFLICTING_H
Index: clang/test/Modules/merge-concepts-cxx-modules.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-concepts-cxx-modules.cpp
@@ -0,0 +1,46 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/same_as.cppm -o %t/same_as.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/concepts.cppm -o %t/concepts.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/format.cppm -o %t/format.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/conflicting.cppm -o %t/conflicting.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use.cppm -fsyntax-only -verify
+
+//--- same_as.cppm
+export module same_as;
+export template 
+concept same_as = __is_same(T, U);
+
+//--- concepts.cppm
+export module concepts;
+export import same_as;
+
+export template 
+concept ConflictingConcept = true;
+
+//--- format.cppm
+
+export module format;
+export import concepts;
+export import same_as;
+
+export template  void foo()
+  requires same_as
+{}
+
+//--- conflicting.cppm
+export module conflicting;
+export template 
+concept ConflictingConcept = true;
+
+//--- Use.cppm
+import format;
+import conflicting;
+
+template  void foo()
+  requires same_as
+{}
+ConflictingConcept auto x = 10; // expected-error {{reference to 'ConflictingConcept' is ambiguous}}
+// expected-note@* 2 {{candidate}}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -8656,23 +8656,52 @@
   // Check for conflicting previous declaration.
   DeclarationNameInfo NameInfo(NewDecl->getDeclName(), NameLoc);
   LookupResult Previous(*this, NameInfo, LookupOrdinaryName,
-ForVisibleRedeclaration);
+forRedeclarationInCurContext());
   LookupName(Previous, S);
-
   FilterLookupForScope(Previous, DC, S, /*ConsiderLinkage=*/false,
/*AllowInlineNamespace*/false);
-  if (!Previous.empty()) {
-auto *Old = Previous.getRepresentativeDecl();
-Diag(NameLoc, isa(Old) ? diag::err_redefinition :
- diag::err_redefinition_different_kind) << NewDecl->getDeclName();
-Diag(Old->getLocation(), diag::note_previous_definition);
-  }
+  bool AddToScope = true;
+  CheckConceptRedefinition(NewDecl, Previous, AddToScope);
 
   ActOnDo

[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Many thanks! I didn't know about `split-file`, it's much nicer indeed!

Also incorporating feedback from @rsmith and removing the call to 
`makeMergedDefinitionVisible`. Keeping just `setPrimaryMergedDecl` is enough 
here.
Richard's reply from the email exchange:

  I don't think that makeMergedDefinitionVisible is quite right; that's 
intended to solve a different problem. Specifically, when we have a repeated 
class definition in a single parse:
  
  // in module X
  class A { void f(); };
  
  // in module Y
  class A { void f(); };
  
  Here, we first see and parse the class definition of A in module X. That then 
becomes "the" definition of A, for the purposes of this compilation. Then when 
we see another definition of the class, we choose to skip it because we already 
have a definition, and Clang wants there to only be one definition of each 
class. So we end up behaving as if we parsed this:
  
  // in module X
  class A { void f(); };
  
  // in module Y
  class A;
  
  ... but we still need to track that class A has a complete definition if only 
Y is imported, so that A::f can be used for example. And that's what 
makeMergedDefinitionVisible does: it says that A should be considered as having 
a complete definition when Y is imported, even though the definition actually 
lives in X.
  
  For concepts I think the situation is different: we're not going to skip the 
"body" of the concept and treat
  
  // module X
  template concept bool C = true;
  
  // module Y
  template concept bool C = true;
  
  as if it were
  
  
  // module X
  template concept bool C = true;
  
  // module Y
  template concept bool C; // forward declaration?
  
  ... because there's no such thing as a forward declaration of a concept. 
Instead, I think we should just be calling ASTContext::setPrimaryMergedDecl to 
say that the C in module Y is a redeclaration of the C in module X, so that a 
name lookup that finds both is not ambiguous. It shouldn't matter which one we 
actually pick, because they are the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128921

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

[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

PS I will make sure to look at the patches you sent my way this week.
Wanted to do it earlier, but have been having some personal emergencies I 
needed to take care of.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128921

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

[PATCH] D129218: [CMake][Fuchsia] Install static libuwind

[Petr Hosek via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd34ce04f98c3: [CMake][Fuchsia] Install static libuwind 
(authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129218

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -183,7 +183,6 @@
 set(RUNTIMES_${target}_COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_HIDE_SYMBOLS ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_STATIC_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -183,7 +183,6 @@
 set(RUNTIMES_${target}_COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_HIDE_SYMBOLS ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_STATIC_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] d34ce04 - [CMake][Fuchsia] Install static libuwind

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2022-07-06T17:17:40Z
New Revision: d34ce04f98c387aa86c13cb502e3e2a8a4d8f38b

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

LOG: [CMake][Fuchsia] Install static libuwind

This can now be used with -static-libgcc.

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

Added: 


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

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 4e3ed6086dec5..412dbdf71b68a 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -183,7 +183,6 @@ if(FUCHSIA_SDK)
 set(RUNTIMES_${target}_COMPILER_RT_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBUNWIND_HIDE_SYMBOLS ON CACHE BOOL "")
-set(RUNTIMES_${target}_LIBUNWIND_INSTALL_STATIC_LIBRARY OFF CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_COMPILER_RT ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_USE_LLVM_UNWINDER ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXXABI_ENABLE_STATIC_UNWINDER ON CACHE BOOL "")



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

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

As far as I can tell, this breaks check-clang everywhere: http://45.33.8.238/

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128059

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

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:2644
+
+Control which arrays are considered as flexible arrays members. 
+can be 1 (array of size 0, 1 and undefined are considered), 2 (array of size 0

jyknight wrote:
> Docs should also mention what the default -fno-strict-flex-arrays means -- 
> that ALL sizes of trailing arrays are considered flexible array members. (I'm 
> amazed that's the rule, and I never knew it. I always thought the special 
> casing for FAMs was restricted to sizes 0 and 1!)
> 
> Also, since apparently different parts of the compiler have been (and will 
> now continue to) use different default behaviors, may want to document that 
> as well. I'm sure I don't know what the rules actually are intended to be 
> here. E.g. that a macro-expansion of the size arg disables the 
> special-behavior for [1] is extremely surprising!
it is worse than that: for some checks, any size is valid for FAM, but not for 
alls. For some checks, macro expansion prohibits FAM, but not for all, etc, 
etc. I don't want to document that behavior, because it is too specific to each 
pass. My plan is

1. land -fstrict-flex-array support
2. launch a thread on the ugly situation we put ourselves in, and extract a 
decision for each case
3. support and document an homogeneous behavior across passes.
4. syndicate code across passes



Comment at: clang/lib/Sema/SemaChecking.cpp:15804
+
+  // FIXME: we should also allow Size = 0 here per the definition of
+  // StrictFlexArraysLevel, but that's backward incompatible with previous 
clang

jyknight wrote:
> Presumably the size-zero/unsized cases are already being taken care of 
> elsewhere in the code? I find it hard to believe we are currently emitting 
> diagnostics for size-0 FAM which we don't emit for size-1 FAM?
correct



Comment at: clang/lib/Sema/SemaChecking.cpp:15969
 // access which precedes the array bounds.
 if (BaseType->isIncompleteType())
   return;

And here



Comment at: clang/lib/Sema/SemaChecking.cpp:15973
 llvm::APInt size = ArrayTy->getSize();
 if (!size.isStrictlyPositive())
   return;

Handled here



Comment at: clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp:9
+  } s2;
+  s2.a[2] = 0; // no-warning
+}

jyknight wrote:
> Except we actually _do_ know the bounds of the full-object and ought to be 
> able to warn on this code anyhow...
> 
> Better to have the test function accept a pointer, so that's not a conflating 
> issue?
Correct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864

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

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

[Paul Scoropan via Phabricator via cfe-commits]

pscoro updated this revision to Diff 442627.
pscoro added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129016

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc-stackprotect.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll

Index: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
@@ -0,0 +1,77 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux \
+; RUN:   --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=LINUX
+
+declare void @llvm.ppc.kill.canary()
+define dso_local void @test_kill_canary() {
+; AIX-LABEL: test_kill_canary:
+; AIX:   # %bb.0: # %entry
+; AIX-NEXT:blr
+;
+; LINUX-LABEL: test_kill_canary:
+; LINUX:   # %bb.0: # %entry
+; LINUX-NEXT:blr
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
+
+attributes #0 = { sspreq }
+; Function Attrs: sspreq
+define dso_local void @test_kill_canary_ssp() #0 {
+; AIX-LABEL: test_kill_canary_ssp:
+; AIX:   # %bb.0: # %entry
+; AIX-NEXT:mflr r0
+; AIX-NEXT:std r0, 16(r1)
+; AIX-NEXT:stdu r1, -128(r1)
+; AIX-NEXT:ld r3, L..C0(r2) # @__ssp_canary_word
+; AIX-NEXT:ld r4, 0(r3)
+; AIX-NEXT:std r4, 120(r1)
+; AIX-NEXT:ld r4, 0(r3)
+; AIX-NEXT:xori r4, r4, 65535
+; AIX-NEXT:xoris r4, r4, 65535
+; AIX-NEXT:std r4, 0(r3)
+; AIX-NEXT:ld r3, 0(r3)
+; AIX-NEXT:ld r4, 120(r1)
+; AIX-NEXT:cmpld r3, r4
+; AIX-NEXT:bne cr0, L..BB1_2
+; AIX-NEXT:  # %bb.1: # %entry
+; AIX-NEXT:addi r1, r1, 128
+; AIX-NEXT:ld r0, 16(r1)
+; AIX-NEXT:mtlr r0
+; AIX-NEXT:blr
+; AIX-NEXT:  L..BB1_2: # %entry
+; AIX-NEXT:bl .__stack_chk_fail[PR]
+; AIX-NEXT:nop
+;
+; LINUX-LABEL: test_kill_canary_ssp:
+; LINUX:   # %bb.0: # %entry
+; LINUX-NEXT:mflr r0
+; LINUX-NEXT:std r0, 16(r1)
+; LINUX-NEXT:stdu r1, -128(r1)
+; LINUX-NEXT:.cfi_def_cfa_offset 128
+; LINUX-NEXT:.cfi_offset lr, 16
+; LINUX-NEXT:ld r3, -28688(r13)
+; LINUX-NEXT:std r3, 120(r1)
+; LINUX-NEXT:ld r3, -28688(r13)
+; LINUX-NEXT:xori r3, r3, 65535
+; LINUX-NEXT:xoris r3, r3, 65535
+; LINUX-NEXT:std r3, 120(r1)
+; LINUX-NEXT:ld r3, 120(r1)
+; LINUX-NEXT:ld r4, -28688(r13)
+; LINUX-NEXT:cmpld r4, r3
+; LINUX-NEXT:bne cr0, .LBB1_2
+; LINUX-NEXT:  # %bb.1: # %entry
+; LINUX-NEXT:addi r1, r1, 128
+; LINUX-NEXT:ld r0, 16(r1)
+; LINUX-NEXT:mtlr r0
+; LINUX-NEXT:blr
+; LINUX-NEXT:  .LBB1_2: # %entry
+; LINUX-NEXT:bl __stack_chk_fail
+; LINUX-NEXT:nop
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
Index: llvm/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -11143,6 +11143,72 @@
 }
 break;
   }
+  case Intrinsic::ppc_kill_canary: {
+MachineFunction &MF = DAG.getMachineFunction();
+const Module *M = MF.getMMI().getModule();
+
+/* If SafeStack or !StackProtector, kill_canary not supported */
+if (MF.getFunction().hasFnAttribute(Attribute::SafeStack) ||
+!MF.getFunction().hasStackProtectorFnAttr()) {
+  DAG.ReplaceAllUsesOfValueWith(
+  SDValue(Op.getNode(), 0),
+  Op->getOperand(0)); // prepare node for deletion
+  break;
+}
+
+EVT VT = DAG.getTargetLoweringInfo().getPointerTy(DAG.getDataLayout());
+
+SDValue Load;
+SDValue Store;
+
+const uint64_t XORWord =
+0x; // XORing with 0b111...111 will never
+// result in the original word
+
+if (useLoadStackGuardNode()) { // linux uses LOAD_STACK_GUARD node instead
+   // of having a canary word global value
+  MachineSDNode *LSG =
+  DAG.getMachineNode(PPC::LOAD_STACK_GUARD, DL, VT, Op->getOperand(0));
+  Load = SDValue(LSG, 0);
+
+  /* frame index used to determine stack guard location if
+   * LOAD_STACK_GUARD is used */
+  MachineFrameInfo &MFI = MF.getFrameInfo();
+  int SPI = MFI.getStackProtectorIndex(); // should return -1
+  PPCFunctionInfo *FuncInfo = MF.getInfo();
+  SDValue FIN = DAG.getFrameIndex(FuncInfo->getVarArgsFrameIndex(), VT);
+
+  // XOR canary word and store back
+  Store = DAG.getStore(
+  Op->getOperand(0), DL,
+  DAG.getNode(ISD::XOR, DL, VT, 

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Prabhdeep Soni via Phabricator via cfe-commits]

psoni2628 added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:608
+  ///
+  /// \param DL  Debug location for instructions added by unrolling.
+  /// \param LoopThe simd loop.

jdoerfert wrote:
> psoni2628 wrote:
> > jdoerfert wrote:
> > > No debug location needed. You also copied the comment that makes little 
> > > sene.
> > It isn't used in the original applySimd either. Should I remove it in both 
> > places?
> Yes.
Looks like I'll need to fix `applySimd` on the MLIR side too. I think it makes 
more sense to do that separately in a different patch. For now, I will leave 
`applySimd` with the unused `DebugLocation`. I will fix `applySimdlen`.


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

https://reviews.llvm.org/D129149

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

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

[Paul Scoropan via Phabricator via cfe-commits]

pscoro updated this revision to Diff 442630.
pscoro added a comment.

patch appication troubleshooting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129016

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc-stackprotect.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll

Index: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
@@ -0,0 +1,77 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux \
+; RUN:   --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=LINUX
+
+declare void @llvm.ppc.kill.canary()
+define dso_local void @test_kill_canary() {
+; AIX-LABEL: test_kill_canary:
+; AIX:   # %bb.0: # %entry
+; AIX-NEXT:blr
+;
+; LINUX-LABEL: test_kill_canary:
+; LINUX:   # %bb.0: # %entry
+; LINUX-NEXT:blr
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
+
+attributes #0 = { sspreq }
+; Function Attrs: sspreq
+define dso_local void @test_kill_canary_ssp() #0 {
+; AIX-LABEL: test_kill_canary_ssp:
+; AIX:   # %bb.0: # %entry
+; AIX-NEXT:mflr r0
+; AIX-NEXT:std r0, 16(r1)
+; AIX-NEXT:stdu r1, -128(r1)
+; AIX-NEXT:ld r3, L..C0(r2) # @__ssp_canary_word
+; AIX-NEXT:ld r4, 0(r3)
+; AIX-NEXT:std r4, 120(r1)
+; AIX-NEXT:ld r4, 0(r3)
+; AIX-NEXT:xori r4, r4, 65535
+; AIX-NEXT:xoris r4, r4, 65535
+; AIX-NEXT:std r4, 0(r3)
+; AIX-NEXT:ld r3, 0(r3)
+; AIX-NEXT:ld r4, 120(r1)
+; AIX-NEXT:cmpld r3, r4
+; AIX-NEXT:bne cr0, L..BB1_2
+; AIX-NEXT:  # %bb.1: # %entry
+; AIX-NEXT:addi r1, r1, 128
+; AIX-NEXT:ld r0, 16(r1)
+; AIX-NEXT:mtlr r0
+; AIX-NEXT:blr
+; AIX-NEXT:  L..BB1_2: # %entry
+; AIX-NEXT:bl .__stack_chk_fail[PR]
+; AIX-NEXT:nop
+;
+; LINUX-LABEL: test_kill_canary_ssp:
+; LINUX:   # %bb.0: # %entry
+; LINUX-NEXT:mflr r0
+; LINUX-NEXT:std r0, 16(r1)
+; LINUX-NEXT:stdu r1, -128(r1)
+; LINUX-NEXT:.cfi_def_cfa_offset 128
+; LINUX-NEXT:.cfi_offset lr, 16
+; LINUX-NEXT:ld r3, -28688(r13)
+; LINUX-NEXT:std r3, 120(r1)
+; LINUX-NEXT:ld r3, -28688(r13)
+; LINUX-NEXT:xori r3, r3, 65535
+; LINUX-NEXT:xoris r3, r3, 65535
+; LINUX-NEXT:std r3, 120(r1)
+; LINUX-NEXT:ld r3, 120(r1)
+; LINUX-NEXT:ld r4, -28688(r13)
+; LINUX-NEXT:cmpld r4, r3
+; LINUX-NEXT:bne cr0, .LBB1_2
+; LINUX-NEXT:  # %bb.1: # %entry
+; LINUX-NEXT:addi r1, r1, 128
+; LINUX-NEXT:ld r0, 16(r1)
+; LINUX-NEXT:mtlr r0
+; LINUX-NEXT:blr
+; LINUX-NEXT:  .LBB1_2: # %entry
+; LINUX-NEXT:bl __stack_chk_fail
+; LINUX-NEXT:nop
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
Index: llvm/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -11143,6 +11143,72 @@
 }
 break;
   }
+  case Intrinsic::ppc_kill_canary: {
+MachineFunction &MF = DAG.getMachineFunction();
+const Module *M = MF.getMMI().getModule();
+
+/* If SafeStack or !StackProtector, kill_canary not supported */
+if (MF.getFunction().hasFnAttribute(Attribute::SafeStack) ||
+!MF.getFunction().hasStackProtectorFnAttr()) {
+  DAG.ReplaceAllUsesOfValueWith(
+  SDValue(Op.getNode(), 0),
+  Op->getOperand(0)); // prepare node for deletion
+  break;
+}
+
+EVT VT = DAG.getTargetLoweringInfo().getPointerTy(DAG.getDataLayout());
+
+SDValue Load;
+SDValue Store;
+
+const uint64_t XORWord =
+0x; // XORing with 0b111...111 will never
+// result in the original word
+
+if (useLoadStackGuardNode()) { // linux uses LOAD_STACK_GUARD node instead
+   // of having a canary word global value
+  MachineSDNode *LSG =
+  DAG.getMachineNode(PPC::LOAD_STACK_GUARD, DL, VT, Op->getOperand(0));
+  Load = SDValue(LSG, 0);
+
+  /* frame index used to determine stack guard location if
+   * LOAD_STACK_GUARD is used */
+  MachineFrameInfo &MFI = MF.getFrameInfo();
+  int SPI = MFI.getStackProtectorIndex(); // should return -1
+  PPCFunctionInfo *FuncInfo = MF.getInfo();
+  SDValue FIN = DAG.getFrameIndex(FuncInfo->getVarArgsFrameIndex(), VT);
+
+  // XOR canary word and store back
+  Store = DAG.getStore(
+  Op->getOperand(0), DL,
+  DAG.g

[PATCH] D123878: [AMDGPU] Add remarks to output some resource usage

[Vang Thao via Phabricator via cfe-commits]

vangthao marked 5 inline comments as done.
vangthao added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123878

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

[PATCH] D125693: [DebugInfo] Support types, imports and static locals declared in a lexical block (3/5)

[Ellis Hoag via Phabricator via cfe-commits]

ellis added inline comments.



Comment at: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll:23
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([3 x i8], [3 
x i8]* @__profn_foo, i32 0, i32 0), i64 12345678, i32 2, i32 0)
-  ret void
+  ret void, !dbg !17
 }

krisb wrote:
> ellis wrote:
> > I asked the same question in D113741, but why is this test changed?
> Normally, we emit function-local entities iff a parent function has location 
> information. This is done the same way for local variables, labels, 
> parameters, imported entities, and, //now,// for static locals as well.
> Before this change static locals behaved more like global variables and get 
> emitted doesn't matter its parent function. This patch makes them handled 
> more like local variables.
> 
> I believe either the call or the 'ret' (or, likely, both) had had DILocation 
> attached originally, but it has been removed to simplify the test.
I just checked and the `llvm.instrprof.increment` intrinsic does not have debug 
info attached. I think you're right that I removed debug info from the `ret` 
instruction to simplify the test.

This is a special case where a global and its debug info is synthesized.
https://github.com/llvm/llvm-project/blob/23c2bedfd93cfacc62009425c464e659a34e92e6/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp#L976-L1001

So I don't understand why this added debug info is necessary. Does the test 
fail otherwise?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125693

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

[clang] fb06dd3 - Revert "[Clang] Add a warning on invalid UTF-8 in comments."

[Corentin Jabot via cfe-commits]

Author: Corentin Jabot
Date: 2022-07-06T19:45:12+02:00
New Revision: fb06dd3e8ca1b89579055a74f2217e7665c3f150

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

LOG: Revert "[Clang] Add a warning on invalid UTF-8 in comments."

Reverting while I investigate build failures

This reverts commit e3dc56805f1029dd5959e4c69196a287961afb8d.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/lib/Lex/Lexer.cpp
llvm/include/llvm/Support/ConvertUTF.h
llvm/lib/Support/ConvertUTF.cpp

Removed: 
clang/test/Lexer/comment-invalid-utf8.c



diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c50e1b89649cf..0f542e08b841c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -279,8 +279,6 @@ Improvements to Clang's diagnostics
   unevaluated operands of a ``typeid`` expression, as they are now
   modeled correctly in the CFG. This fixes
   `Issue 21668 `_.
-- Added ``-Winvalid-utf8`` which diagnoses invalid UTF-8 code unit sequences in
-  comments.
 
 Non-comprehensive list of changes in this release
 -
@@ -578,7 +576,7 @@ AST Matchers
 
 - Added ``forEachTemplateArgument`` matcher which creates a match every
   time a ``templateArgument`` matches the matcher supplied to it.
-
+  
 - Added ``objcStringLiteral`` matcher which matches ObjectiveC String
   literal expressions.
 

diff  --git a/clang/include/clang/Basic/DiagnosticLexKinds.td 
b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 38ee022e5f04c..ac86076140c58 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -113,8 +113,6 @@ def warn_four_char_character_literal : Warning<
 // Unicode and UCNs
 def err_invalid_utf8 : Error<
   "source file is not valid UTF-8">;
-def warn_invalid_utf8_in_comment : Extension<
-  "invalid UTF-8 in comment">, InGroup>;
 def err_character_not_allowed : Error<
   "unexpected character ">;
 def err_character_not_allowed_identifier : Error<

diff  --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 351e518c7ed37..6820057642bea 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -2392,37 +2392,13 @@ bool Lexer::SkipLineComment(Token &Result, const char 
*CurPtr,
   //
   // This loop terminates with CurPtr pointing at the newline (or end of 
buffer)
   // character that ends the line comment.
-
-  // C++23 [lex.phases] p1
-  // Diagnose invalid UTF-8 if the corresponding warning is enabled, emitting a
-  // diagnostic only once per entire ill-formed subsequence to avoid
-  // emiting to many diagnostics (see http://unicode.org/review/pr-121.html).
-  bool UnicodeDecodingAlreadyDiagnosed = false;
-
   char C;
   while (true) {
 C = *CurPtr;
 // Skip over characters in the fast loop.
-while (isASCII(C) && C != 0 &&   // Potentially EOF.
-   C != '\n' && C != '\r') { // Newline or DOS-style newline.
+while (C != 0 &&// Potentially EOF.
+   C != '\n' && C != '\r')  // Newline or DOS-style newline.
   C = *++CurPtr;
-  UnicodeDecodingAlreadyDiagnosed = false;
-}
-
-if (!isASCII(C)) {
-  unsigned Length = llvm::getUTF8SequenceSize(
-  (const llvm::UTF8 *)CurPtr, (const llvm::UTF8 *)BufferEnd);
-  if (Length == 0) {
-if (!UnicodeDecodingAlreadyDiagnosed && !isLexingRawMode())
-  Diag(CurPtr, diag::warn_invalid_utf8_in_comment);
-UnicodeDecodingAlreadyDiagnosed = true;
-++CurPtr;
-  } else {
-UnicodeDecodingAlreadyDiagnosed = false;
-CurPtr += Length;
-  }
-  continue;
-}
 
 const char *NextLine = CurPtr;
 if (C != 0) {
@@ -2689,12 +2665,6 @@ bool Lexer::SkipBlockComment(Token &Result, const char 
*CurPtr,
   if (C == '/')
 C = *CurPtr++;
 
-  // C++23 [lex.phases] p1
-  // Diagnose invalid UTF-8 if the corresponding warning is enabled, emitting a
-  // diagnostic only once per entire ill-formed subsequence to avoid
-  // emiting to many diagnostics (see http://unicode.org/review/pr-121.html).
-  bool UnicodeDecodingAlreadyDiagnosed = false;
-
   while (true) {
 // Skip over all non-interesting characters until we find end of buffer or 
a
 // (probably ending) '/' character.
@@ -2703,24 +2673,14 @@ bool Lexer::SkipBlockComment(Token &Result, const char 
*CurPtr,
 // doesn't check for '\0'.
 !(PP && PP->getCodeCompletionFileLoc() == FileLoc)) {
   // While not aligned to a 16-byte boundary.
-  while (C != '/' && (intptr_t)CurPtr % 16 != 0) {
-if (!isASCII(C)) {
-  CurPtr--;
-  goto MultiByteUTF8;
-

[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 442633.
ilya-biryukov added a comment.

- Always call PushToScopeChains. This is the right behavior after we stopped 
calling makeMergedDefinitionVisible


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128921

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Modules/merge-concepts-cxx-modules.cpp
  clang/test/Modules/merge-concepts.cpp

Index: clang/test/Modules/merge-concepts.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-concepts.cpp
@@ -0,0 +1,83 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -fmodules -fmodule-name=library \
+// RUN: -emit-module %t/modules.map \
+// RUN: -o %t/module.pcm
+//
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -fmodules -fmodule-file=%t/module.pcm  \
+// RUN: -fmodule-map-file=%t/modules.map \
+// RUN: -fsyntax-only -verify %t/use.cpp
+//
+//--- use.cpp
+
+#include "concepts.h"
+#include "conflicting.h"
+#include "format.h"
+
+template  void foo()
+  requires same_as
+{}
+ConflictingConcept auto x = 10; // expected-error {{reference to 'ConflictingConcept' is ambiguous}}
+// expected-note@* 2 {{candidate}}
+
+//--- modules.map
+module "library" {
+	export *
+	module "concepts" {
+		export *
+		header "concepts.h"
+	}
+	module "format" {
+		export *
+		header "format.h"
+	}
+	module "conflicting" {
+		export *
+		header "conflicting.h"
+	}
+}
+
+//--- concepts.h
+#ifndef SAMEAS_CONCEPTS_H_
+#define SAMEAS_CONCEPTS_H_
+
+#include "same_as.h"
+
+template 
+concept ConflictingConcept = true;
+
+#endif // SAMEAS_CONCEPTS_H
+
+//--- same_as.h
+#ifndef SAME_AS_H
+#define SAME_AS_H
+
+template 
+concept same_as = __is_same(T, U);
+
+#endif // SAME_AS_H
+
+//--- format.h
+#ifndef FORMAT_H
+#define FORMAT_H
+
+#include "concepts.h"
+#include "same_as.h"
+
+template  void foo()
+  requires same_as
+{}
+
+#endif // FORMAT_H
+
+//--- conflicting.h
+#ifndef CONFLICTING_H
+#define CONFLICTING_H
+
+template 
+concept ConflictingConcept = true;
+
+#endif // CONFLICTING_H
Index: clang/test/Modules/merge-concepts-cxx-modules.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-concepts-cxx-modules.cpp
@@ -0,0 +1,46 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/same_as.cppm -o %t/same_as.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/concepts.cppm -o %t/concepts.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/format.cppm -o %t/format.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/conflicting.cppm -o %t/conflicting.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use.cppm -fsyntax-only -verify
+
+//--- same_as.cppm
+export module same_as;
+export template 
+concept same_as = __is_same(T, U);
+
+//--- concepts.cppm
+export module concepts;
+export import same_as;
+
+export template 
+concept ConflictingConcept = true;
+
+//--- format.cppm
+
+export module format;
+export import concepts;
+export import same_as;
+
+export template  void foo()
+  requires same_as
+{}
+
+//--- conflicting.cppm
+export module conflicting;
+export template 
+concept ConflictingConcept = true;
+
+//--- Use.cppm
+import format;
+import conflicting;
+
+template  void foo()
+  requires same_as
+{}
+ConflictingConcept auto x = 10; // expected-error {{reference to 'ConflictingConcept' is ambiguous}}
+// expected-note@* 2 {{candidate}}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -8656,23 +8656,48 @@
   // Check for conflicting previous declaration.
   DeclarationNameInfo NameInfo(NewDecl->getDeclName(), NameLoc);
   LookupResult Previous(*this, NameInfo, LookupOrdinaryName,
-ForVisibleRedeclaration);
+forRedeclarationInCurContext());
   LookupName(Previous, S);
-
   FilterLookupForScope(Previous, DC, S, /*ConsiderLinkage=*/false,
/*AllowInlineNamespace*/false);
-  if (!Previous.empty()) {
-auto *Old = Previous.getRepresentativeDecl();
-Diag(NameLoc, isa(Old) ? diag::err_redefinition :
- diag::err_redefinition_different_kind) << NewDecl->getDeclName();
-Diag(Old->getLocation(), diag::note_previous_definition);
-  }
+  CheckConceptRedefinition(NewDecl, Previous);
 
   ActOnDocumentableDecl(NewDecl);
   PushOnScopeChains(NewDecl, S);
   return NewDecl;
 }
 
+void Sema::CheckConceptRedefinition(ConceptDecl *NewDecl,
+LookupRes

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D128059#3633343 , @thakis wrote:

> As far as I can tell, this breaks check-clang everywhere: http://45.33.8.238/
>
> Please take a look and revert for now if it takes a while to fix.

Thanks for letting me know, i hadn't noticed the bots screaming at me.  I did 
revert until i can investigate and fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128059

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

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Prabhdeep Soni via Phabricator via cfe-commits]

psoni2628 updated this revision to Diff 442637.
psoni2628 added a comment.

- Autogenerate check lines for test case
- Use isa instead of dyncast
- Remove unused DebugLoc
- Remove `llvm::` from `llvm::ConstantInt`


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

https://reviews.llvm.org/D129149

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/irbuilder_simd.cpp
  clang/test/OpenMP/irbuilder_simdlen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  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
@@ -1794,6 +1794,39 @@
   }));
 }
 
+TEST_F(OpenMPIRBuilderTest, ApplySimdlen) {
+  OpenMPIRBuilder OMPBuilder(*M);
+
+  CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
+
+  // Simd-ize the loop.
+  OMPBuilder.applySimd(DL, CLI);
+  OMPBuilder.applySimdlen(DL, CLI, ConstantInt::get(Type::getInt32Ty(Ctx), 3));
+
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+
+  PassBuilder PB;
+  FunctionAnalysisManager FAM;
+  PB.registerFunctionAnalyses(FAM);
+  LoopInfo &LI = FAM.getResult(*F);
+
+  const std::vector &TopLvl = LI.getTopLevelLoops();
+  EXPECT_EQ(TopLvl.size(), 1u);
+
+  Loop *L = TopLvl.front();
+  EXPECT_TRUE(findStringMetadataForLoop(L, "llvm.loop.parallel_accesses"));
+  EXPECT_TRUE(getBooleanLoopAttribute(L, "llvm.loop.vectorize.enable"));
+  EXPECT_EQ(getIntLoopAttribute(L, "llvm.loop.vectorize.width"), 3);
+
+  // Check for llvm.access.group metadata attached to the printf
+  // function in the loop body.
+  BasicBlock *LoopBody = CLI->getBody();
+  EXPECT_TRUE(any_of(*LoopBody, [](Instruction &I) {
+return I.getMetadata("llvm.access.group") != nullptr;
+  }));
+}
+
 TEST_F(OpenMPIRBuilderTest, UnrollLoopFull) {
   OpenMPIRBuilder OMPBuilder(*M);
 
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2881,6 +2881,15 @@
  BoolConst})});
 }
 
+void OpenMPIRBuilder::applySimdlen(CanonicalLoopInfo *CanonicalLoop,
+   ConstantInt *Simdlen) {
+  LLVMContext &Ctx = Builder.getContext();
+  addLoopMetadata(
+  CanonicalLoop,
+  MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.vectorize.width"),
+ConstantAsMetadata::get(Simdlen)}));
+}
+
 /// Create the TargetMachine object to query the backend for optimization
 /// preferences.
 ///
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -603,6 +603,12 @@
   /// \param Loop The loop to simd-ize.
   void applySimd(DebugLoc DL, CanonicalLoopInfo *Loop);
 
+  /// Add metadata for simdlen to a simd loop.
+  ///
+  /// \param LoopThe simd loop.
+  /// \param Simdlen The Simdlen length to apply to the simd loop.
+  void applySimdlen(CanonicalLoopInfo *Loop, llvm::ConstantInt *Simdlen);
+
   /// Generator for '#omp flush'
   ///
   /// \param Loc The location where the flush directive was encountered
Index: clang/test/OpenMP/irbuilder_simdlen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/irbuilder_simdlen.cpp
@@ -0,0 +1,125 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-enable-irbuilder -verify -fopenmp -fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+struct S {
+  int a, b;
+};
+
+struct P {
+  int a, b;
+};
+
+void simple(float *a, float *b, int *c) {
+  S s, *p;
+  P pp;
+#pragma omp simd simdlen(3)
+  for (int i = 3; i < 32; i += 5) {
+a[i] = b[i] + s.a + p->a;
+  }
+
+#pragma omp simd
+  for (int j = 3; j < 32; j += 5) {
+c[j] = pp.a;
+  }
+}
+
+// CHECK-LABEL: @_Z6simplePfS_Pi(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[A_ADDR:%.*]] = alloca float*, align 8
+// CHECK-NEXT:[[B_ADDR:%.*]] = alloca float*, align 8
+// CHECK-NEXT:[[C_ADDR:%.*]] = alloca i32*, align 8
+// CHECK-NEXT:[[S:%.*]] = alloca [[STRUCT_S:%.*]], align 4
+// CHECK-NEXT:[[P:%.*]] = alloca %struct.S*, align 8
+// CHECK-NEXT:[[PP:%.*]] = alloca [[STRUCT_P:%.*]], align 4
+// CHECK-NEXT:[[I:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[AGG_CAPTURED:%.*]] = alloca [[STRUCT_ANON:%.*]], align 8
+// CHECK-NEXT:[[AGG_CAPTURED1:%.*]] = alloca [[STRUCT_ANON_0:%.*]], align 4
+// CHECK-NEXT:[[DOTCOUNT_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:[[J:%.*]] = alloca i32

[PATCH] D129149: [OMPIRBuilder] Add support for simdlen clause

[Prabhdeep Soni via Phabricator via cfe-commits]

psoni2628 marked 8 inline comments as done.
psoni2628 added inline comments.



Comment at: clang/test/OpenMP/irbuilder_simdlen.cpp:1
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-enable-irbuilder -verify 
-fopenmp -fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm 
%s -o - | FileCheck %s
+// expected-no-diagnostics

jdoerfert wrote:
> The check lines look auto-generated and then modified by hand. Why is that?
I originally took `irbuilder_simd.cpp` and modified it to include `simdlen`.  I 
have now auto-generated the check lines.


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

https://reviews.llvm.org/D129149

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

[PATCH] D128927: [libc++] Always build c++experimental.a

[Mark de Wever via Phabricator via cfe-commits]

Mordante accepted this revision.
Mordante added a comment.

I had a look at the changes and still LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128927

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