[PATCH] D114124: [clang][ARM] only check -mtp=cp15 for non-asm sources

2021-12-03 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 391752.
nickdesaulniers added a comment.

- rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114124

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/clang-translation.c


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -122,12 +122,14 @@
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // RUN: %clang -target armv6-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
-// ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
-
 // RUN: %clang -target armv5t-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
+// RUN: %clang -target armv5t-linux -mtp=cp15 -x assembler -### %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv5_THREAD_POINTER_ASSEMBLER %s
+// ARMv5_THREAD_POINTER_ASSEMBLER-NOT: hardware TLS register is not supported 
for the armv5 sub-architecture
+
 // RUN: %clang -target armv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=THUMBv6_THREAD_POINTER_UNSUPP %s
 // RUN: %clang -target thumbv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -55,7 +55,7 @@
  llvm::Triple );
 bool isHardTPSupported(const llvm::Triple );
 ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList ,
- const llvm::Triple );
+ const llvm::Triple , bool ForAS);
 void setArchNameInTriple(const Driver , const llvm::opt::ArgList ,
  types::ID InputType, llvm::Triple );
 
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -159,14 +159,15 @@
 
 // Select mode for reading thread pointer (-mtp=soft/cp15).
 arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ,
-   const llvm::Triple ) {
+   const llvm::Triple , bool ForAS) {
   if (Arg *A = Args.getLastArg(options::OPT_mtp_mode_EQ)) {
 arm::ReadTPMode ThreadPointer =
 llvm::StringSwitch(A->getValue())
 .Case("cp15", ReadTPMode::Cp15)
 .Case("soft", ReadTPMode::Soft)
 .Default(ReadTPMode::Invalid);
-if (ThreadPointer == ReadTPMode::Cp15 && !isHardTPSupported(Triple)) {
+if (ThreadPointer == ReadTPMode::Cp15 && !isHardTPSupported(Triple) &&
+!ForAS) {
   D.Diag(diag::err_target_unsupported_tp_hard) << Triple.getArchName();
   return ReadTPMode::Invalid;
 }
@@ -488,7 +489,7 @@
 }
   }
 
-  if (getReadTPMode(D, Args, Triple) == ReadTPMode::Cp15)
+  if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::Cp15)
 Features.push_back("+read-tp-hard");
 
   const Arg *ArchArg = Args.getLastArg(options::OPT_march_EQ);


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -122,12 +122,14 @@
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // RUN: %clang -target armv6-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
-// ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
-
 // RUN: %clang -target armv5t-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
+// RUN: %clang -target armv5t-linux -mtp=cp15 -x assembler -### %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv5_THREAD_POINTER_ASSEMBLER %s
+// ARMv5_THREAD_POINTER_ASSEMBLER-NOT: hardware TLS register is not supported for the armv5 sub-architecture
+
 // RUN: %clang -target armv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=THUMBv6_THREAD_POINTER_UNSUPP %s
 // RUN: %clang -target thumbv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -55,7 +55,7 @@
  llvm::Triple );
 bool isHardTPSupported(const llvm::Triple );
 ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList ,
- const llvm::Triple 

[PATCH] D114124: [clang][ARM] only check -mtp=cp15 for non-asm sources

2021-12-03 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 391754.
nickdesaulniers edited the summary of this revision.
nickdesaulniers added a comment.

- update description


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114124

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/clang-translation.c


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -122,12 +122,14 @@
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // RUN: %clang -target armv6-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
-// ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
-
 // RUN: %clang -target armv5t-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
+// RUN: %clang -target armv5t-linux -mtp=cp15 -x assembler -### %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv5_THREAD_POINTER_ASSEMBLER %s
+// ARMv5_THREAD_POINTER_ASSEMBLER-NOT: hardware TLS register is not supported 
for the armv5 sub-architecture
+
 // RUN: %clang -target armv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=THUMBv6_THREAD_POINTER_UNSUPP %s
 // RUN: %clang -target thumbv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -55,7 +55,7 @@
  llvm::Triple );
 bool isHardTPSupported(const llvm::Triple );
 ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList ,
- const llvm::Triple );
+ const llvm::Triple , bool ForAS);
 void setArchNameInTriple(const Driver , const llvm::opt::ArgList ,
  types::ID InputType, llvm::Triple );
 
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -159,14 +159,15 @@
 
 // Select mode for reading thread pointer (-mtp=soft/cp15).
 arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ,
-   const llvm::Triple ) {
+   const llvm::Triple , bool ForAS) {
   if (Arg *A = Args.getLastArg(options::OPT_mtp_mode_EQ)) {
 arm::ReadTPMode ThreadPointer =
 llvm::StringSwitch(A->getValue())
 .Case("cp15", ReadTPMode::Cp15)
 .Case("soft", ReadTPMode::Soft)
 .Default(ReadTPMode::Invalid);
-if (ThreadPointer == ReadTPMode::Cp15 && !isHardTPSupported(Triple)) {
+if (ThreadPointer == ReadTPMode::Cp15 && !isHardTPSupported(Triple) &&
+!ForAS) {
   D.Diag(diag::err_target_unsupported_tp_hard) << Triple.getArchName();
   return ReadTPMode::Invalid;
 }
@@ -488,7 +489,7 @@
 }
   }
 
-  if (getReadTPMode(D, Args, Triple) == ReadTPMode::Cp15)
+  if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::Cp15)
 Features.push_back("+read-tp-hard");
 
   const Arg *ArchArg = Args.getLastArg(options::OPT_march_EQ);


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -122,12 +122,14 @@
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // RUN: %clang -target armv6-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
-// ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
-
 // RUN: %clang -target armv5t-linux -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
+// RUN: %clang -target armv5t-linux -mtp=cp15 -x assembler -### %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv5_THREAD_POINTER_ASSEMBLER %s
+// ARMv5_THREAD_POINTER_ASSEMBLER-NOT: hardware TLS register is not supported for the armv5 sub-architecture
+
 // RUN: %clang -target armv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=THUMBv6_THREAD_POINTER_UNSUPP %s
 // RUN: %clang -target thumbv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -55,7 +55,7 @@
  llvm::Triple );
 bool isHardTPSupported(const llvm::Triple );
 ReadTPMode getReadTPMode(const Driver , const 

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

No I don't think there is a performance penalty. **I** just don't like using 
pointers. I'm happy to drop this, if the Style says to use pointers. But from 
what I've seen in clang-format pointers are mostly used when it can be null, 
but there is a wild mix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115061

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


[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-03 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray added inline comments.



Comment at: clang/lib/AST/APValue.cpp:628-629
 
+static bool TryPrintAsStringLiteral(raw_ostream , const ArrayType *ATy,
+const APValue *Data, size_t Size) {
+  if (Size == 0)

rsmith wrote:
> Is there anything you can factor out of `StringLiteral::outputString` and 
> reuse here? At least the single-character printing code seems like something 
> we should try to not overly duplicate.
The goals of the two routines are different. `StringLiteral::outputString` must 
output something that can parse back as a string literal, even it doesn't 
decode UTF-8 right now; the new routine in this patch doesn't have to -- it can 
give up anytime it wants.

I wish `switch` statements were more composable. But if you don't mind, I can 
add a function in `CharInfo.h` to test and get the C-style escaped char and 
retire the outmost `switch` statements in those two routines.



Comment at: clang/lib/AST/APValue.cpp:637-639
+  // Nothing we can do about a sequence that is not null-terminated
+  if (!Data[--Size].getInt().isZero())
+return false;

rsmith wrote:
> We should drop all trailing zeroes here, because array initialization from a 
> string literal will reconstruct them.
Are you sure? `MyType<{""}>` and `MyType<{"\0\0\0"}>` are different types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

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


[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added a comment.

LGTM.




Comment at: clang/lib/Driver/ToolChain.cpp:545
+const char *ToolChain::getDefaultLinker() const {
+  if (StringRef(CLANG_DEFAULT_LINKER).empty())
+return "ld";

`CLANG_DEFAUALT_LINKER[0] == '\0'`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115045

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


[PATCH] D113319: [clang-format] Improve require and concept handling

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks marked 4 inline comments as done.
HazardyKnusperkeks added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3464-3483
+  * ``RCPS_TwoLines`` (in configuration: ``TwoLines``)
+The clause always gets its own line, and the content of the clause go
+into the next line with some indentation.
+
+.. code-block:: c++
+
+  template

Quuxplusone wrote:
> This option strikes me as actively harmful; I think you should remove it from 
> the PR completely. Nobody does this today and nobody should do it in the 
> future either.
I will never use it, so I have no strong opinion in that, because it was really 
easy to add. I will let others decide if we want to offer it.



Comment at: clang/include/clang/Format/Format.h:3130-3136
+  /// \brief The position of the ``requires`` clause for class templates.
+  /// \version 14
+  RequiresClausePositionStyle RequiresClausePositionForClasses;
+
+  /// \brief The position of the ``requires`` clause for function templates.
+  /// \version 14
+  RequiresClausePositionStyle RequiresClausePositionForFunctions;

Quuxplusone wrote:
> What about for variable templates? What about for alias templates? What about 
> for deduction guides?
> It makes sense to me to have //one// option for this. It doesn't make sense 
> to have 2, 3, or 4. Having 5 feels insane. So, I think you should just have 
> one option that applies consistently to all `requires`-clauses everywhere in 
> the code.
That makes sense, in my defense I thought about it and came to the conclusion 
there would be no need for requires on variable templates, and the others I did 
not think of.

Why I have chosen to options is maybe someone wants
```template 
requires ...
class ...```

But also
```template 
void foo(T) requires ... {
```

What's your opinion on that?



Comment at: clang/unittests/Format/FormatTest.cpp:22288-22291
+  verifyFormat("template \n"
+   "concept C = ((false || foo()) && C2) || "
+   "(std::trait::value && Baz) ||\n "
+   "   sizeof(T) >= 6;");

Quuxplusone wrote:
> Nit: I was initially very confused by the formatting here, until I realized 
> that lines 22289 and 22290 are two lines //physically// but represent one 
> single line in the //test case//. Strongly recommend eliminating the 
> gratuitous line break.
I'm not really happy wit the two solutions which come to my mind:
Using a Style with a lower column limit, or using // clang format off.



Comment at: clang/unittests/Format/FormatTest.cpp:22374-22378
+  "template \n"
+  "concept C = decltype([]() { return std::true_type{}; }())::value &&\n"
+  "requires(T t) {\n"
+  "  t.bar();\n"
+  "} && sizeof(T) <= 8;");

Quuxplusone wrote:
> This looks wrong. Current:
> ```
> template \n"
> concept C = decltype([]() { return std::true_type{}; }())::value &&
> requires(T t) {
>   t.bar();
> } && sizeof(T) <= 8;
> ```
> but should be:
> ```
> template \n"
> concept C = decltype([]() { return std::true_type{}; }())::value &&
>   requires(T t) {
> t.bar();
>   } && sizeof(T) <= 8;
> ```
> (assuming that all the relevant indent-widths are set to `2`).
For me personally it should look
```template \n"
concept C = decltype([]() { return std::true_type{}; }())::value &&
requires(T t) {
  t.bar();
} && sizeof(T) <= 8;```

I have not taken any action to manipulate the position, this is what dropped 
out of clang-format naturally. And I think the requires should start below the 
decltype the same as all other conditions would. If the body should be indented 
is open for discussion.



Comment at: clang/unittests/Format/FormatTest.cpp:22597
+  verifyFormat("template \n"
+ "requires(std::invocable...>)\n"
+ "struct constant;",

Quuxplusone wrote:
> Currently:
> ```
> template 
> requires(std::invocable...>)
> struct constant;
> ```
> Should be:
> ```
> template 
> requires (std::invocable...>)
> struct constant;
> ```
> (notice the extra single space). Add a TODO comment and ship it anyway?
Take a look at D113369 which will most likely land simultaneously with this 
change, but currently needs to be updated.



Comment at: clang/unittests/Format/FormatTest.cpp:22643
   verifyFormat("template \n"
-   "concept Context = Traits or\n"
-   "Interface or\n"
-   "Request or\n"
-   "Response or\n"
-   "ContextExtension or\n"
-   "::std::is_copy_constructable or "
-   "::std::is_move_constructable or\n"
-   "requires (T c) {\n"
-   "  { c.response; } -> Response;\n"
-   "} or requires (T c) {\n"
-   "  { c.request; } -> Request;\n"
-   

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-03 Thread Owen Pan via Phabricator via cfe-commits
owenpan accepted this revision.
owenpan added a comment.
This revision is now accepted and ready to land.

In D115061#3170908 , 
@HazardyKnusperkeks wrote:

> No I don't think there is a performance penalty. **I** just don't like using 
> pointers. I'm happy to drop this, if the Style says to use pointers. But from 
> what I've seen in clang-format pointers are mostly used when it can be null, 
> but there is a wild mix.

I don't think LLVM Coding Standards specify it one way or the other, so it LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115061

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


[PATCH] D109751: [Clang] Support conversion between PPC double-double and IEEE float128

2021-12-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:1540
   // Diagnose attempts to convert between __ibm128, __float128 and long double
-  // where such conversions currently can't be handled.
+  // in arithmetic or comparison.
   if (unsupportedTypeConversion(*this, LHSType, RHSType))

The ternary operator seems to be something other than (straightforward) 
"arithmetic or comparison".



Comment at: clang/test/CodeGen/ibm128-cast.c:46
   !b ? w : l; // expected-error {{incompatible operand types ('__ibm128' and 
'long double')}}
 }
 #elif __LONG_DOUBLE_IBM128__

Should have compound assignment tests for C as well.



Comment at: clang/test/CodeGen/ibm128-cast.c:67
+  !b ? w : l; // expected-no-error
 }
 #endif

Should have compound assignment tests for C as well.



Comment at: clang/test/Sema/float128-ld-incompatibility.cpp:13
+long double ld{qf()}; // expected-error {{non-constant-expression cannot be 
narrowed from type '__float128' to 'long double' in initializer list}} 
expected-note {{insert an explicit cast to silence this issue}}
+__float128 q{ldf()}; // expected-no-error
 

Should also test `__ibm128` cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109751

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


[PATCH] D109751: [Clang] Support conversion between PPC double-double and IEEE float128

2021-12-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/test/Sema/float128-ld-incompatibility.cpp:13
+long double ld{qf()}; // expected-error {{non-constant-expression cannot be 
narrowed from type '__float128' to 'long double' in initializer list}} 
expected-note {{insert an explicit cast to silence this issue}}
+__float128 q{ldf()}; // expected-no-error
 

hubert.reinterpretcast wrote:
> Should also test `__ibm128` cases.
The C++ committee has advised that this implicit conversion should be 
considered ill-formed (see other comment).

Note that the //allowed// implicit conversion from `__ibm128` to `long double` 
(and vice versa) is still a conversion, which means that overload resolution is 
still a problem:
```
void f(__ibm128);
void f(int);
void g(long double d) { return f(d); } // okay with GCC but not Clang; 
https://godbolt.org/z/fonsEbbY1
```



Comment at: clang/test/Sema/float128-ld-incompatibility.cpp:36-37
   q / ld; // expected-error {{invalid operands to binary expression 
('__float128' and 'long double')}}
-  ld = q; // expected-error {{assigning to 'long double' from incompatible 
type '__float128'}}
-  q = ld; // expected-error {{assigning to '__float128' from incompatible type 
'long double'}}
+  ld = q; // expected-no-error {{assigning to 'long double' from incompatible 
type '__float128'}}
+  q = ld; // expected-no-error {{assigning to '__float128' from incompatible 
type 'long double'}}
   q + b ? q : ld; // expected-error {{incompatible operand types ('__float128' 
and 'long double')}}

The C++ committee has advised that these are not okay as implicit conversions:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1467r7.html#implicit

Additional lines testing `static_cast` would be appropriate.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109751

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


[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-03 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray added inline comments.



Comment at: clang/lib/AST/APValue.cpp:637-639
+  // Nothing we can do about a sequence that is not null-terminated
+  if (!Data[--Size].getInt().isZero())
+return false;

lichray wrote:
> rsmith wrote:
> > lichray wrote:
> > > rsmith wrote:
> > > > We should drop all trailing zeroes here, because array initialization 
> > > > from a string literal will reconstruct them.
> > > Are you sure? `MyType<{""}>` and `MyType<{"\0\0\0"}>` are different types.
> > That's a separate bug. `APValue`'s printing is not intended to identify the 
> > type of the result, only to identify the value in the case where we already 
> > know the type. Eg, we don't print a type suffix on a pretty-printed integer 
> > value. For the specific case of template parameters, it's the 
> > responsibility of the template argument printing code to uniquely identify 
> > the template argument.
> > 
> > When a template parameter has a deduced class type, we should include that 
> > type in the printed output in order to uniquely identify the 
> > specialization, but we don't, because that's not been implemented yet. When 
> > that's fixed, we should print those cases as `MyType{""}>` and 
> > `MyType{""}>`. This is necessary anyway, because we can't in 
> > general assume that the resulting value is enough to indicate what type 
> > CTAD will have selected.
> > 
> > We already handle this properly in some cases, but see this FIXMEs: 
> > https://github.com/llvm/llvm-project/blob/6c2be3015e07f25912b8cd5b75214c532f568638/clang/lib/AST/TemplateBase.cpp#L433
> > 
> > See also some related issues: https://godbolt.org/z/YhcdMYx6n (note that 
> > the non-struct enum case is handled properly but the enum-in-struct case is 
> > not).
> > That's a separate bug. [...]
> > 
> > When a template parameter has a deduced class type, we should include that 
> > type in the printed output in order to uniquely identify the 
> > specialization, but we don't, because that's not been implemented yet. When 
> > that's fixed, we should print those cases as `MyType{""}>` and 
> > `MyType{""}>`. This is necessary anyway, because we can't in 
> > general assume that the resulting value is enough to indicate what type 
> > CTAD will have selected.
> >
> 
> But it looks like a feature rather than a bug? String literals provided both 
> type and value to emphasis a structural type's value for equivalence 
> comparison. The reason of why `MyType<{""}>` and `MyType<"\0\0\0">` are 
> different looks more obvious to me.
> 
> > See also some related issues: https://godbolt.org/z/YhcdMYx6n (note that 
> > the non-struct enum case is handled properly but the enum-in-struct case is 
> > not).
> 
> I'm fine if a specific NTTP, `A`, is treated differently from `auto`. But the 
> last case is similar to this https://godbolt.org/z/YcscTrchM Which I thought 
> about printing something like `Y<{(E[]){97, 98}}>` :)
That discussion looks OT... For now, not shrinking `"\0\0\0"` down to `""` 
follows the existing logic, i.e., we are printing `{0, 0, 0}` even though it's 
an array.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

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


[PATCH] D115049: Fall back on Android triple w/o API level for runtimes search

2021-12-03 Thread Collin Baker via Phabricator via cfe-commits
collinbaker updated this revision to Diff 391661.
collinbaker added a comment.

Fix variable name style


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115049

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-unknown-linux-android/.keep
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-unknown-linux-android/libclang_rt.builtins.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-unknown-linux-android21/.keep
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-unknown-linux-android21/libclang_rt.builtins.a
  clang/test/Driver/linux-per-target-runtime-dir.c

Index: clang/test/Driver/linux-per-target-runtime-dir.c
===
--- clang/test/Driver/linux-per-target-runtime-dir.c
+++ clang/test/Driver/linux-per-target-runtime-dir.c
@@ -25,3 +25,21 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664 %s
 // CHECK-FILE-NAME-X8664: lib{{/|\\}}x86_64-unknown-linux-gnu{{/|\\}}libclang_rt.builtins.a
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=aarch64-unknown-linux-android21 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID21 %s
+// CHECK-FILE-NAME-ANDROID21: lib{{/|\\}}aarch64-unknown-linux-android21{{/|\\}}libclang_rt.builtins.a
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=aarch64-unknown-linux-android23 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID23 %s
+// CHECK-FILE-NAME-ANDROID23: lib{{/|\\}}aarch64-unknown-linux-android{{/|\\}}libclang_rt.builtins.a
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=aarch64-unknown-linux-android \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID %s
+// CHECK-FILE-NAME-ANDROID: lib{{/|\\}}aarch64-unknown-linux-android{{/|\\}}libclang_rt.builtins.a
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -191,9 +191,11 @@
 
   auto FilePaths = [&](const Multilib ) -> std::vector {
 std::vector FP;
-SmallString<128> P(getStdlibPath());
-llvm::sys::path::append(P, M.gccSuffix());
-FP.push_back(std::string(P.str()));
+for (const std::string  : getStdlibPaths()) {
+  SmallString<128> P(Path);
+  llvm::sys::path::append(P, M.gccSuffix());
+  FP.push_back(std::string(P.str()));
+}
 return FP;
   };
 
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -75,17 +75,16 @@
  const ArgList )
 : D(D), Triple(T), Args(Args), CachedRTTIArg(GetRTTIArgument(Args)),
   CachedRTTIMode(CalculateRTTIMode(Args, Triple, CachedRTTIArg)) {
-  std::string RuntimePath = getRuntimePath();
-  if (getVFS().exists(RuntimePath))
-getLibraryPaths().push_back(RuntimePath);
-
-  std::string StdlibPath = getStdlibPath();
-  if (getVFS().exists(StdlibPath))
-getFilePaths().push_back(StdlibPath);
+  auto addIfExists = [this](path_list , const std::string ) {
+if (getVFS().exists(Path))
+  List.push_back(Path);
+  };
 
-  std::string CandidateLibPath = getArchSpecificLibPath();
-  if (getVFS().exists(CandidateLibPath))
-getFilePaths().push_back(CandidateLibPath);
+  for (const auto  : getRuntimePaths())
+addIfExists(getLibraryPaths(), Path);
+  for (const auto  : getStdlibPaths())
+addIfExists(getFilePaths(), Path);
+  addIfExists(getFilePaths(), getArchSpecificLibPath());
 }
 
 void ToolChain::setTripleEnvironment(llvm::Triple::EnvironmentType Env) {
@@ -485,16 +484,36 @@
   return Args.MakeArgString(getCompilerRT(Args, Component, Type));
 }
 
-std::string ToolChain::getRuntimePath() const {
-  SmallString<128> P(D.ResourceDir);
-  llvm::sys::path::append(P, "lib", getTripleString());
-  return std::string(P.str());
+ToolChain::path_list ToolChain::getRuntimePaths() const {
+  path_list Paths;
+  auto addPathForTriple = [this, ](const llvm::Triple ) {
+SmallString<128> P(D.ResourceDir);
+llvm::sys::path::append(P, "lib", Triple.str());
+Paths.push_back(std::string(P.str()));
+  };
+
+  addPathForTriple(getTriple());
+
+  // 

[PATCH] D115015: CodeGen: Strip exception specifications from function types in CFI type names.

2021-12-03 Thread Nico Weber via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0a14674f276b: CodeGen: Strip exception specifications from 
function types in CFI type names. (authored by pcc, committed by thakis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115015

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/cfi-icall-noexcept.cpp


Index: clang/test/CodeGenCXX/cfi-icall-noexcept.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cfi-icall-noexcept.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall 
-emit-llvm -std=c++17 -o - %s | FileCheck %s
+
+// Tests that exception specifiers are stripped when forming the
+// mangled CFI type name.
+
+void f() noexcept {}
+
+// CHECK: define{{.*}} void @_Z1fv({{.*}} !type [[TS1:![0-9]+]] !type 
[[TS2:![0-9]+]]
+
+// CHECK: [[TS1]] = !{i64 0, !"_ZTSFvvE"}
+// CHECK: [[TS2]] = !{i64 0, !"_ZTSFvvE.generalized"}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6398,6 +6398,11 @@
 llvm::Metadata *
 CodeGenModule::CreateMetadataIdentifierImpl(QualType T, MetadataTypeMap ,
 StringRef Suffix) {
+  if (auto *FnType = T->getAs())
+T = getContext().getFunctionType(
+FnType->getReturnType(), FnType->getParamTypes(),
+FnType->getExtProtoInfo().withExceptionSpec(EST_None));
+
   llvm::Metadata * = Map[T.getCanonicalType()];
   if (InternalId)
 return InternalId;


Index: clang/test/CodeGenCXX/cfi-icall-noexcept.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cfi-icall-noexcept.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -emit-llvm -std=c++17 -o - %s | FileCheck %s
+
+// Tests that exception specifiers are stripped when forming the
+// mangled CFI type name.
+
+void f() noexcept {}
+
+// CHECK: define{{.*}} void @_Z1fv({{.*}} !type [[TS1:![0-9]+]] !type [[TS2:![0-9]+]]
+
+// CHECK: [[TS1]] = !{i64 0, !"_ZTSFvvE"}
+// CHECK: [[TS2]] = !{i64 0, !"_ZTSFvvE.generalized"}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -6398,6 +6398,11 @@
 llvm::Metadata *
 CodeGenModule::CreateMetadataIdentifierImpl(QualType T, MetadataTypeMap ,
 StringRef Suffix) {
+  if (auto *FnType = T->getAs())
+T = getContext().getFunctionType(
+FnType->getReturnType(), FnType->getParamTypes(),
+FnType->getExtProtoInfo().withExceptionSpec(EST_None));
+
   llvm::Metadata * = Map[T.getCanonicalType()];
   if (InternalId)
 return InternalId;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115063: [clang-format][NFC] Rename variable so no shadowing happens

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, owenpan, curdeius.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In the loop there is also a Node.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115063

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1017,9 +1017,9 @@
 QueueType Queue;
 
 // Insert start element into queue.
-StateNode *Node =
+StateNode *RootNode =
 new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
-Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
+Queue.push(QueueItem(OrderedPenalty(0, Count), RootNode));
 ++Count;
 
 unsigned Penalty = 0;


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1017,9 +1017,9 @@
 QueueType Queue;
 
 // Insert start element into queue.
-StateNode *Node =
+StateNode *RootNode =
 new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
-Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
+Queue.push(QueueItem(OrderedPenalty(0, Count), RootNode));
 ++Count;
 
 unsigned Penalty = 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115069: [clang-format][NFC] Merge another two calls to isOneOf

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius, owenpan.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks 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/D115069

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1291,8 +1291,8 @@
 State.Stack[State.Stack.size() - 2].NestedBlockInlined = false;
   }
   if (Previous &&
-  (Previous->isOneOf(tok::l_paren, tok::comma, tok::colon) ||
-   Previous->isOneOf(TT_BinaryOperator, TT_ConditionalExpr)) &&
+  Previous->isOneOf(tok::l_paren, tok::comma, tok::colon, 
TT_BinaryOperator,
+TT_ConditionalExpr) &&
   !Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
 State.Stack.back().NestedBlockInlined =
 !Newline && hasNestedBlockInlined(Previous, Current, Style);


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1291,8 +1291,8 @@
 State.Stack[State.Stack.size() - 2].NestedBlockInlined = false;
   }
   if (Previous &&
-  (Previous->isOneOf(tok::l_paren, tok::comma, tok::colon) ||
-   Previous->isOneOf(TT_BinaryOperator, TT_ConditionalExpr)) &&
+  Previous->isOneOf(tok::l_paren, tok::comma, tok::colon, TT_BinaryOperator,
+TT_ConditionalExpr) &&
   !Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
 State.Stack.back().NestedBlockInlined =
 !Newline && hasNestedBlockInlined(Previous, Current, Style);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115063: [clang-format][NFC] Rename variable so no shadowing happens

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115063

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


[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-03 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray updated this revision to Diff 391744.
lichray added a comment.

Ensure the ellipses output is never shorter than the normal ones (will look at 
review comments later)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

Files:
  clang/lib/AST/APValue.cpp
  clang/test/SemaTemplate/temp_arg_string_printing.cpp

Index: clang/test/SemaTemplate/temp_arg_string_printing.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_string_printing.cpp
@@ -0,0 +1,137 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -ast-print %s | FileCheck %s
+
+using size_t = __SIZE_TYPE__;
+static_assert(__has_builtin(__make_integer_seq));
+
+template  class idx_seq {};
+template  using make_idx_seq = __make_integer_seq;
+
+template 
+struct Str {
+  constexpr Str(CharT const ()[N]) : Str(s, make_idx_seq()) {}
+  CharT value[N];
+
+private:
+  template 
+  constexpr Str(CharT const ()[N], idx_seq) : value{s[I]...} {}
+};
+
+template  class ASCII {};
+
+void narrow() {
+  // CHECK{LITERAL}: ASCII<{""}>
+  new ASCII<"">;
+  // CHECK{LITERAL}: ASCII<{"the quick brown fox jumps"}>
+  new ASCII<"the quick brown fox jumps">;
+  // CHECK{LITERAL}: ASCII<{"OVER THE LAZY DOG 0123456789"}>
+  new ASCII<"OVER THE LAZY DOG 0123456789">;
+  // CHECK{LITERAL}: ASCII<{"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{"escape\0"}>
+  new ASCII<"escape\0">;
+  // CHECK{LITERAL}: ASCII<{"escape\r\n"}>
+  new ASCII<"escape\r\n">;
+  // CHECK{LITERAL}: ASCII<{"escape\\\t\'\f\v"}>
+  new ASCII<"escape\\\t'\f\v">;
+  // CHECK{LITERAL}: ASCII<{"escape\a\bc"}>
+  new ASCII<"escape\a\b\c">;
+  // CHECK{LITERAL}: ASCII<{{110, 111, 116, 17, 0}}>
+  new ASCII<"not\x11">;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 127, 16, 1, 32, 97, 98, 99, 0}}>
+  new ASCII<"\x12\x14\x7f\x10\x01 abc">;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 127, 16, 1, 32, 97, 98, 99, 100, ...}}>
+  new ASCII<"\x12\x14\x7f\x10\x01 abcd">;
+  // CHECK{LITERAL}: ASCII<{"print more characters as string"}>
+  new ASCII<"print more characters as string">;
+  // CHECK{LITERAL}: ASCII<{"print even more characters as string"}>
+  new ASCII<"print even more characters as string">;
+  // CHECK{LITERAL}: ASCII<{"print many characters no more than[...]"}>
+  new ASCII<"print many characters no more than a limit">;
+  // CHECK{LITERAL}: ASCII<{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"}>
+  new ASCII<"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0">;
+  // CHECK{LITERAL}: ASCII<{"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n[...]"}>
+  new ASCII<"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n">;
+}
+
+void wide() {
+  // CHECK{LITERAL}: ASCII<{L""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"the quick brown fox jumps"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"OVER THE LAZY DOG 0123456789"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{L"escape\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\\\t\f\v"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\a\bc"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{110, 111, 116, 17, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 255, 22909, 136, 32, 97, 98, 99, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 255, 22909, 136, 32, 97, 98, 99, 100, ...}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print more characters as string"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print even more characters as string"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print many characters no more than[...]"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n[...]"}>
+  new ASCII;
+}
+
+void utf8()
+{
+  // CHECK{LITERAL}: ASCII<{u8""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{u8"escape\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{229, 165, 189, 239, 191, 189, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"print many characters no more than[...]"}>
+  new ASCII;
+}
+
+void utf16()
+{
+  // CHECK{LITERAL}: ASCII<{u""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{u"escape\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{22909, 65533, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u"print many characters no more 

Maintenance works at llvm lab today at 6PM PST

2021-12-03 Thread Galina Kistanova via cfe-commits
Hello everyone,

LLVM lab and build bot will be unavailable for about an hour starting from
6:00 PM PST today for maintenance works.

Thanks

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


[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

2021-12-03 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

How about naming `I[-1]` as well? And maybe `I[i + 1]` too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115060

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


Re: Maintenance works at llvm lab today at 7PM PST

2021-12-03 Thread Galina Kistanova via cfe-commits
LLVM lab and build bot are back to normal work.

Thanks

Galina

On Fri, Oct 1, 2021 at 4:50 PM Galina Kistanova 
wrote:

> Hello everyone,
>
> LLVM lab and build bot will be unavailable for about an hour starting from
> 7:00 PM PST today for some maintenance work.
>
> Thank you for understanding.
>
> Galina
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/AST/APValue.cpp:637-639
+  // Nothing we can do about a sequence that is not null-terminated
+  if (!Data[--Size].getInt().isZero())
+return false;

lichray wrote:
> rsmith wrote:
> > We should drop all trailing zeroes here, because array initialization from 
> > a string literal will reconstruct them.
> Are you sure? `MyType<{""}>` and `MyType<{"\0\0\0"}>` are different types.
That's a separate bug. `APValue`'s printing is not intended to identify the 
type of the result, only to identify the value in the case where we already 
know the type. Eg, we don't print a type suffix on a pretty-printed integer 
value. For the specific case of template parameters, it's the responsibility of 
the template argument printing code to uniquely identify the template argument.

When a template parameter has a deduced class type, we should include that type 
in the printed output in order to uniquely identify the specialization, but we 
don't, because that's not been implemented yet. When that's fixed, we should 
print those cases as `MyType{""}>` and `MyType{""}>`. 
This is necessary anyway, because we can't in general assume that the resulting 
value is enough to indicate what type CTAD will have selected.

We already handle this properly in some cases, but see this FIXMEs: 
https://github.com/llvm/llvm-project/blob/6c2be3015e07f25912b8cd5b75214c532f568638/clang/lib/AST/TemplateBase.cpp#L433

See also some related issues: https://godbolt.org/z/YhcdMYx6n (note that the 
non-struct enum case is handled properly but the enum-in-struct case is not).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

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


[PATCH] D114565: [InstrProf] Attach debug info to counters

2021-12-03 Thread Ellis Hoag via Phabricator via cfe-commits
ellis added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:961
+  DB.finalize();
+}
+  }

kyulee wrote:
> I think when `-debug-info-correlate` is used without debug info, we should 
> fail or emit a warning here instead of silently proceeding it because we 
> cannot correlate it anyhow down the road.
I'll emit a warning. I don't want to fail in this case when a single function 
doesn't have debug info. Instead, I'm planning on adding a check in the 
frontend when parsing the `-fprofile-generate-correlate=debug-info` flag (which 
I will add later).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114565

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


[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/AST/APValue.cpp:628-629
 
+static bool TryPrintAsStringLiteral(raw_ostream , const ArrayType *ATy,
+const APValue *Data, size_t Size) {
+  if (Size == 0)

Is there anything you can factor out of `StringLiteral::outputString` and reuse 
here? At least the single-character printing code seems like something we 
should try to not overly duplicate.



Comment at: clang/lib/AST/APValue.cpp:637-639
+  // Nothing we can do about a sequence that is not null-terminated
+  if (!Data[--Size].getInt().isZero())
+return false;

We should drop all trailing zeroes here, because array initialization from a 
string literal will reconstruct them.



Comment at: clang/lib/AST/APValue.cpp:641-643
+  constexpr size_t MaxN = 36;
+  char Buf[MaxN * 2 + 3] = {'"'}; // "At most 36 escaped chars" + \0
+  auto *pBuf = Buf + 1;

Prefer `llvm::SmallString` and `push_back`.



Comment at: clang/lib/AST/APValue.cpp:645
+
+  // Better than printing a two-digit sequence of 10 integers.
+  StringRef Ellipsis;

...except that some callers want output that uniquely identifies the template 
argument, and moreover some callers want output that is valid C++ code that 
produces the same template-id.

It'd be better to do this behind a `PrintingPolicy` flag that the diagnostics 
engine sets. But we'd want to turn that flag off during template type diffing.



Comment at: clang/lib/AST/APValue.cpp:902
 if (I == 10) {
   // Avoid printing out the entire contents of large arrays.
+  Out << "...}";

(The existing code is also doing elision here that will be inappropriate for 
some callers. If you add a `PrintingPolicy` to unambiguously print template 
arguments, it should disable this check too.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

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


[PATCH] D115069: [clang-format][NFC] Merge another two calls to isOneOf

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1294-1296
+  Previous->isOneOf(tok::l_paren, tok::comma, tok::colon, 
TT_BinaryOperator,
+TT_ConditionalExpr) &&
   !Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {

curdeius wrote:
> Unrelated to your change, it only shed light on this:
> This condition looks strange to me. How can `Previous` be at the same time 
> two different `TokenType`s (?), for instance, both `TT_BinaryOperator` and 
> `TT_DictLiteral`?
> 
> Shouldn't it be this?
Yeah, I will update it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115069

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


[PATCH] D114124: [clang][ARM] only check -mtp=cp15 for non-asm sources

2021-12-03 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 391755.
nickdesaulniers added a comment.

- try to fix rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114124

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/clang-translation.c


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -126,6 +126,10 @@
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
+// RUN: %clang -target armv5t-linux -mtp=cp15 -x assembler -### %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv5_THREAD_POINTER_ASSEMBLER %s
+// ARMv5_THREAD_POINTER_ASSEMBLER-NOT: hardware TLS register is not supported 
for the armv5 sub-architecture
+
 // RUN: %clang -target armv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=THUMBv6_THREAD_POINTER_UNSUPP %s
 // RUN: %clang -target thumbv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -55,7 +55,7 @@
  llvm::Triple );
 bool isHardTPSupported(const llvm::Triple );
 ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList ,
- const llvm::Triple );
+ const llvm::Triple , bool ForAS);
 void setArchNameInTriple(const Driver , const llvm::opt::ArgList ,
  types::ID InputType, llvm::Triple );
 
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -159,14 +159,15 @@
 
 // Select mode for reading thread pointer (-mtp=soft/cp15).
 arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ,
-   const llvm::Triple ) {
+   const llvm::Triple , bool ForAS) {
   if (Arg *A = Args.getLastArg(options::OPT_mtp_mode_EQ)) {
 arm::ReadTPMode ThreadPointer =
 llvm::StringSwitch(A->getValue())
 .Case("cp15", ReadTPMode::Cp15)
 .Case("soft", ReadTPMode::Soft)
 .Default(ReadTPMode::Invalid);
-if (ThreadPointer == ReadTPMode::Cp15 && !isHardTPSupported(Triple)) {
+if (ThreadPointer == ReadTPMode::Cp15 && !isHardTPSupported(Triple) &&
+!ForAS) {
   D.Diag(diag::err_target_unsupported_tp_hard) << Triple.getArchName();
   return ReadTPMode::Invalid;
 }
@@ -488,7 +489,7 @@
 }
   }
 
-  if (getReadTPMode(D, Args, Triple) == ReadTPMode::Cp15)
+  if (getReadTPMode(D, Args, Triple, ForAS) == ReadTPMode::Cp15)
 Features.push_back("+read-tp-hard");
 
   const Arg *ArchArg = Args.getLastArg(options::OPT_march_EQ);


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -126,6 +126,10 @@
 // RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
+// RUN: %clang -target armv5t-linux -mtp=cp15 -x assembler -### %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARMv5_THREAD_POINTER_ASSEMBLER %s
+// ARMv5_THREAD_POINTER_ASSEMBLER-NOT: hardware TLS register is not supported for the armv5 sub-architecture
+
 // RUN: %clang -target armv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=THUMBv6_THREAD_POINTER_UNSUPP %s
 // RUN: %clang -target thumbv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -55,7 +55,7 @@
  llvm::Triple );
 bool isHardTPSupported(const llvm::Triple );
 ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList ,
- const llvm::Triple );
+ const llvm::Triple , bool ForAS);
 void setArchNameInTriple(const Driver , const llvm::opt::ArgList ,
  types::ID InputType, llvm::Triple );
 
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -159,14 +159,15 @@
 
 // Select mode for reading thread pointer (-mtp=soft/cp15).
 arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ,
-   const llvm::Triple ) {
+   

[PATCH] D114565: [InstrProf] Attach debug info to counters

2021-12-03 Thread Ellis Hoag via Phabricator via cfe-commits
ellis updated this revision to Diff 391780.
ellis added a comment.

Add warning when debug info is missing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114565

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  compiler-rt/include/profile/InstrProfData.inc
  compiler-rt/lib/profile/InstrProfiling.c
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/lib/profile/InstrProfilingWriter.c
  llvm/include/llvm/ProfileData/InstrProf.h
  llvm/include/llvm/ProfileData/InstrProfData.inc
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/InstrProfWriter.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll

Index: llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/InstrProfiling/debug-info-correlate.ll
@@ -0,0 +1,74 @@
+; RUN: opt < %s -instrprof -debug-info-correlate -S > %t.ll
+; RUN: llc < %t.ll --filetype=asm > %t.s
+; RUN: llc < %t.ll --filetype=obj > %t.o
+; RUN: FileCheck < %t.ll %s
+; RUN: llvm-dwarfdump %t.o | FileCheck %s --implicit-check-not "{{DW_TAG|NULL}}" --check-prefix CHECK-DWARF
+; RUN: FileCheck < %t.s %s --check-prefix CHECK-ASM
+
+target triple = "aarch64-unknown-linux-gnu"
+
+@__profn_foo = private constant [3 x i8] c"foo"
+; CHECK:  @__profc_foo =
+; CHECK-SAME: !dbg ![[EXPR:[0-9]+]]
+
+; CHECK:  ![[EXPR]] = !DIGlobalVariableExpression(var: ![[GLOBAL:[0-9]+]]
+; CHECK:  ![[GLOBAL]] = {{.*}} !DIGlobalVariable(name: "__profc_foo"
+; CHECK-SAME: scope: ![[SCOPE:[0-9]+]]
+; CHECK-SAME: annotations: ![[ANNOTATIONS:[0-9]+]]
+; CHECK:  ![[SCOPE]] = {{.*}} !DISubprogram(name: "foo"
+; CHECK:  ![[ANNOTATIONS]] = !{![[NAME:[0-9]+]], ![[HASH:[0-9]+]], ![[COUNTERS:[0-9]+]]}
+; CHECK:  ![[NAME]] = !{!"Function Name", !"foo"}
+; CHECK:  ![[HASH]] = !{!"CFG Hash", i64 12345678}
+; CHECK:  ![[COUNTERS]] = !{!"Num Counters", i32 2}
+
+define void @_Z3foov() !dbg !12 {
+  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
+}
+
+declare void @llvm.instrprof.increment(i8*, i64, i32, i32)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 1, !"branch-target-enforcement", i32 0}
+!6 = !{i32 1, !"sign-return-address", i32 0}
+!7 = !{i32 1, !"sign-return-address-all", i32 0}
+!8 = !{i32 1, !"sign-return-address-with-bkey", i32 0}
+!9 = !{i32 7, !"uwtable", i32 1}
+!10 = !{i32 7, !"frame-pointer", i32 1}
+!11 = !{!"clang version 14.0.0"}
+!12 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !13, file: !13, line: 1, type: !14, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !16)
+!13 = !DIFile(filename: "debug-info-correlate.cpp", directory: "")
+!14 = !DISubroutineType(types: !15)
+!15 = !{null}
+!16 = !{}
+
+; CHECK-DWARF: DW_TAG_compile_unit
+; CHECK-DWARF:   DW_TAG_subprogram
+; CHECK-DWARF: DW_AT_name	("foo")
+; CHECK-DWARF: DW_TAG_variable
+; CHECK-DWARF:   DW_AT_name	("__profc_foo")
+; CHECK-DWARF:   DW_AT_type	({{.*}} "Profile Data Type")
+; CHECK-DWARF:   DW_TAG_LLVM_annotation
+; CHECK-DWARF: DW_AT_name	("Function Name")
+; CHECK-DWARF: DW_AT_const_value	("foo")
+; CHECK-DWARF:   DW_TAG_LLVM_annotation
+; CHECK-DWARF: DW_AT_name	("CFG Hash")
+; CHECK-DWARF: DW_AT_const_value	(12345678)
+; CHECK-DWARF:   DW_TAG_LLVM_annotation
+; CHECK-DWARF: DW_AT_name	("Num Counters")
+; CHECK-DWARF: DW_AT_const_value	(2)
+; CHECK-DWARF:   NULL
+; CHECK-DWARF: NULL
+; CHECK-DWARF:   DW_TAG_unspecified_type
+; CHECK-DWARF:   NULL
+
+; CHECK-ASM-NOT: .section   __llvm_prf_data
+; CHECK-ASM-NOT: .section   __llvm_prf_names
Index: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
===
--- llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -291,6 +291,8 @@
 // Command line option to specify the name of the function for CFG dump
 // Defined in Analysis/BlockFrequencyInfo.cpp:  -view-bfi-func-name=
 extern cl::opt ViewBlockFreqFuncName;
+

[PATCH] D113422: [clang-tidy][NFC] Move CachedGlobList to GlobList.h

2021-12-03 Thread Salman Javed via Phabricator via cfe-commits
salman-javed-nz accepted this revision.
salman-javed-nz added a comment.
This revision is now accepted and ready to land.

LGTM. The use of `mutable` with public inheritence is all over the 
clang-tools-extra code. See `class SwapIndex : public SymbolIndex`.


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

https://reviews.llvm.org/D113422

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


[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-03 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray added inline comments.



Comment at: clang/lib/AST/APValue.cpp:637-639
+  // Nothing we can do about a sequence that is not null-terminated
+  if (!Data[--Size].getInt().isZero())
+return false;

rsmith wrote:
> lichray wrote:
> > rsmith wrote:
> > > We should drop all trailing zeroes here, because array initialization 
> > > from a string literal will reconstruct them.
> > Are you sure? `MyType<{""}>` and `MyType<{"\0\0\0"}>` are different types.
> That's a separate bug. `APValue`'s printing is not intended to identify the 
> type of the result, only to identify the value in the case where we already 
> know the type. Eg, we don't print a type suffix on a pretty-printed integer 
> value. For the specific case of template parameters, it's the responsibility 
> of the template argument printing code to uniquely identify the template 
> argument.
> 
> When a template parameter has a deduced class type, we should include that 
> type in the printed output in order to uniquely identify the specialization, 
> but we don't, because that's not been implemented yet. When that's fixed, we 
> should print those cases as `MyType{""}>` and `MyType 4>{""}>`. This is necessary anyway, because we can't in general assume that 
> the resulting value is enough to indicate what type CTAD will have selected.
> 
> We already handle this properly in some cases, but see this FIXMEs: 
> https://github.com/llvm/llvm-project/blob/6c2be3015e07f25912b8cd5b75214c532f568638/clang/lib/AST/TemplateBase.cpp#L433
> 
> See also some related issues: https://godbolt.org/z/YhcdMYx6n (note that the 
> non-struct enum case is handled properly but the enum-in-struct case is not).
> That's a separate bug. [...]
> 
> When a template parameter has a deduced class type, we should include that 
> type in the printed output in order to uniquely identify the specialization, 
> but we don't, because that's not been implemented yet. When that's fixed, we 
> should print those cases as `MyType{""}>` and `MyType 4>{""}>`. This is necessary anyway, because we can't in general assume that 
> the resulting value is enough to indicate what type CTAD will have selected.
>

But it looks like a feature rather than a bug? String literals provided both 
type and value to emphasis a structural type's value for equivalence 
comparison. The reason of why `MyType<{""}>` and `MyType<"\0\0\0">` are 
different looks more obvious to me.

> See also some related issues: https://godbolt.org/z/YhcdMYx6n (note that the 
> non-struct enum case is handled properly but the enum-in-struct case is not).

I'm fine if a specific NTTP, `A`, is treated differently from `auto`. But the 
last case is similar to this https://godbolt.org/z/YcscTrchM Which I thought 
about printing something like `Y<{(E[]){97, 98}}>` :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

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


[PATCH] D108901: [Sparc] Create an error when `__builtin_longjmp` is used

2021-12-03 Thread Brad Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeeb4266f8137: [Sparc] Create an error when 
`__builtin_longjmp` is used (authored by xtkoba, committed by brad).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108901

Files:
  clang/lib/Basic/Targets/Sparc.h
  clang/test/Sema/builtin-longjmp.c


Index: clang/test/Sema/builtin-longjmp.c
===
--- clang/test/Sema/builtin-longjmp.c
+++ clang/test/Sema/builtin-longjmp.c
@@ -3,12 +3,12 @@
 // RUN: %clang_cc1 -triple x86_64-windows -emit-llvm < %s| FileCheck %s
 // RUN: %clang_cc1 -triple powerpc-unknown-unknown -emit-llvm < %s| FileCheck 
%s
 // RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm < %s| 
FileCheck %s
-// RUN: %clang_cc1 -triple sparc-eabi-unknown -emit-llvm < %s | FileCheck %s
 // RUN: %clang_cc1 -triple ve-unknown-unknown -emit-llvm < %s | FileCheck %s
 
 // RUN: %clang_cc1 -triple aarch64-unknown-unknown -emit-llvm-only -verify %s
 // RUN: %clang_cc1 -triple mips-unknown-unknown -emit-llvm-only -verify %s
 // RUN: %clang_cc1 -triple mips64-unknown-unknown -emit-llvm-only -verify %s
+// RUN: %clang_cc1 -triple sparc-eabi-unknown -emit-llvm-only -verify %s
 
 // Check that __builtin_longjmp and __builtin_setjmp are lowered into
 // IR intrinsics on those architectures that can handle them.
Index: clang/lib/Basic/Targets/Sparc.h
===
--- clang/lib/Basic/Targets/Sparc.h
+++ clang/lib/Basic/Targets/Sparc.h
@@ -48,8 +48,6 @@
 
   bool hasFeature(StringRef Feature) const override;
 
-  bool hasSjLjLowering() const override { return true; }
-
   ArrayRef getTargetBuiltins() const override {
 // FIXME: Implement!
 return None;
@@ -178,7 +176,6 @@
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override;
 
-  bool hasSjLjLowering() const override { return true; }
   bool hasExtIntType() const override { return true; }
 };
 


Index: clang/test/Sema/builtin-longjmp.c
===
--- clang/test/Sema/builtin-longjmp.c
+++ clang/test/Sema/builtin-longjmp.c
@@ -3,12 +3,12 @@
 // RUN: %clang_cc1 -triple x86_64-windows -emit-llvm < %s| FileCheck %s
 // RUN: %clang_cc1 -triple powerpc-unknown-unknown -emit-llvm < %s| FileCheck %s
 // RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm < %s| FileCheck %s
-// RUN: %clang_cc1 -triple sparc-eabi-unknown -emit-llvm < %s | FileCheck %s
 // RUN: %clang_cc1 -triple ve-unknown-unknown -emit-llvm < %s | FileCheck %s
 
 // RUN: %clang_cc1 -triple aarch64-unknown-unknown -emit-llvm-only -verify %s
 // RUN: %clang_cc1 -triple mips-unknown-unknown -emit-llvm-only -verify %s
 // RUN: %clang_cc1 -triple mips64-unknown-unknown -emit-llvm-only -verify %s
+// RUN: %clang_cc1 -triple sparc-eabi-unknown -emit-llvm-only -verify %s
 
 // Check that __builtin_longjmp and __builtin_setjmp are lowered into
 // IR intrinsics on those architectures that can handle them.
Index: clang/lib/Basic/Targets/Sparc.h
===
--- clang/lib/Basic/Targets/Sparc.h
+++ clang/lib/Basic/Targets/Sparc.h
@@ -48,8 +48,6 @@
 
   bool hasFeature(StringRef Feature) const override;
 
-  bool hasSjLjLowering() const override { return true; }
-
   ArrayRef getTargetBuiltins() const override {
 // FIXME: Implement!
 return None;
@@ -178,7 +176,6 @@
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override;
 
-  bool hasSjLjLowering() const override { return true; }
   bool hasExtIntType() const override { return true; }
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] eeb4266 - [Sparc] Create an error when `__builtin_longjmp` is used

2021-12-03 Thread Brad Smith via cfe-commits

Author: Tee KOBAYASHI
Date: 2021-12-03T23:41:50-05:00
New Revision: eeb4266f8137c232f0f218a727dd12b5d4f52adc

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

LOG: [Sparc] Create an error when `__builtin_longjmp` is used

Support for builtin setjmp/longjmp was removed by 
https://reviews.llvm.org/D51487. An
error should be created when compiling C code using __builtin_setjmp or 
__builtin_longjmp.

Reviewed By: dcederman

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

Added: 


Modified: 
clang/lib/Basic/Targets/Sparc.h
clang/test/Sema/builtin-longjmp.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/Sparc.h b/clang/lib/Basic/Targets/Sparc.h
index 22a1621fcb9fe..badb568dfb805 100644
--- a/clang/lib/Basic/Targets/Sparc.h
+++ b/clang/lib/Basic/Targets/Sparc.h
@@ -48,8 +48,6 @@ class LLVM_LIBRARY_VISIBILITY SparcTargetInfo : public 
TargetInfo {
 
   bool hasFeature(StringRef Feature) const override;
 
-  bool hasSjLjLowering() const override { return true; }
-
   ArrayRef getTargetBuiltins() const override {
 // FIXME: Implement!
 return None;
@@ -178,7 +176,6 @@ class LLVM_LIBRARY_VISIBILITY SparcV8TargetInfo : public 
SparcTargetInfo {
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override;
 
-  bool hasSjLjLowering() const override { return true; }
   bool hasExtIntType() const override { return true; }
 };
 

diff  --git a/clang/test/Sema/builtin-longjmp.c 
b/clang/test/Sema/builtin-longjmp.c
index 3023098a76310..99463cf3385a1 100644
--- a/clang/test/Sema/builtin-longjmp.c
+++ b/clang/test/Sema/builtin-longjmp.c
@@ -3,12 +3,12 @@
 // RUN: %clang_cc1 -triple x86_64-windows -emit-llvm < %s| FileCheck %s
 // RUN: %clang_cc1 -triple powerpc-unknown-unknown -emit-llvm < %s| FileCheck 
%s
 // RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm < %s| 
FileCheck %s
-// RUN: %clang_cc1 -triple sparc-eabi-unknown -emit-llvm < %s | FileCheck %s
 // RUN: %clang_cc1 -triple ve-unknown-unknown -emit-llvm < %s | FileCheck %s
 
 // RUN: %clang_cc1 -triple aarch64-unknown-unknown -emit-llvm-only -verify %s
 // RUN: %clang_cc1 -triple mips-unknown-unknown -emit-llvm-only -verify %s
 // RUN: %clang_cc1 -triple mips64-unknown-unknown -emit-llvm-only -verify %s
+// RUN: %clang_cc1 -triple sparc-eabi-unknown -emit-llvm-only -verify %s
 
 // Check that __builtin_longjmp and __builtin_setjmp are lowered into
 // IR intrinsics on those architectures that can handle them.



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


[PATCH] D109818: [HIPSPV] Convert HIP kernels to SPIR-V kernels

2021-12-03 Thread Alexey Bader via Phabricator via cfe-commits
bader added a comment.

In D109818#3169531 , @linjamaki wrote:

> The patch is ready to land. @Anastasia, @bader, could you commit this patch 
> to the LLVM for us? Thanks.

Could you rebase on the tip of the main branch, please? I see a couple of 
conflicts when I cherry-pick the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109818

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


[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

2021-12-03 Thread Paul Walker via Phabricator via cfe-commits
paulwalker-arm added a comment.

I agree, it's the change to VScaleMin that has caused the issue.  If the 
LangOpts default can remain as 0 and you can still achieve what you're after 
then that would be perfect.


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

https://reviews.llvm.org/D113294

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


[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3052
+// & 1
+if (Right.Tok.isLiteral())
+  return true;

Is this valid code? Or did we just wrongly assign PointerOrReference? I'd say 
after that there can not be a literal in valid code, thus we do not need to 
handle it.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3065
+// for (auto a = 0, b = 0; const auto& c : {1, 2, 3})
+if (Left.Previous && Left.Previous->is(tok::kw_auto) &&
+Right.is(tok::identifier))

Do we need this just for `auto`? What when `auto` is replaced with `int`?



Comment at: clang/lib/Format/TokenAnnotator.cpp:3070
+
+return (
+!Right.isOneOf(TT_PointerOrReference, TT_ArraySubscriptLSquare,

Drop the outer paren?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115050

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


[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

2021-12-03 Thread Cullen Rhodes via Phabricator via cfe-commits
c-rhodes marked an inline comment as done.
c-rhodes added a comment.

In D113294#3170007 , @paulwalker-arm 
wrote:

> I agree, it's the change to VScaleMin that has caused the issue.  If the 
> LangOpts default can remain as 0 and you can still achieve what you're after 
> then that would be perfect.

Not sure why this was an issue when I first looked at it, fresh eyes maybe, 
fixed now. Thanks for raising.


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

https://reviews.llvm.org/D113294

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


[clang] 0a14674 - CodeGen: Strip exception specifications from function types in CFI type names.

2021-12-03 Thread Nico Weber via cfe-commits

Author: Peter Collingbourne
Date: 2021-12-03T14:50:52-05:00
New Revision: 0a14674f276b598d23353290635fc62f93e9ab30

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

LOG: CodeGen: Strip exception specifications from function types in CFI type 
names.

With C++17 the exception specification has been made part of the
function type, and therefore part of mangled type names.

However, it's valid to convert function pointers with an exception
specification to function pointers with the same argument and return
types but without an exception specification, which means that e.g. a
function of type "void () noexcept" can be called through a pointer
of type "void ()". We must therefore consider the two types to be
compatible for CFI purposes.

We can do this by stripping the exception specification before mangling
the type name, which is what this patch does.

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

Added: 
clang/test/CodeGenCXX/cfi-icall-noexcept.cpp

Modified: 
clang/lib/CodeGen/CodeGenModule.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 9ba1a5c25e81a..39044617d6774 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -6398,6 +6398,11 @@ void CodeGenModule::EmitOMPThreadPrivateDecl(const 
OMPThreadPrivateDecl *D) {
 llvm::Metadata *
 CodeGenModule::CreateMetadataIdentifierImpl(QualType T, MetadataTypeMap ,
 StringRef Suffix) {
+  if (auto *FnType = T->getAs())
+T = getContext().getFunctionType(
+FnType->getReturnType(), FnType->getParamTypes(),
+FnType->getExtProtoInfo().withExceptionSpec(EST_None));
+
   llvm::Metadata * = Map[T.getCanonicalType()];
   if (InternalId)
 return InternalId;

diff  --git a/clang/test/CodeGenCXX/cfi-icall-noexcept.cpp 
b/clang/test/CodeGenCXX/cfi-icall-noexcept.cpp
new file mode 100644
index 0..eabc4862b4c52
--- /dev/null
+++ b/clang/test/CodeGenCXX/cfi-icall-noexcept.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall 
-emit-llvm -std=c++17 -o - %s | FileCheck %s
+
+// Tests that exception specifiers are stripped when forming the
+// mangled CFI type name.
+
+void f() noexcept {}
+
+// CHECK: define{{.*}} void @_Z1fv({{.*}} !type [[TS1:![0-9]+]] !type 
[[TS2:![0-9]+]]
+
+// CHECK: [[TS1]] = !{i64 0, !"_ZTSFvvE"}
+// CHECK: [[TS2]] = !{i64 0, !"_ZTSFvvE.generalized"}



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


[PATCH] D115068: [clang-format][NFC] Move static variable in scope

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius, owenpan.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Let only the JS/TS users pay for the initialistation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115068

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -493,14 +493,14 @@
   return true;
   }
 
-  // Break after the closing parenthesis of TypeScript decorators before
-  // functions, getters and setters.
-  static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
-   "function"};
   if (Style.Language == FormatStyle::LK_JavaScript &&
-  BreakBeforeDecoratedTokens.contains(Current.TokenText) &&
   Previous.is(tok::r_paren) && Previous.is(TT_JavaAnnotation)) {
-return true;
+// Break after the closing parenthesis of TypeScript decorators before
+// functions, getters and setters.
+static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
+ "function"};
+if (BreakBeforeDecoratedTokens.contains(Current.TokenText))
+  return true;
   }
 
   // If the return type spans multiple lines, wrap before the function name.


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -493,14 +493,14 @@
   return true;
   }
 
-  // Break after the closing parenthesis of TypeScript decorators before
-  // functions, getters and setters.
-  static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
-   "function"};
   if (Style.Language == FormatStyle::LK_JavaScript &&
-  BreakBeforeDecoratedTokens.contains(Current.TokenText) &&
   Previous.is(tok::r_paren) && Previous.is(TT_JavaAnnotation)) {
-return true;
+// Break after the closing parenthesis of TypeScript decorators before
+// functions, getters and setters.
+static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
+ "function"};
+if (BreakBeforeDecoratedTokens.contains(Current.TokenText))
+  return true;
   }
 
   // If the return type spans multiple lines, wrap before the function name.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113319: [clang-format] Improve require and concept handling

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 391708.
HazardyKnusperkeks retitled this revision from "[clang-format] Improve require 
handling" to "[clang-format] Improve require and concept handling".
HazardyKnusperkeks edited the summary of this revision.
HazardyKnusperkeks added reviewers: gonzalobg, CaseyCarter.
HazardyKnusperkeks added a comment.
This revision is now accepted and ready to land.

Update on my work in progress. The new test cases are all passed. The old ones 
are only kept to pick some lines from them.

I would like to hear some feedback while I finish the rest, mostly writing 
tests.


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

https://reviews.llvm.org/D113319

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -67,6 +67,51 @@
   EXPECT_TOKEN(Tokens[11], tok::star, TT_PointerOrReference);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsRequiresClauses) {
+  auto Tokens = annotate("template \n"
+ "concept C = (Foo && Bar) && (Bar && Baz);");
+
+  EXPECT_EQ(Tokens.size(), 21u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[16], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template \n"
+"concept C = requires(T t) {\n"
+"  { t.foo() };\n"
+"} && Bar && Baz;");
+  EXPECT_EQ(Tokens.size(), 35u) << Tokens;
+  EXPECT_TOKEN(Tokens[23], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[28], tok::ampamp, TT_BinaryOperator);
+
+  Tokens = annotate("template\n"
+"requires C1 && (C21 || C22 && C2e) && C3\n"
+"struct Foo;");
+  EXPECT_EQ(Tokens.size(), 36u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::identifier, TT_Unknown);
+  EXPECT_EQ(Tokens[6]->FakeLParens.size(), 1u);
+  EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[16], tok::pipepipe, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[21], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[27], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[31], tok::greater, TT_TemplateCloser);
+  EXPECT_EQ(Tokens[31]->FakeRParens, 1u);
+
+  Tokens =
+  annotate("template\n"
+   "requires (C1 && (C21 || C22 && C2e) && C3)\n"
+   "struct Foo;");
+  EXPECT_EQ(Tokens.size(), 38u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_Unknown);
+  EXPECT_EQ(Tokens[7]->FakeLParens.size(), 1u);
+  EXPECT_TOKEN(Tokens[11], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::pipepipe, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[22], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[28], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[32], tok::greater, TT_TemplateCloser);
+  EXPECT_EQ(Tokens[32]->FakeRParens, 1u);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19143,6 +19143,30 @@
   // For backward compatibility:
   CHECK_PARSE("SpacesInAngles: false", SpacesInAngles, FormatStyle::SIAS_Never);
   CHECK_PARSE("SpacesInAngles: true", SpacesInAngles, FormatStyle::SIAS_Always);
+
+  CHECK_PARSE("RequiresClausePositionForClasses: ToPreceeding",
+  RequiresClausePositionForClasses, FormatStyle::RCPS_ToPreceeding);
+  CHECK_PARSE("RequiresClausePositionForClasses: ToFollowing",
+  RequiresClausePositionForClasses, FormatStyle::RCPS_ToFollowing);
+  CHECK_PARSE("RequiresClausePositionForClasses: SingleLine",
+  RequiresClausePositionForClasses, FormatStyle::RCPS_SingleLine);
+  CHECK_PARSE("RequiresClausePositionForClasses: TwoLines",
+  RequiresClausePositionForClasses, FormatStyle::RCPS_TwoLines);
+  CHECK_PARSE("RequiresClausePositionForClasses: OwnLine",
+  RequiresClausePositionForClasses, FormatStyle::RCPS_OwnLine);
+
+  CHECK_PARSE("RequiresClausePositionForFunctions: ToPreceeding",
+  RequiresClausePositionForFunctions,
+  FormatStyle::RCPS_ToPreceeding);
+  CHECK_PARSE("RequiresClausePositionForFunctions: ToFollowing",
+  RequiresClausePositionForFunctions,
+  

[PATCH] D115070: [clang-format][NFC] Early return when nothing to do

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115070

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


[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

2021-12-03 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision.
phosek 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/D115045/new/

https://reviews.llvm.org/D115045

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


[PATCH] D106585: Fix clang debug info irgen of i128 enums

2021-12-03 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D106585#3169898 , @rnk wrote:

> This usage of isSameValue seems suspicious: 
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/LLVMContextImpl.h#L389
>
> It seems to allow the possibility that APInts of differing bitwidth compare 
> equal, but the hash value incorporates the bitwidth, so they may be inserted 
> into differing hash buckets.

yup, also checking the bit width makes the non-determinism go away, I'll send 
out a patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106585

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


[PATCH] D115032: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

2021-12-03 Thread Stanislav Mekhanoshin via Phabricator via cfe-commits
rampitec accepted this revision.
rampitec 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/D115032/new/

https://reviews.llvm.org/D115032

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


[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

https://bugs.llvm.org/show_bug.cgi?id=48916

Left and Right Alignment inside a loop is misaligned.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115050

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1924,6 +1924,7 @@
   verifyFormat("int *a = f1();", Style);
   verifyFormat("int  = f2();", Style);
   verifyFormat("int & = f3();", Style);
+  verifyFormat("for (auto a = 0, b = 0; const auto  : {1, 2, 3})", Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int *c;\n"
@@ -1943,6 +1944,7 @@
   verifyFormat("int* a = f1();", Style);
   verifyFormat("int& b = f2();", Style);
   verifyFormat("int&& c = f3();", Style);
+  verifyFormat("for (auto a = 0, b = 0; const auto& c : {1, 2, 3})", Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int* c;\n"
@@ -1979,6 +1981,7 @@
   verifyFormat("int* a = f1();", Style);
   verifyFormat("int & b = f2();", Style);
   verifyFormat("int && c = f3();", Style);
+  verifyFormat("for (auto a = 0, b = 0; const auto & c : {1, 2, 3})", Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int*  c;\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3024,8 +3024,12 @@
  Style.SpaceAroundPointerQualifiers == FormatStyle::SAPQ_Both) &&
 (Left.is(TT_AttributeParen) || 
Left.canBePointerOrReferenceQualifier()))
   return true;
+if (Left.Tok.isLiteral())
+  return true;
+if (Left.Tok.is(tok::kw_auto))
+  return getTokenPointerOrReferenceAlignment(Right) !=
+ FormatStyle::PAS_Left;
 return (
-Left.Tok.isLiteral() ||
 (!Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
  (getTokenPointerOrReferenceAlignment(Right) != FormatStyle::PAS_Left 
||
   (Line.IsMultiVariableDeclStmt &&
@@ -3044,18 +3048,32 @@
  Style.SpaceAroundPointerQualifiers == FormatStyle::SAPQ_Both) &&
 Right.canBePointerOrReferenceQualifier())
   return true;
-return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
-   (Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
-!Right.is(TT_StartOfName)) ||
-   (Right.is(tok::l_brace) && Right.is(BK_Block)) ||
-   (!Right.isOneOf(TT_PointerOrReference, TT_ArraySubscriptLSquare,
-   tok::l_paren) &&
-(getTokenPointerOrReferenceAlignment(Left) !=
- FormatStyle::PAS_Right &&
- !Line.IsMultiVariableDeclStmt) &&
-Left.Previous &&
-!Left.Previous->isOneOf(tok::l_paren, tok::coloncolon,
-tok::l_square));
+// & 1
+if (Right.Tok.isLiteral())
+  return true;
+// & // comment
+if (Right.is(TT_BlockComment))
+  return true;
+// foo() -> const Bar * override/final
+if (Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
+!Right.is(TT_StartOfName))
+  return true;
+// & {
+if (Right.is(tok::l_brace) && Right.is(BK_Block))
+  return true;
+// for (auto a = 0, b = 0; const auto& c : {1, 2, 3})
+if (Left.Previous && Left.Previous->is(tok::kw_auto) &&
+Right.is(tok::identifier))
+  return getTokenPointerOrReferenceAlignment(Left) !=
+ FormatStyle::PAS_Right;
+
+return (
+!Right.isOneOf(TT_PointerOrReference, TT_ArraySubscriptLSquare,
+   tok::l_paren) &&
+(getTokenPointerOrReferenceAlignment(Left) != FormatStyle::PAS_Right &&
+ !Line.IsMultiVariableDeclStmt) &&
+Left.Previous &&
+!Left.Previous->isOneOf(tok::l_paren, tok::coloncolon, tok::l_square));
   }
   // Ensure right pointer alignment with ellipsis e.g. int *...P
   if (Left.is(tok::ellipsis) && Left.Previous &&


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1924,6 +1924,7 @@
   verifyFormat("int *a = f1();", Style);
   verifyFormat("int  = f2();", Style);
   verifyFormat("int & = f3();", Style);
+  verifyFormat("for (auto a = 0, b = 0; const auto  : {1, 2, 3})", Style);
 
   Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   verifyFormat("Const unsigned int 

[PATCH] D113917: Add infrastructure to support matcher names.

2021-12-03 Thread James King via Phabricator via cfe-commits
jcking1034 added a comment.

@hokein @aaron.ballman Following up on this, let me know if there are any other 
action items to address!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113917

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


[PATCH] D115067: [clang-format][NFC] Use range based for

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius, owenpan.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

That's much easier to read.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115067

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2373,11 +2373,9 @@
 }
 
 void TokenAnnotator::annotate(AnnotatedLine ) {
-  for (SmallVectorImpl::iterator I = Line.Children.begin(),
-  E = Line.Children.end();
-   I != E; ++I) {
-annotate(**I);
-  }
+  for (auto  : Line.Children)
+annotate(*Child);
+
   AnnotatingParser Parser(Style, Line, Keywords);
   Line.Type = Parser.parseLine();
 


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2373,11 +2373,9 @@
 }
 
 void TokenAnnotator::annotate(AnnotatedLine ) {
-  for (SmallVectorImpl::iterator I = Line.Children.begin(),
-  E = Line.Children.end();
-   I != E; ++I) {
-annotate(**I);
-  }
+  for (auto  : Line.Children)
+annotate(*Child);
+
   AnnotatingParser Parser(Style, Line, Keywords);
   Line.Type = Parser.parseLine();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3052
+// & 1
+if (Right.Tok.isLiteral())
+  return true;

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > Is this valid code? Or did we just wrongly assign PointerOrReference? I'd 
> > say after that there can not be a literal in valid code, thus we do not 
> > need to handle it.
> Ok so as you see I broken out the compound statement, to be honest I find 
> these compound if's unreadable and I can't for the life of me work out what 
> case they are trying to handle. (I want to do this more)..
> 
> Please no one say doing it separately is slower! without measuring it..we are 
> talking ns difference if anything at all.
> 
> I agree I truly believe we are mis assigning 
> PointerOrReference/BinaryOperator this as I mentioned  in
> https://lists.llvm.org/pipermail/cfe-dev/2021-December/069486.html is a major 
> issue and I think from time to time these rules try to correct that 
> situation, in this case & is the logical operation on a reference.
I'm with you to split up these monsters!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115050

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


[PATCH] D115068: [clang-format][NFC] Move static variable in scope

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115068

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


[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

2021-12-03 Thread Paul Walker via Phabricator via cfe-commits
paulwalker-arm requested changes to this revision.
paulwalker-arm added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:476-484
+  assert(LangOpts.VScaleMin && "vscale min must be greater than 0!");
+
+  if (LangOpts.VScaleMax)
 return std::pair(LangOpts.VScaleMin,
  LangOpts.VScaleMax);
+
   if (hasFeature("sve"))

This looks like a change of behaviour to me.  Previously the command line flags 
would override the "sve" default but now that only happens when the user 
specifies a maximum value.  That means the interface can no longer be used to 
force truly width agnostic values.


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

https://reviews.llvm.org/D113294

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


[PATCH] D115065: [clang-format][NFC] Merge two calls of isOneOf

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, curdeius.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks 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/D115065

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2034,9 +2034,8 @@
tok::comma, tok::semi, tok::kw_return, tok::colon,
tok::kw_co_return, tok::kw_co_await,
tok::kw_co_yield, tok::equal, tok::kw_delete,
-   tok::kw_sizeof, tok::kw_throw) ||
-PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
-   TT_UnaryOperator, TT_CastRParen))
+   tok::kw_sizeof, tok::kw_throw, TT_BinaryOperator,
+   TT_ConditionalExpr, TT_UnaryOperator, 
TT_CastRParen))
   return TT_UnaryOperator;
 
 if (NextToken->is(tok::l_square) && NextToken->isNot(TT_LambdaLSquare))


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2034,9 +2034,8 @@
tok::comma, tok::semi, tok::kw_return, tok::colon,
tok::kw_co_return, tok::kw_co_await,
tok::kw_co_yield, tok::equal, tok::kw_delete,
-   tok::kw_sizeof, tok::kw_throw) ||
-PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
-   TT_UnaryOperator, TT_CastRParen))
+   tok::kw_sizeof, tok::kw_throw, TT_BinaryOperator,
+   TT_ConditionalExpr, TT_UnaryOperator, TT_CastRParen))
   return TT_UnaryOperator;
 
 if (NextToken->is(tok::l_square) && NextToken->isNot(TT_LambdaLSquare))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115071: [clang-format][NFC] Use range based for for fake l parens

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius, owenpan.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks 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/D115071

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1351,10 +1351,7 @@
 (Previous->getPrecedence() == prec::Assignment &&
  Style.AlignOperands != FormatStyle::OAS_DontAlign) ||
 Previous->is(TT_ObjCMethodExpr)));
-  for (SmallVectorImpl::const_reverse_iterator
-   I = Current.FakeLParens.rbegin(),
-   E = Current.FakeLParens.rend();
-   I != E; ++I) {
+  for (const auto  : llvm::reverse(Current.FakeLParens)) {
 ParenState NewParenState = State.Stack.back();
 NewParenState.Tok = nullptr;
 NewParenState.ContainsLineBreak = false;
@@ -1366,7 +1363,7 @@
 NewParenState.NoLineBreak || State.Stack.back().NoLineBreakInOperand;
 
 // Don't propagate AvoidBinPacking into subexpressions of arg/param lists.
-if (*I > prec::Comma)
+if (PrecedenceLevel > prec::Comma)
   NewParenState.AvoidBinPacking = false;
 
 // Indent from 'LastSpace' unless these are fake parentheses encapsulating
@@ -1374,11 +1371,11 @@
 // brackets is disabled.
 if (!Current.isTrailingComment() &&
 (Style.AlignOperands != FormatStyle::OAS_DontAlign ||
- *I < prec::Assignment) &&
+ PrecedenceLevel < prec::Assignment) &&
 (!Previous || Previous->isNot(tok::kw_return) ||
- (Style.Language != FormatStyle::LK_Java && *I > 0)) &&
+ (Style.Language != FormatStyle::LK_Java && PrecedenceLevel > 0)) &&
 (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign ||
- *I != prec::Comma || Current.NestingLevel == 0)) {
+ PrecedenceLevel != prec::Comma || Current.NestingLevel == 0)) {
   NewParenState.Indent =
   std::max(std::max(State.Column, NewParenState.Indent),
State.Stack.back().LastSpace);
@@ -1387,7 +1384,7 @@
 if (Previous &&
 (Previous->getPrecedence() == prec::Assignment ||
  Previous->is(tok::kw_return) ||
- (*I == prec::Conditional && Previous->is(tok::question) &&
+ (PrecedenceLevel == prec::Conditional && Previous->is(tok::question) 
&&
   Previous->is(TT_ConditionalExpr))) &&
 !Newline) {
   // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
@@ -1405,9 +1402,9 @@
 //   ParameterToInnerFunction));
 //   OuterFunction(SomeObject.InnerFunctionCall( // break
 //   ParameterToInnerFunction));
-if (*I > prec::Unknown)
+if (PrecedenceLevel > prec::Unknown)
   NewParenState.LastSpace = std::max(NewParenState.LastSpace, 
State.Column);
-if (*I != prec::Conditional && !Current.is(TT_UnaryOperator) &&
+if (PrecedenceLevel != prec::Conditional && !Current.is(TT_UnaryOperator) 
&&
 Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
   NewParenState.StartOfFunctionCall = State.Column;
 
@@ -1416,17 +1413,18 @@
 // an assignment (i.e. *I <= prec::Assignment) as those have different
 // indentation rules. Indent other expression, unless the indentation needs
 // to be skipped.
-if (*I == prec::Conditional && Previous && Previous->is(tok::colon) &&
-Previous->is(TT_ConditionalExpr) && I == Current.FakeLParens.rbegin() 
&&
+if (PrecedenceLevel == prec::Conditional && Previous &&
+Previous->is(tok::colon) && Previous->is(TT_ConditionalExpr) &&
+ == () &&
 !State.Stack.back().IsWrappedConditional) {
   NewParenState.IsChainedConditional = true;
   NewParenState.UnindentOperator = State.Stack.back().UnindentOperator;
-} else if (*I == prec::Conditional ||
-   (!SkipFirstExtraIndent && *I > prec::Assignment &&
+} else if (PrecedenceLevel == prec::Conditional ||
+   (!SkipFirstExtraIndent && PrecedenceLevel > prec::Assignment &&
 !Current.isTrailingComment())) {
   NewParenState.Indent += Style.ContinuationIndentWidth;
 }
-if ((Previous && !Previous->opensScope()) || *I != prec::Comma)
+if ((Previous && !Previous->opensScope()) || PrecedenceLevel != 
prec::Comma)
   NewParenState.BreakBeforeParameter = false;
 State.Stack.push_back(NewParenState);
 SkipFirstExtraIndent = false;


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1351,10 +1351,7 

[PATCH] D115072: [clang-format][NFC] Use member directly

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM if CI is happy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115072

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


[PATCH] D115066: [clang-format][NFC] Reorder conditions

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115066

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


[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-03 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray updated this revision to Diff 391717.
lichray added a comment.

- Add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

Files:
  clang/lib/AST/APValue.cpp
  clang/test/SemaTemplate/temp_arg_string_printing.cpp

Index: clang/test/SemaTemplate/temp_arg_string_printing.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_string_printing.cpp
@@ -0,0 +1,137 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -ast-print %s | FileCheck %s
+
+using size_t = __SIZE_TYPE__;
+static_assert(__has_builtin(__make_integer_seq));
+
+template  class idx_seq {};
+template  using make_idx_seq = __make_integer_seq;
+
+template 
+struct Str {
+  constexpr Str(CharT const ()[N]) : Str(s, make_idx_seq()) {}
+  CharT value[N];
+
+private:
+  template 
+  constexpr Str(CharT const ()[N], idx_seq) : value{s[I]...} {}
+};
+
+template  class ASCII {};
+
+void narrow() {
+  // CHECK{LITERAL}: ASCII<{""}>
+  new ASCII<"">;
+  // CHECK{LITERAL}: ASCII<{"the quick brown fox jumps"}>
+  new ASCII<"the quick brown fox jumps">;
+  // CHECK{LITERAL}: ASCII<{"OVER THE LAZY DOG 0123456789"}>
+  new ASCII<"OVER THE LAZY DOG 0123456789">;
+  // CHECK{LITERAL}: ASCII<{"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{"escape\0"}>
+  new ASCII<"escape\0">;
+  // CHECK{LITERAL}: ASCII<{"escape\r\n"}>
+  new ASCII<"escape\r\n">;
+  // CHECK{LITERAL}: ASCII<{"escape\\\t\'\f\v"}>
+  new ASCII<"escape\\\t'\f\v">;
+  // CHECK{LITERAL}: ASCII<{"escape\a\bc"}>
+  new ASCII<"escape\a\b\c">;
+  // CHECK{LITERAL}: ASCII<{{110, 111, 116, 17, 0}}>
+  new ASCII<"not\x11">;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 127, 16, 1, 32, 97, 98, 99, 0}}>
+  new ASCII<"\x12\x14\x7f\x10\x01 abc">;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 127, 16, 1, 32, 97, 98, 99, 100, ...}}>
+  new ASCII<"\x12\x14\x7f\x10\x01 abcd">;
+  // CHECK{LITERAL}: ASCII<{"print more characters as string"}>
+  new ASCII<"print more characters as string">;
+  // CHECK{LITERAL}: ASCII<{"print even more characters as string"}>
+  new ASCII<"print even more characters as string">;
+  // CHECK{LITERAL}: ASCII<{"print many characters no more t[...]"}>
+  new ASCII<"print many characters no more than a limit">;
+  // CHECK{LITERAL}: ASCII<{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"}>
+  new ASCII<"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0">;
+  // CHECK{LITERAL}: ASCII<{"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n[...]"}>
+  new ASCII<"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n">;
+}
+
+void wide() {
+  // CHECK{LITERAL}: ASCII<{L""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"the quick brown fox jumps"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"OVER THE LAZY DOG 0123456789"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{L"escape\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\\\t\f\v"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\a\bc"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{110, 111, 116, 17, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 255, 22909, 136, 32, 97, 98, 99, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 255, 22909, 136, 32, 97, 98, 99, 100, ...}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print more characters as string"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print even more characters as string"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print many characters no more t[...]"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n[...]"}>
+  new ASCII;
+}
+
+void utf8()
+{
+  // CHECK{LITERAL}: ASCII<{u8""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{u8"escape\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{229, 165, 189, 239, 191, 189, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"print many characters no more t[...]"}>
+  new ASCII;
+}
+
+void utf16()
+{
+  // CHECK{LITERAL}: ASCII<{u""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{u"escape\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{22909, 65533, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u"print many characters no more t[...]"}>
+  new ASCII;
+}
+
+void utf32()
+{
+  // CHECK{LITERAL}: ASCII<{U""}>
+  new ASCII;
+  // CHECK{LITERAL}: 

[PATCH] D113319: [clang-format] Improve require and concept handling

2021-12-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3424
+templates or between template and function delcarations. In case of
+after the function delcaration it tries to stick to this.
+

`s/delca/decla/g`
`s/Preceeding/Preceding/g`



Comment at: clang/docs/ClangFormatStyleOptions.rst:3464-3483
+  * ``RCPS_TwoLines`` (in configuration: ``TwoLines``)
+The clause always gets its own line, and the content of the clause go
+into the next line with some indentation.
+
+.. code-block:: c++
+
+  template

This option strikes me as actively harmful; I think you should remove it from 
the PR completely. Nobody does this today and nobody should do it in the future 
either.



Comment at: clang/include/clang/Format/Format.h:2504-2505
 
-  /// Indent the requires clause in a template
+  /// Indent the requires clause in a template, this only applies if the
+  /// ``requires`` is at the beginning of a line.
   /// \code

Comma-splice. But I don't think you need to change this comment at all. It's 
obvious that "indent" applies only to constructs at the left side of the 
(logical) source line.



Comment at: clang/include/clang/Format/Format.h:3130-3136
+  /// \brief The position of the ``requires`` clause for class templates.
+  /// \version 14
+  RequiresClausePositionStyle RequiresClausePositionForClasses;
+
+  /// \brief The position of the ``requires`` clause for function templates.
+  /// \version 14
+  RequiresClausePositionStyle RequiresClausePositionForFunctions;

What about for variable templates? What about for alias templates? What about 
for deduction guides?
It makes sense to me to have //one// option for this. It doesn't make sense to 
have 2, 3, or 4. Having 5 feels insane. So, I think you should just have one 
option that applies consistently to all `requires`-clauses everywhere in the 
code.



Comment at: clang/lib/Format/Format.cpp:1213
+  // This is open for discussions! When will LLVM adapt C++20?
+  LLVMStyle.RequiresClausePositionForClasses = FormatStyle::RCPS_OwnLine;
+  LLVMStyle.RequiresClausePositionForFunctions = FormatStyle::RCPS_OwnLine;

HazardyKnusperkeks wrote:
> What are your opinions on that?
+1 `RequiresClausePosition=OwnLine`, but also `IndentRequires=true`.



Comment at: clang/unittests/Format/FormatTest.cpp:22288-22291
+  verifyFormat("template \n"
+   "concept C = ((false || foo()) && C2) || "
+   "(std::trait::value && Baz) ||\n "
+   "   sizeof(T) >= 6;");

Nit: I was initially very confused by the formatting here, until I realized 
that lines 22289 and 22290 are two lines //physically// but represent one 
single line in the //test case//. Strongly recommend eliminating the gratuitous 
line break.



Comment at: clang/unittests/Format/FormatTest.cpp:22374-22378
+  "template \n"
+  "concept C = decltype([]() { return std::true_type{}; }())::value &&\n"
+  "requires(T t) {\n"
+  "  t.bar();\n"
+  "} && sizeof(T) <= 8;");

This looks wrong. Current:
```
template \n"
concept C = decltype([]() { return std::true_type{}; }())::value &&
requires(T t) {
  t.bar();
} && sizeof(T) <= 8;
```
but should be:
```
template \n"
concept C = decltype([]() { return std::true_type{}; }())::value &&
  requires(T t) {
t.bar();
  } && sizeof(T) <= 8;
```
(assuming that all the relevant indent-widths are set to `2`).



Comment at: clang/unittests/Format/FormatTest.cpp:22597
+  verifyFormat("template \n"
+ "requires(std::invocable...>)\n"
+ "struct constant;",

Currently:
```
template 
requires(std::invocable...>)
struct constant;
```
Should be:
```
template 
requires (std::invocable...>)
struct constant;
```
(notice the extra single space). Add a TODO comment and ship it anyway?



Comment at: clang/unittests/Format/FormatTest.cpp:22643
   verifyFormat("template \n"
-   "concept Context = Traits or\n"
-   "Interface or\n"
-   "Request or\n"
-   "Response or\n"
-   "ContextExtension or\n"
-   "::std::is_copy_constructable or "
-   "::std::is_move_constructable or\n"
-   "requires (T c) {\n"
-   "  { c.response; } -> Response;\n"
-   "} or requires (T c) {\n"
-   "  { c.request; } -> Request;\n"
-   "}\n",
+   "requires requires(T t) {\n"
+   "  typename T::Bar;\n"

Data point: libc++ has a mix of `requires (T t) { }` and `requires(T t) { }`. 
Am I correct that clang-format is not yet smart enough to have a meaningful 
opinion about this 

[PATCH] D115032: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

2021-12-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.

Ideally, we could let the builtins accept both vec3 and vec4. But I am OK with 
this for now. I think the overhead may be minimal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115032

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


[libunwind] f178a05 - [libunwind] Fix unwind_leaffunction test

2021-12-03 Thread Leonard Chan via cfe-commits

Author: Leonard Chan
Date: 2021-12-03T11:21:20-08:00
New Revision: f178a05f220403f2a9d73c7640bfcc7dc2d7be72

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

LOG: [libunwind] Fix unwind_leaffunction test

It's possible for this test not to pass if the libc used does not provide
unwind info for raise. We can replace it with __builtin_cast, which can lead
to a SIGTRAP on x86_64 and a SIGILL on aarch64.

Using this alternative, a nop is needed before the __builtin_cast. This is
because libunwind incorrectly decrements pc, which can cause pc to jump into
the previous function and use the incorrect FDE.

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

Added: 


Modified: 
libunwind/test/unwind_leaffunction.pass.cpp

Removed: 




diff  --git a/libunwind/test/unwind_leaffunction.pass.cpp 
b/libunwind/test/unwind_leaffunction.pass.cpp
index 2a6d8311e24c7..8ff21dd35449c 100644
--- a/libunwind/test/unwind_leaffunction.pass.cpp
+++ b/libunwind/test/unwind_leaffunction.pass.cpp
@@ -39,11 +39,17 @@ void signal_handler(int signum) {
 }
 
 __attribute__((noinline)) void crashing_leaf_func(void) {
-  raise(SIGSEGV);
+  // libunwind searches for the address before the return address which points
+  // to the trap instruction. NOP guarantees the trap instruction is not the
+  // first instruction of the function.
+  // We should keep this here for other unwinders that also decrement pc.
+  __asm__ __volatile__("nop");
+  __builtin_trap();
 }
 
 int main(int, char**) {
-  signal(SIGSEGV, signal_handler);
+  signal(SIGTRAP, signal_handler);
+  signal(SIGILL, signal_handler);
   crashing_leaf_func();
   return -2;
 }



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


[clang] 9f95bc7 - [clang][ARM] relax -mtp=cp15 for non-thumb cases

2021-12-03 Thread Nick Desaulniers via cfe-commits

Author: Nick Desaulniers
Date: 2021-12-03T14:00:00-08:00
New Revision: 9f95bc7dc18390199553cf2ea3bfcdc6a95717ef

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

LOG: [clang][ARM] relax -mtp=cp15 for non-thumb cases

Building -march=armv6k Linux kernels with -mtp=cp15 fails to
compile:

error: hardware TLS register is not supported for the arm
sub-architecture

@ardb found docs for ARM1176JZF-S (ARMv6K) that reference hard thread
pointer.

Relax our ARMv6 check for cases where we're targeting ARM via -marm (vs
Thumb1 via -mthumb).  This more closely matches the KConfig requirements
for where we plan to use these (ie. ARMv6K, ARMv7 (arm or thumb2)).

As @peter.smith mentions:
  on armv5 we can write the instruction to read/write to CP15 C13 with
  the ThreadID opcode. However on no armv5 implementation will the CP15
  C13 have a Thread ID register. The GCC intent seems to be whether the
  instruction is encodable rather than check what the CPU supports.

Link: https://github.com/ClangBuiltLinux/linux/issues/1502
Link: 
https://developer.arm.com/documentation/ddi0301/h/system-control-coprocessor/system-control-processor-registers/c13--thread-and-process-id-registers

Reviewed By: ardb, peter.smith

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Arch/ARM.h
clang/test/Driver/clang-translation.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp 
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 21c091e1a0ba4..8d5c64d975502 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -147,6 +147,16 @@ bool arm::useAAPCSForMachO(const llvm::Triple ) {
  T.getOS() == llvm::Triple::UnknownOS || isARMMProfile(T);
 }
 
+// We follow GCC and support when the backend has support for the MRC/MCR
+// instructions that are used to set the hard thread pointer ("CP15 C13
+// Thread id").
+bool arm::isHardTPSupported(const llvm::Triple ) {
+  int Ver = getARMSubArchVersionNumber(Triple);
+  llvm::ARM::ArchKind AK = llvm::ARM::parseArch(Triple.getArchName());
+  return Triple.isARM() || AK == llvm::ARM::ArchKind::ARMV6T2 ||
+ (Ver >= 7 && AK != llvm::ARM::ArchKind::ARMV8MBaseline);
+}
+
 // Select mode for reading thread pointer (-mtp=soft/cp15).
 arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ,
const llvm::Triple ) {
@@ -156,10 +166,7 @@ arm::ReadTPMode arm::getReadTPMode(const Driver , const 
ArgList ,
 .Case("cp15", ReadTPMode::Cp15)
 .Case("soft", ReadTPMode::Soft)
 .Default(ReadTPMode::Invalid);
-if (ThreadPointer == ReadTPMode::Cp15 &&
-getARMSubArchVersionNumber(Triple) < 7 &&
-llvm::ARM::parseArch(Triple.getArchName()) !=
-llvm::ARM::ArchKind::ARMV6T2) {
+if (ThreadPointer == ReadTPMode::Cp15 && !isHardTPSupported(Triple)) {
   D.Diag(diag::err_target_unsupported_tp_hard) << Triple.getArchName();
   return ReadTPMode::Invalid;
 }
@@ -430,7 +437,6 @@ void arm::getARMTargetFeatures(const Driver , const 
llvm::Triple ,
   bool KernelOrKext =
   Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
   arm::FloatABI ABI = arm::getARMFloatABI(D, Triple, Args);
-  arm::ReadTPMode ThreadPointer = arm::getReadTPMode(D, Args, Triple);
   llvm::Optional> WaCPU, WaFPU, WaHDiv,
   WaArch;
 
@@ -482,7 +488,7 @@ void arm::getARMTargetFeatures(const Driver , const 
llvm::Triple ,
 }
   }
 
-  if (ThreadPointer == arm::ReadTPMode::Cp15)
+  if (getReadTPMode(D, Args, Triple) == ReadTPMode::Cp15)
 Features.push_back("+read-tp-hard");
 
   const Arg *ArchArg = Args.getLastArg(options::OPT_march_EQ);

diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.h 
b/clang/lib/Driver/ToolChains/Arch/ARM.h
index b6fd68fbb9c62..fc5b8c87bef9d 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.h
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -53,6 +53,7 @@ FloatABI getARMFloatABI(const Driver , const llvm::Triple 
,
 const llvm::opt::ArgList );
 void setFloatABIInTriple(const Driver , const llvm::opt::ArgList ,
  llvm::Triple );
+bool isHardTPSupported(const llvm::Triple );
 ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList ,
  const llvm::Triple );
 void setArchNameInTriple(const Driver , const llvm::opt::ArgList ,

diff  --git a/clang/test/Driver/clang-translation.c 
b/clang/test/Driver/clang-translation.c
index 230ea8e302024..32dd43b8352f9 100644
--- a/clang/test/Driver/clang-translation.c
+++ b/clang/test/Driver/clang-translation.c
@@ -115,16 +115,22 @@
 // ARMv7_THREAD_POINTER-HARD: 

[PATCH] D114116: [clang][ARM] relax -mtp=cp15 for non-thumb cases

2021-12-03 Thread Nick Desaulniers 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 rG9f95bc7dc183: [clang][ARM] relax -mtp=cp15 for non-thumb 
cases (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114116

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/clang-translation.c


Index: clang/test/Driver/clang-translation.c
===
--- clang/test/Driver/clang-translation.c
+++ clang/test/Driver/clang-translation.c
@@ -115,16 +115,22 @@
 // ARMv7_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
 
 // RUN: %clang -target armv6t2-linux -mtp=cp15 -### -S %s 2>&1 | \
-// RUN: FileCheck -check-prefix=ARMv6T2_THREAD_POINTER-HARD %s
-// ARMv6T2_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
-
+// RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
+// RUN: %clang -target thumbv6t2-linux -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
+// RUN: %clang -target armv6k-linux -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
+// RUN: %clang -target armv6-linux -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
 // RUN: %clang -target armv5t-linux -mtp=cp15 -### -S %s 2>&1 | \
-// RUN: FileCheck -check-prefix=ARMv5_THREAD_POINTER_UNSUPP %s
-// ARMv5_THREAD_POINTER_UNSUPP: hardware TLS register is not supported for the 
armv5 sub-architecture
-
-// RUN: %clang -target thumbv6-linux -mtp=cp15 -### -S %s 2>&1 | \
-// RUN: FileCheck -check-prefix=ARMv6_THREAD_POINTER_UNSUPP %s
-// ARMv6_THREAD_POINTER_UNSUPP: hardware TLS register is not supported for the 
armv6 sub-architecture
+// RUN: FileCheck -check-prefix=ARM_THREAD_POINTER-HARD %s
+// ARM_THREAD_POINTER-HARD: "-target-feature" "+read-tp-hard"
+
+// RUN: %clang -target armv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=THUMBv6_THREAD_POINTER_UNSUPP %s
+// RUN: %clang -target thumbv6-linux -mthumb -mtp=cp15 -### -S %s 2>&1 | \
+// RUN: FileCheck -check-prefix=THUMBv6_THREAD_POINTER_UNSUPP %s
+// THUMBv6_THREAD_POINTER_UNSUPP: hardware TLS register is not supported for 
the thumbv6 sub-architecture
 
 // RUN: %clang -target armv7-linux -mtp=soft -### -S %s 2>&1 | \
 // RUN: FileCheck -check-prefix=ARMv7_THREAD_POINTER_SOFT %s
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -53,6 +53,7 @@
 const llvm::opt::ArgList );
 void setFloatABIInTriple(const Driver , const llvm::opt::ArgList ,
  llvm::Triple );
+bool isHardTPSupported(const llvm::Triple );
 ReadTPMode getReadTPMode(const Driver , const llvm::opt::ArgList ,
  const llvm::Triple );
 void setArchNameInTriple(const Driver , const llvm::opt::ArgList ,
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -147,6 +147,16 @@
  T.getOS() == llvm::Triple::UnknownOS || isARMMProfile(T);
 }
 
+// We follow GCC and support when the backend has support for the MRC/MCR
+// instructions that are used to set the hard thread pointer ("CP15 C13
+// Thread id").
+bool arm::isHardTPSupported(const llvm::Triple ) {
+  int Ver = getARMSubArchVersionNumber(Triple);
+  llvm::ARM::ArchKind AK = llvm::ARM::parseArch(Triple.getArchName());
+  return Triple.isARM() || AK == llvm::ARM::ArchKind::ARMV6T2 ||
+ (Ver >= 7 && AK != llvm::ARM::ArchKind::ARMV8MBaseline);
+}
+
 // Select mode for reading thread pointer (-mtp=soft/cp15).
 arm::ReadTPMode arm::getReadTPMode(const Driver , const ArgList ,
const llvm::Triple ) {
@@ -156,10 +166,7 @@
 .Case("cp15", ReadTPMode::Cp15)
 .Case("soft", ReadTPMode::Soft)
 .Default(ReadTPMode::Invalid);
-if (ThreadPointer == ReadTPMode::Cp15 &&
-getARMSubArchVersionNumber(Triple) < 7 &&
-llvm::ARM::parseArch(Triple.getArchName()) !=
-llvm::ARM::ArchKind::ARMV6T2) {
+if (ThreadPointer == ReadTPMode::Cp15 && !isHardTPSupported(Triple)) {
   D.Diag(diag::err_target_unsupported_tp_hard) << Triple.getArchName();
   return ReadTPMode::Invalid;
 }
@@ -430,7 +437,6 @@
   bool KernelOrKext =
   Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
   arm::FloatABI ABI = arm::getARMFloatABI(D, Triple, Args);
-  arm::ReadTPMode ThreadPointer = arm::getReadTPMode(D, Args, Triple);
   llvm::Optional> WaCPU, WaFPU, WaHDiv,
   WaArch;
 
@@ -482,7 

[PATCH] D106585: Fix clang debug info irgen of i128 enums

2021-12-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: aeubanks.
rnk added a comment.

Thanks for the reduction, it sounds like there is something wrong with the way 
DIEnumerator is uniqued in the LLVMContext. I probably don't have time to 
follow up on this, but maybe @dblaikie and @aeubanks can help out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106585

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


[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

2021-12-03 Thread Cullen Rhodes via Phabricator via cfe-commits
c-rhodes added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:476-484
+  assert(LangOpts.VScaleMin && "vscale min must be greater than 0!");
+
+  if (LangOpts.VScaleMax)
 return std::pair(LangOpts.VScaleMin,
  LangOpts.VScaleMax);
+
   if (hasFeature("sve"))

paulwalker-arm wrote:
> This looks like a change of behaviour to me.  Previously the command line 
> flags would override the "sve" default but now that only happens when the 
> user specifies a maximum value.  That means the interface can no longer be 
> used to force truly width agnostic values.
> This looks like a change of behaviour to me.  Previously the command line 
> flags would override the "sve" default but now that only happens when the 
> user specifies a maximum value.  That means the interface can no longer be 
> used to force truly width agnostic values.

I think the issue here is the default of 1 for min would always trigger `if 
(LangOpts.VScaleMin || LangOpts.VScaleMax)` overriding the SVE default. Perhaps 
the default can be removed from the driver option and handled here, i.e.

```
if (LangOpts.VScaleMin || LangOpts.VScaleMax)
return std::pair(LangOpts.VScaleMin ? 
LangOpts.VScaleMin : 1,
 LangOpts.VScaleMax);
```




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

https://reviews.llvm.org/D113294

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


[PATCH] D115021: [funcatts] Rewrite callsite operand handling in memory access inference

2021-12-03 Thread Philip Reames via Phabricator via cfe-commits
reames planned changes to this revision.
reames added a comment.

Needs reworked over previous patch in stack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115021

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


[PATCH] D115003: [funcattrs] Infer writeonly argument attribute [part 2]

2021-12-03 Thread Philip Reames via Phabricator via cfe-commits
reames planned changes to this revision.
reames added a comment.

Has the same propagation bug fixed in 7b54de5f 
, need to 
rework.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115003

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


[PATCH] D113319: [clang-format] Improve require and concept handling

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

Still WIP, not marking as changes planned, so that it pops up on your list and 
you can give me feedback. :)




Comment at: clang/lib/Format/Format.cpp:1213
+  // This is open for discussions! When will LLVM adapt C++20?
+  LLVMStyle.RequiresClausePositionForClasses = FormatStyle::RCPS_OwnLine;
+  LLVMStyle.RequiresClausePositionForFunctions = FormatStyle::RCPS_OwnLine;

What are your opinions on that?



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:435
 
-void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
+/// \brief Parses a level, that is ???.
+/// \param HasOpeningBrace If that level is started by an opening brace.

Does anyone know for sure what the level is? I have an idea, but I'm not 100% 
sure yet.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2643
+  RequiresToken->setType(TT_RequiresClauseForFunctions);
+// Else: We are a requires clause within a requires expression.
+// The Semi is eaten by the caller, he also adds the unwrapped line.

Here I have an requires for a requires clause which did not get a type. Same as 
requires for requires expressions right now.

Should I tag all of them, even if I do not use the type right now?



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2727
 
-void UnwrappedLineParser::parseRequires() {
+/*void UnwrappedLineParser::parseRequires() {
   assert(FormatTok->Tok.is(tok::kw_requires) && "'requires' expected");

This will be removed.



Comment at: clang/unittests/Format/FormatTest.cpp:22715-22717
+   "  typename T::Bar;\n"
+   "  { t.bar() } -> std::same_as;\n"
+   "}\n"

Should these lines be indented because requires is indented, or not?


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

https://reviews.llvm.org/D113319

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


[PATCH] D115067: [clang-format][NFC] Use range based for

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: clang/lib/Format/TokenAnnotator.cpp:2376
 void TokenAnnotator::annotate(AnnotatedLine ) {
-  for (SmallVectorImpl::iterator I = Line.Children.begin(),
-  E = Line.Children.end();
-   I != E; ++I) {
-annotate(**I);
-  }
+  for (auto  : Line.Children)
+annotate(*Child);

Nit: I'd keep the type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115067

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


[PATCH] D115049: Fall back on Android triple w/o API level for runtimes search

2021-12-03 Thread Collin Baker via Phabricator via cfe-commits
collinbaker created this revision.
Herald added subscribers: abrachet, danielkiss, kristof.beyls.
collinbaker requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang searches for runtimes (e.g. libclang_rt*) first in a
subdirectory named for the target triple (corresponding to
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON), then if it's not found uses
.../lib//libclang_rt* with a suffix corresponding to the arch and
environment name.

Android triples optionally include an API level indicating the minimum
Android version to be run on
(e.g. aarch64-unknown-linux-android21). When compiler-rt is built with
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON this API level is part of the
output path.

Linking code built for a later API level against a runtime built for
an earlier one is safe. In projects with several API level targets
this is desireable to avoid re-building the same runtimes many
times. This is difficult with the current runtime search method: if
the API levels don't exactly match Clang gives up on the per-target
runtime directory path.

To enable this more simply, this change tries target triple without
the API level before falling back on the old layout.

Another option would be to try every API level in the triple,
e.g. check aarch-64-unknown-linux-android21, then ...20, then ...19,
etc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115049

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-unknown-linux-android/.keep
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-unknown-linux-android/libclang_rt.builtins.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-unknown-linux-android21/.keep
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/lib/aarch64-unknown-linux-android21/libclang_rt.builtins.a
  clang/test/Driver/linux-per-target-runtime-dir.c

Index: clang/test/Driver/linux-per-target-runtime-dir.c
===
--- clang/test/Driver/linux-per-target-runtime-dir.c
+++ clang/test/Driver/linux-per-target-runtime-dir.c
@@ -25,3 +25,21 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664 %s
 // CHECK-FILE-NAME-X8664: lib{{/|\\}}x86_64-unknown-linux-gnu{{/|\\}}libclang_rt.builtins.a
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=aarch64-unknown-linux-android21 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID21 %s
+// CHECK-FILE-NAME-ANDROID21: lib{{/|\\}}aarch64-unknown-linux-android21{{/|\\}}libclang_rt.builtins.a
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=aarch64-unknown-linux-android23 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID23 %s
+// CHECK-FILE-NAME-ANDROID23: lib{{/|\\}}aarch64-unknown-linux-android{{/|\\}}libclang_rt.builtins.a
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=aarch64-unknown-linux-android \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID %s
+// CHECK-FILE-NAME-ANDROID: lib{{/|\\}}aarch64-unknown-linux-android{{/|\\}}libclang_rt.builtins.a
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -191,9 +191,11 @@
 
   auto FilePaths = [&](const Multilib ) -> std::vector {
 std::vector FP;
-SmallString<128> P(getStdlibPath());
-llvm::sys::path::append(P, M.gccSuffix());
-FP.push_back(std::string(P.str()));
+for (const std::string  : getStdlibPaths()) {
+  SmallString<128> P(Path);
+  llvm::sys::path::append(P, M.gccSuffix());
+  FP.push_back(std::string(P.str()));
+}
 return FP;
   };
 
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -75,17 +75,16 @@
  const ArgList )
 : D(D), Triple(T), Args(Args), CachedRTTIArg(GetRTTIArgument(Args)),
   CachedRTTIMode(CalculateRTTIMode(Args, Triple, CachedRTTIArg)) {
-  std::string RuntimePath = getRuntimePath();
-  if (getVFS().exists(RuntimePath))
-getLibraryPaths().push_back(RuntimePath);
-
-  std::string StdlibPath = getStdlibPath();
-  if (getVFS().exists(StdlibPath))
-

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

2021-12-03 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment.

@martong

> Nice, assiduous work!

Many thanks for your time! Your work is not less assiduous!

> (LHS, RHS) swaps

it doesn't really matter, as the operation is comutative, but, yes, it does 
matter to depict the particular test case. I'll revise twise LHS-RHS's.

> I am going to continue with the next patch in the stack. I recognize that the 
> handling of casts is very important and problems related to not handling them 
> occurs even more frequently as other parts of the engine evolve (e.g. 
> https://reviews.llvm.org/D113753#3167134)

Aha. I saw you patch. I'll join to review it.




Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:441
+  //___/__/_\__\___   ___/___\___
+  this->checkUnite({{MID, MID}}, {A, D}, {{A, D}});
+  this->checkUnite({{B, C}}, {A, D}, {{A, D}});

martong wrote:
> 
There are three overloads of `checkUnite` which correspond to three overloads 
of `RangeSet::unite`. I made it consistent to other check-functions (`add` 
e.g.).



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:449
+  //___/___\___   ___/___\___
+  this->checkUnite({{MIN, MIN}}, MIN, {{MIN, MIN}});
+  this->checkUnite({{A, B}}, {A, B}, {{A, B}});

martong wrote:
> I think, either we should use `{{X, X}}` or `X` everywhere, but not mixed. 
This tests different oveloaded versions of `RangeSet::unite`.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:528-529
+  //_/__\_/__\_/\_/\_/\_   _/__\_/__\_/\_/\_/\_
+  this->checkUnite({{MIN, A}, {A + 2, B}}, {{MID, C}, {C + 2, D - 2}, {D, 
MAX}},
+   {{MIN, A}, {A + 2, B}, {MID, C}, {C + 2, D - 2}, {D, MAX}});
+  this->checkUnite({{MIN, MIN}, {A, A}}, {{B, B}, {C, C}, {MAX, MAX}},

martong wrote:
> I think we could better format these more complex cases.
clang-fromat acts on its own. But I agree, it looks the way better. I'll 
consider wrapping it into `// clang-format on/off` directives.



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:590-592
+  this->checkUnite({{A, B - 1}, {B + 1, C - 1}, {C + 2, D}, {MAX - 1, MAX}},
+   {{MIN, MIN}, {B, MID}, {MID + 1, C}, {C + 4, D - 1}},
+   {{MIN, MIN}, {A, C}, {C + 2, D}, {MAX - 1, MAX}});

martong wrote:
> martong wrote:
> > 
> What do you think about this format? The result can be easily verified this 
> way I think, but a bit ugly ...
I think ASCII art does this job. Let code look as code :)


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

https://reviews.llvm.org/D99797

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


[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

2021-12-03 Thread Cullen Rhodes via Phabricator via cfe-commits
c-rhodes updated this revision to Diff 391668.
c-rhodes added a comment.

Revert to previous behaviour where both the min/max Clang flags override SVE.


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

https://reviews.llvm.org/D113294

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  clang/test/Frontend/aarch64-vscale-min.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/InstCombine/icmp-vscale.ll
  llvm/test/Transforms/InstCombine/vscale_sext_and_zext.ll
  llvm/test/Transforms/InstCombine/vscale_trunc.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
  llvm/test/Verifier/vscale_range.ll

Index: llvm/test/Verifier/vscale_range.ll
===
--- llvm/test/Verifier/vscale_range.ll
+++ llvm/test/Verifier/vscale_range.ll
@@ -1,4 +1,7 @@
 ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
 
+; CHECK: 'vscale_range' minimum must be greater than 0
+declare i8* @a(i32*) vscale_range(0, 1)
+
 ; CHECK: 'vscale_range' minimum cannot be greater than maximum
 declare i8* @b(i32*) vscale_range(8, 1)
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -175,7 +175,7 @@
   ret i32 %tmp5
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -53,7 +53,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -90,7 +90,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
@@ -59,7 +59,7 @@
   ret void
 }
 
-attributes #0 = { "target-features"="+neon,+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+neon,+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
@@ -153,7 +153,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
+++ 

[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, curdeius.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Give I[1] a name:
  - Easier to understand
  - Easier to debug (since you don't go through operator[] everytime)
- TheLine->First != TheLine->Last follows since last is a l brace and first 
isn't.
- Factor the check for is(tok::l_brace) out.
- Drop else after return.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115060

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp

Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -211,10 +211,11 @@
 const AnnotatedLine *TheLine = *I;
 if (TheLine->Last->is(TT_LineComment))
   return 0;
-if (I[1]->Type == LT_Invalid || I[1]->First->MustBreakBefore)
+const auto  = *I[1];
+if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore)
   return 0;
 if (TheLine->InPPDirective &&
-(!I[1]->InPPDirective || I[1]->First->HasUnescapedNewline))
+(!NextLine.InPPDirective || NextLine.First->HasUnescapedNewline))
   return 0;
 
 if (Style.ColumnLimit > 0 && Indent > Style.ColumnLimit)
@@ -231,13 +232,13 @@
 if (TheLine->Last->is(TT_FunctionLBrace) &&
 TheLine->First == TheLine->Last &&
 !Style.BraceWrapping.SplitEmptyFunction &&
-I[1]->First->is(tok::r_brace))
+NextLine.First->is(tok::r_brace))
   return tryMergeSimpleBlock(I, E, Limit);
 
 // Handle empty record blocks where the brace has already been wrapped
 if (TheLine->Last->is(tok::l_brace) && TheLine->First == TheLine->Last &&
 I != AnnotatedLines.begin()) {
-  bool EmptyBlock = I[1]->First->is(tok::r_brace);
+  bool EmptyBlock = NextLine.First->is(tok::r_brace);
 
   const FormatToken *Tok = I[-1]->First;
   if (Tok && Tok->is(tok::comment))
@@ -267,7 +268,7 @@
 bool MergeShortFunctions =
 Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
 (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
- I[1]->First->is(tok::r_brace)) ||
+ NextLine.First->is(tok::r_brace)) ||
 (Style.AllowShortFunctionsOnASingleLine & FormatStyle::SFS_InlineOnly &&
  TheLine->Level != 0);
 
@@ -312,48 +313,49 @@
   return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
 }
 // Try to merge a control statement block with left brace unwrapped
-if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
+if (TheLine->Last->is(tok::l_brace) &&
 TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
   return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
  ? tryMergeSimpleBlock(I, E, Limit)
  : 0;
 }
 // Try to merge a control statement block with left brace wrapped
-if (I[1]->First->is(tok::l_brace) &&
-(TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
- tok::kw_for, tok::kw_switch, tok::kw_try,
- tok::kw_do, TT_ForEachMacro) ||
- (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
-  TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
-Style.BraceWrapping.AfterControlStatement ==
-FormatStyle::BWACS_MultiLine) {
-  // If possible, merge the next line's wrapped left brace with the current
-  // line. Otherwise, leave it on the next line, as this is a multi-line
-  // control statement.
-  return (Style.ColumnLimit == 0 ||
-  TheLine->Last->TotalLength <= Style.ColumnLimit)
- ? 1
- : 0;
-} else if (I[1]->First->is(tok::l_brace) &&
-   TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
-   tok::kw_for)) {
-  return (Style.BraceWrapping.AfterControlStatement ==
-  FormatStyle::BWACS_Always)
- ? tryMergeSimpleBlock(I, E, Limit)
- : 0;
-} else if (I[1]->First->is(tok::l_brace) &&
-   TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&
-   Style.BraceWrapping.AfterControlStatement ==
-   FormatStyle::BWACS_MultiLine) {
-  // This case if different from the upper BWACS_MultiLine processing
-  // in that a preceding r_brace is not on the same line as else/catch
-  // most likely because of BeforeElse/BeforeCatch set to true.
-  // If the line length doesn't fit ColumnLimit, leave l_brace on the
-  // next line to respect the BWACS_MultiLine.
-  return 

[PATCH] D115064: [clang-format][NFC] Replace deque with vector

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius, owenpan.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I think the deque was chosen because of a better push_front, but in combination 
with llvm::reverse the push_back'ed vector should be the better choice.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115064

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1094,22 +1094,22 @@
   /// Applies the best formatting by reconstructing the path in the
   /// solution space that leads to \c Best.
   void reconstructPath(LineState , StateNode *Best) {
-std::deque Path;
+std::vector Path;
 // We do not need a break before the initial token.
 while (Best->Previous) {
-  Path.push_front(Best);
+  Path.push_back(Best);
   Best = Best->Previous;
 }
-for (auto I = Path.begin(), E = Path.end(); I != E; ++I) {
+for (const auto& Node : llvm::reverse(Path)) {
   unsigned Penalty = 0;
-  formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
-  Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
+  formatChildren(State, Node->NewLine, /*DryRun=*/false, Penalty);
+  Penalty += Indenter->addTokenToState(State, Node->NewLine, false);
 
   LLVM_DEBUG({
-printLineState((*I)->Previous->State);
-if ((*I)->NewLine) {
+printLineState(Node->Previous->State);
+if (Node->NewLine) {
   llvm::dbgs() << "Penalty for placing "
-   << (*I)->Previous->State.NextToken->Tok.getName()
+   << Node->Previous->State.NextToken->Tok.getName()
<< " on a new line: " << Penalty << "\n";
 }
   });


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1094,22 +1094,22 @@
   /// Applies the best formatting by reconstructing the path in the
   /// solution space that leads to \c Best.
   void reconstructPath(LineState , StateNode *Best) {
-std::deque Path;
+std::vector Path;
 // We do not need a break before the initial token.
 while (Best->Previous) {
-  Path.push_front(Best);
+  Path.push_back(Best);
   Best = Best->Previous;
 }
-for (auto I = Path.begin(), E = Path.end(); I != E; ++I) {
+for (const auto& Node : llvm::reverse(Path)) {
   unsigned Penalty = 0;
-  formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
-  Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
+  formatChildren(State, Node->NewLine, /*DryRun=*/false, Penalty);
+  Penalty += Indenter->addTokenToState(State, Node->NewLine, false);
 
   LLVM_DEBUG({
-printLineState((*I)->Previous->State);
-if ((*I)->NewLine) {
+printLineState(Node->Previous->State);
+if (Node->NewLine) {
   llvm::dbgs() << "Penalty for placing "
-   << (*I)->Previous->State.NextToken->Tok.getName()
+   << Node->Previous->State.NextToken->Tok.getName()
<< " on a new line: " << Penalty << "\n";
 }
   });
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115015: CodeGen: Strip exception specifications from function types in CFI type names.

2021-12-03 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

I patched this in and verified that it fixes the make_top_domain_list_variables 
issue in https://bugs.chromium.org/p/chromium/issues/detail?id=1273966 and went 
ahead and landed this. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115015

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


[PATCH] D115069: [clang-format][NFC] Merge another two calls to isOneOf

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1294-1296
+  Previous->isOneOf(tok::l_paren, tok::comma, tok::colon, 
TT_BinaryOperator,
+TT_ConditionalExpr) &&
   !Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {

Unrelated to your change, it only shed light on this:
This condition looks strange to me. How can `Previous` be at the same time two 
different `TokenType`s (?), for instance, both `TT_BinaryOperator` and 
`TT_DictLiteral`?

Shouldn't it be this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115069

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


[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM. Don't forget to reformat please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115060

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


[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-03 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray updated this revision to Diff 391718.
lichray added a comment.

Add EOL to source file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

Files:
  clang/lib/AST/APValue.cpp
  clang/test/SemaTemplate/temp_arg_string_printing.cpp

Index: clang/test/SemaTemplate/temp_arg_string_printing.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_string_printing.cpp
@@ -0,0 +1,137 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -ast-print %s | FileCheck %s
+
+using size_t = __SIZE_TYPE__;
+static_assert(__has_builtin(__make_integer_seq));
+
+template  class idx_seq {};
+template  using make_idx_seq = __make_integer_seq;
+
+template 
+struct Str {
+  constexpr Str(CharT const ()[N]) : Str(s, make_idx_seq()) {}
+  CharT value[N];
+
+private:
+  template 
+  constexpr Str(CharT const ()[N], idx_seq) : value{s[I]...} {}
+};
+
+template  class ASCII {};
+
+void narrow() {
+  // CHECK{LITERAL}: ASCII<{""}>
+  new ASCII<"">;
+  // CHECK{LITERAL}: ASCII<{"the quick brown fox jumps"}>
+  new ASCII<"the quick brown fox jumps">;
+  // CHECK{LITERAL}: ASCII<{"OVER THE LAZY DOG 0123456789"}>
+  new ASCII<"OVER THE LAZY DOG 0123456789">;
+  // CHECK{LITERAL}: ASCII<{"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{"escape\0"}>
+  new ASCII<"escape\0">;
+  // CHECK{LITERAL}: ASCII<{"escape\r\n"}>
+  new ASCII<"escape\r\n">;
+  // CHECK{LITERAL}: ASCII<{"escape\\\t\'\f\v"}>
+  new ASCII<"escape\\\t'\f\v">;
+  // CHECK{LITERAL}: ASCII<{"escape\a\bc"}>
+  new ASCII<"escape\a\b\c">;
+  // CHECK{LITERAL}: ASCII<{{110, 111, 116, 17, 0}}>
+  new ASCII<"not\x11">;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 127, 16, 1, 32, 97, 98, 99, 0}}>
+  new ASCII<"\x12\x14\x7f\x10\x01 abc">;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 127, 16, 1, 32, 97, 98, 99, 100, ...}}>
+  new ASCII<"\x12\x14\x7f\x10\x01 abcd">;
+  // CHECK{LITERAL}: ASCII<{"print more characters as string"}>
+  new ASCII<"print more characters as string">;
+  // CHECK{LITERAL}: ASCII<{"print even more characters as string"}>
+  new ASCII<"print even more characters as string">;
+  // CHECK{LITERAL}: ASCII<{"print many characters no more t[...]"}>
+  new ASCII<"print many characters no more than a limit">;
+  // CHECK{LITERAL}: ASCII<{"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"}>
+  new ASCII<"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0">;
+  // CHECK{LITERAL}: ASCII<{"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n[...]"}>
+  new ASCII<"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n">;
+}
+
+void wide() {
+  // CHECK{LITERAL}: ASCII<{L""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"the quick brown fox jumps"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"OVER THE LAZY DOG 0123456789"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{L"escape\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\\\t\f\v"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\a\bc"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{110, 111, 116, 17, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 255, 22909, 136, 32, 97, 98, 99, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 255, 22909, 136, 32, 97, 98, 99, 100, ...}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print more characters as string"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print even more characters as string"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print many characters no more t[...]"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n[...]"}>
+  new ASCII;
+}
+
+void utf8()
+{
+  // CHECK{LITERAL}: ASCII<{u8""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{u8"escape\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{229, 165, 189, 239, 191, 189, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"print many characters no more t[...]"}>
+  new ASCII;
+}
+
+void utf16()
+{
+  // CHECK{LITERAL}: ASCII<{u""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{u"escape\0"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{22909, 65533, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u"print many characters no more t[...]"}>
+  new ASCII;
+}
+
+void utf32()
+{
+  // CHECK{LITERAL}: ASCII<{U""}>
+  new ASCII;
+  // 

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment.

FWLIW, I'm strongly in favor of "Pass out-parameters by pointer," for the 
reason Marek said (and the reason Google, Bloomberg, Facebook, Mongo, etc, do 
it) — it makes life easier for the reader of the calling code. Especially for 
e.g. `addNextStateToQueue(Penalty, Node, /*NewLine=*/false, Count, Queue)`, I 
don't think it's at all obvious that this is going to modify the value of 
`Count`!
But this isn't my code and I don't know what LLVM's house style is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115061

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


[PATCH] D115065: [clang-format][NFC] Merge two calls of isOneOf

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

I guess this is matter of taste whether to call `isOneOf` separately for 
`TokenKind`s and for `TokenType`s (but +1 for removing the repetition).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115065

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


[clang] 7b54de5 - [funcattrs] Fix a bug in recently introduced writeonly argument inference

2021-12-03 Thread Philip Reames via cfe-commits

Author: Philip Reames
Date: 2021-12-03T08:57:15-08:00
New Revision: 7b54de5feffedfc08e5a02d6c9e27c54e3b7f119

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

LOG: [funcattrs] Fix a bug in recently introduced writeonly argument inference

This fixes a bug in 740057d.  There's two ways to describe the issue:
* One caller hasn't yet proven nocapture on the argument.  Given that, the 
inference routine is responsible for bailing out on a potential capture.
* Even if we know the argument is nocapture, the access inference needs to 
traverse the exact set of users the capture tracking would (or exit 
conservatively).  Even if capture tracking can prove a store is non-capturing 
(e.g. to a local alloc which doesn't escape), we still need to track the copy 
of the pointer to see if it's later reloaded and accessed again.

Note that all the test changes except the newly added ones appear to be false 
negatives.  That is, cases where we could prove writeonly, but the current code 
isn't strong enough.  That's why I didn't spot this originally.

Added: 


Modified: 
clang/test/CodeGen/ms-mixed-ptr-sizes.c
llvm/lib/Transforms/IPO/FunctionAttrs.cpp
llvm/test/Feature/OperandBundles/pr26510.ll
llvm/test/Transforms/Coroutines/coro-async.ll
llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll
llvm/test/Transforms/FunctionAttrs/nocapture.ll
llvm/test/Transforms/FunctionAttrs/readattrs.ll
llvm/test/Transforms/FunctionAttrs/writeonly.ll

Removed: 




diff  --git a/clang/test/CodeGen/ms-mixed-ptr-sizes.c 
b/clang/test/CodeGen/ms-mixed-ptr-sizes.c
index 294a8910e13e3..ececa42a4c4dd 100644
--- a/clang/test/CodeGen/ms-mixed-ptr-sizes.c
+++ b/clang/test/CodeGen/ms-mixed-ptr-sizes.c
@@ -7,32 +7,32 @@ struct Foo {
 };
 void use_foo(struct Foo *f);
 void test_sign_ext(struct Foo *f, int * __ptr32 __sptr i) {
-// X64-LABEL: define dso_local void @test_sign_ext({{.*}}i32 addrspace(270)* 
writeonly %i)
-// X86-LABEL: define dso_local void @test_sign_ext(%struct.Foo* %f, i32* 
writeonly %i)
+// X64-LABEL: define dso_local void @test_sign_ext({{.*}}i32 addrspace(270)* 
%i)
+// X86-LABEL: define dso_local void @test_sign_ext(%struct.Foo* %f, i32* %i)
 // X64: %{{.+}} = addrspacecast i32 addrspace(270)* %i to i32*
 // X86: %{{.+}} = addrspacecast i32* %i to i32 addrspace(272)*
   f->p64 = i;
   use_foo(f);
 }
 void test_zero_ext(struct Foo *f, int * __ptr32 __uptr i) {
-// X64-LABEL: define dso_local void @test_zero_ext({{.*}}i32 addrspace(271)* 
writeonly %i)
-// X86-LABEL: define dso_local void @test_zero_ext({{.*}}i32 addrspace(271)* 
writeonly %i)
+// X64-LABEL: define dso_local void @test_zero_ext({{.*}}i32 addrspace(271)* 
%i)
+// X86-LABEL: define dso_local void @test_zero_ext({{.*}}i32 addrspace(271)* 
%i)
 // X64: %{{.+}} = addrspacecast i32 addrspace(271)* %i to i32*
 // X86: %{{.+}} = addrspacecast i32 addrspace(271)* %i to i32 addrspace(272)*
   f->p64 = i;
   use_foo(f);
 }
 void test_trunc(struct Foo *f, int * __ptr64 i) {
-// X64-LABEL: define dso_local void @test_trunc(%struct.Foo* %f, i32* 
writeonly %i)
-// X86-LABEL: define dso_local void @test_trunc({{.*}}i32 addrspace(272)* 
writeonly %i)
+// X64-LABEL: define dso_local void @test_trunc(%struct.Foo* %f, i32* %i)
+// X86-LABEL: define dso_local void @test_trunc({{.*}}i32 addrspace(272)* %i)
 // X64: %{{.+}} = addrspacecast i32* %i to i32 addrspace(270)*
 // X86: %{{.+}} = addrspacecast i32 addrspace(272)* %i to i32*
   f->p32 = i;
   use_foo(f);
 }
 void test_noop(struct Foo *f, int * __ptr32 i) {
-// X64-LABEL: define dso_local void @test_noop({{.*}}i32 addrspace(270)* 
writeonly %i)
-// X86-LABEL: define dso_local void @test_noop({{.*}}i32* writeonly %i)
+// X64-LABEL: define dso_local void @test_noop({{.*}}i32 addrspace(270)* %i)
+// X86-LABEL: define dso_local void @test_noop({{.*}}i32* %i)
 // X64-NOT: addrspacecast
 // X86-NOT: addrspacecast
   f->p32 = i;
@@ -40,8 +40,8 @@ void test_noop(struct Foo *f, int * __ptr32 i) {
 }
 
 void test_other(struct Foo *f, __attribute__((address_space(10))) int *i) {
-// X64-LABEL: define dso_local void @test_other({{.*}}i32 addrspace(10)* 
writeonly %i)
-// X86-LABEL: define dso_local void @test_other({{.*}}i32 addrspace(10)* 
writeonly %i)
+// X64-LABEL: define dso_local void @test_other({{.*}}i32 addrspace(10)* %i)
+// X86-LABEL: define dso_local void @test_other({{.*}}i32 addrspace(10)* %i)
 // X64: %{{.+}} = addrspacecast i32 addrspace(10)* %i to i32 addrspace(270)*
 // X86: %{{.+}} = addrspacecast i32 addrspace(10)* %i to i32*
   f->p32 = (int * __ptr32)i;

diff  --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp 
b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index 1ad0055b56382..2cee9c0b4766a 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ 

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3052
+// & 1
+if (Right.Tok.isLiteral())
+  return true;

HazardyKnusperkeks wrote:
> Is this valid code? Or did we just wrongly assign PointerOrReference? I'd say 
> after that there can not be a literal in valid code, thus we do not need to 
> handle it.
Ok so as you see I broken out the compound statement, to be honest I find these 
compound if's unreadable and I can't for the life of me work out what case they 
are trying to handle. (I want to do this more)..

Please no one say doing it separately is slower! without measuring it..we are 
talking ns difference if anything at all.

I agree I truly believe we are mis assigning PointerOrReference/BinaryOperator 
this as I mentioned  in
https://lists.llvm.org/pipermail/cfe-dev/2021-December/069486.html is a major 
issue and I think from time to time these rules try to correct that situation, 
in this case & is the logical operation on a reference.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3065
+// for (auto a = 0, b = 0; const auto& c : {1, 2, 3})
+if (Left.Previous && Left.Previous->is(tok::kw_auto) &&
+Right.is(tok::identifier))

HazardyKnusperkeks wrote:
> Do we need this just for `auto`? What when `auto` is replaced with `int`?
Man! you right...duh how did I miss that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115050

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


[PATCH] D106585: Fix clang debug info irgen of i128 enums

2021-12-03 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D106585#3170019 , @aeubanks wrote:

> In D106585#3169898 , @rnk wrote:
>
>> This usage of isSameValue seems suspicious: 
>> https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/LLVMContextImpl.h#L389
>>
>> It seems to allow the possibility that APInts of differing bitwidth compare 
>> equal, but the hash value incorporates the bitwidth, so they may be inserted 
>> into differing hash buckets.
>
> yup, also checking the bit width makes the non-determinism go away, I'll send 
> out a patch

https://reviews.llvm.org/D115054


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106585

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


[PATCH] D115066: [clang-format][NFC] Reorder conditions

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, curdeius.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Prefer to check the local variables first before dereferencing the pointer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115066

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2173,8 +2173,8 @@
 
   int CurrentPrecedence = getCurrentPrecedence();
 
-  if (Current && Current->is(TT_SelectorName) &&
-  Precedence == CurrentPrecedence) {
+  if (Precedence == CurrentPrecedence && Current &&
+  Current->is(TT_SelectorName)) {
 if (LatestOperator)
   addFakeParenthesis(Start, prec::Level(Precedence));
 Start = Current;


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2173,8 +2173,8 @@
 
   int CurrentPrecedence = getCurrentPrecedence();
 
-  if (Current && Current->is(TT_SelectorName) &&
-  Precedence == CurrentPrecedence) {
+  if (Precedence == CurrentPrecedence && Current &&
+  Current->is(TT_SelectorName)) {
 if (LatestOperator)
   addFakeParenthesis(Start, prec::Level(Precedence));
 Start = Current;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115070: [clang-format][NFC] Early return when nothing to do

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, owenpan, curdeius.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Do not compute SkipFirstExtraIndent just to see that there are no fake l parens.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115070

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1337,6 +1337,9 @@
 void ContinuationIndenter::moveStatePastFakeLParens(LineState ,
 bool Newline) {
   const FormatToken  = *State.NextToken;
+  if (Current.FakeLParens.empty())
+return;
+
   const FormatToken *Previous = Current.getPreviousNonComment();
 
   // Don't add extra indentation for the first fake parenthesis after


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1337,6 +1337,9 @@
 void ContinuationIndenter::moveStatePastFakeLParens(LineState ,
 bool Newline) {
   const FormatToken  = *State.NextToken;
+  if (Current.FakeLParens.empty())
+return;
+
   const FormatToken *Previous = Current.getPreviousNonComment();
 
   // Don't add extra indentation for the first fake parenthesis after
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115072: [clang-format][NFC] Use member directly

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius, owenpan.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Instead of passing it as argument to the member function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115072

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -62,7 +62,7 @@
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
   IndentForLevel.resize(Line.Level + 1);
-  Indent = getIndent(IndentForLevel, Line.Level);
+  Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
   Indent += Offset;
@@ -118,12 +118,12 @@
   /// \p IndentForLevel must contain the indent for the level \c l
   /// at \p IndentForLevel[l], or a value < 0 if the indent for
   /// that level is unknown.
-  unsigned getIndent(ArrayRef IndentForLevel, unsigned Level) {
+  unsigned getIndent(unsigned Level) const {
 if (IndentForLevel[Level] != -1)
   return IndentForLevel[Level];
 if (Level == 0)
   return 0;
-return getIndent(IndentForLevel, Level - 1) + Style.IndentWidth;
+return getIndent(Level - 1) + Style.IndentWidth;
   }
 
   const FormatStyle 


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -62,7 +62,7 @@
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
   IndentForLevel.resize(Line.Level + 1);
-  Indent = getIndent(IndentForLevel, Line.Level);
+  Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
   Indent += Offset;
@@ -118,12 +118,12 @@
   /// \p IndentForLevel must contain the indent for the level \c l
   /// at \p IndentForLevel[l], or a value < 0 if the indent for
   /// that level is unknown.
-  unsigned getIndent(ArrayRef IndentForLevel, unsigned Level) {
+  unsigned getIndent(unsigned Level) const {
 if (IndentForLevel[Level] != -1)
   return IndentForLevel[Level];
 if (Level == 0)
   return 0;
-return getIndent(IndentForLevel, Level - 1) + Style.IndentWidth;
+return getIndent(Level - 1) + Style.IndentWidth;
   }
 
   const FormatStyle 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D106585: Fix clang debug info irgen of i128 enums

2021-12-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

This usage of isSameValue seems suspicious: 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/LLVMContextImpl.h#L389

It seems to allow the possibility that APInts of differing bitwidth compare 
equal, but the hash value incorporates the bitwidth, so they may be inserted 
into differing hash buckets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106585

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


[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

2021-12-03 Thread Paul Walker via Phabricator via cfe-commits
paulwalker-arm added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:476-484
+  assert(LangOpts.VScaleMin && "vscale min must be greater than 0!");
+
+  if (LangOpts.VScaleMax)
 return std::pair(LangOpts.VScaleMin,
  LangOpts.VScaleMax);
+
   if (hasFeature("sve"))

c-rhodes wrote:
> paulwalker-arm wrote:
> > This looks like a change of behaviour to me.  Previously the command line 
> > flags would override the "sve" default but now that only happens when the 
> > user specifies a maximum value.  That means the interface can no longer be 
> > used to force truly width agnostic values.
> > This looks like a change of behaviour to me.  Previously the command line 
> > flags would override the "sve" default but now that only happens when the 
> > user specifies a maximum value.  That means the interface can no longer be 
> > used to force truly width agnostic values.
> 
> I think the issue here is the default of 1 for min would always trigger `if 
> (LangOpts.VScaleMin || LangOpts.VScaleMax)` overriding the SVE default. 
> Perhaps the default can be removed from the driver option and handled here, 
> i.e.
> 
> ```
> if (LangOpts.VScaleMin || LangOpts.VScaleMax)
> return std::pair(LangOpts.VScaleMin ? 
> LangOpts.VScaleMin : 1,
>  LangOpts.VScaleMax);
> ```
> 
> 
Is this enough?  I'm not sure it'll work because `LangOpts.VScaleMin` defaults 
to 1 and thus you'll always end up passing the first check, unless the user 
specifically uses `-mvscale-min=0` which they cannot because that'll result in 
`diag::err_cc1_unbounded_vscale_min`.

Do we need to link the LangOpt defaults to the attribute defaults?  I'm think 
that having the LangOpts default to zero is a good way to represent "value is 
unspecified" with regards to the `LangOpts.VScaleMin`.


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

https://reviews.llvm.org/D113294

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


[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, curdeius.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks 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/D115061

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1046,9 +1046,9 @@
 
   FormatDecision LastFormat = Node->State.NextToken->getDecision();
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, , );
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false, Count, Queue);
   if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/true, , );
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true, Count, Queue);
 }
 
 if (Queue.empty()) {
@@ -1074,7 +1074,7 @@
   /// Assume the current state is \p PreviousNode and has been reached with a
   /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.
   void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode,
-   bool NewLine, unsigned *Count, QueueType *Queue) {
+   bool NewLine, unsigned , QueueType ) {
 if (NewLine && !Indenter->canBreak(PreviousNode->State))
   return;
 if (!NewLine && Indenter->mustBreak(PreviousNode->State))
@@ -1087,8 +1087,8 @@
 
 Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
 
-Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node));
-++(*Count);
+Queue.push(QueueItem(OrderedPenalty(Penalty, Count), Node));
+++Count;
   }
 
   /// Applies the best formatting by reconstructing the path in the


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1046,9 +1046,9 @@
 
   FormatDecision LastFormat = Node->State.NextToken->getDecision();
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, , );
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false, Count, Queue);
   if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/true, , );
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true, Count, Queue);
 }
 
 if (Queue.empty()) {
@@ -1074,7 +1074,7 @@
   /// Assume the current state is \p PreviousNode and has been reached with a
   /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.
   void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode,
-   bool NewLine, unsigned *Count, QueueType *Queue) {
+   bool NewLine, unsigned , QueueType ) {
 if (NewLine && !Indenter->canBreak(PreviousNode->State))
   return;
 if (!NewLine && Indenter->mustBreak(PreviousNode->State))
@@ -1087,8 +1087,8 @@
 
 Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
 
-Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node));
-++(*Count);
+Queue.push(QueueItem(OrderedPenalty(Penalty, Count), Node));
+++Count;
   }
 
   /// Applies the best formatting by reconstructing the path in the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113369: [clang-format] Extend SpaceBeforeParens for requires

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks planned changes to this revision.
HazardyKnusperkeks marked an inline comment as done.
HazardyKnusperkeks added a comment.

Delayed until D113319  is resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113369

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


[PATCH] D115071: [clang-format][NFC] Use range based for for fake l parens

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115071

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


[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

What do we usually do for output parameters?
I'm ok with both refs and pointers. It seems to me to be google-style thingy to 
pass by pointer.
It is indeed clearer at the caller site that the passed variable will be 
modified.
Are you worried about any performance penalty when using pointers? If so, 
adding `nonnull` attribute or sth like this may help.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115061

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


[PATCH] D115064: [clang-format][NFC] Replace deque with vector

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

Deque, contrary to the vector, doesn't need to move the elements when growing. 
I'm not sure if that's relevant here though.
Have you checked what's on average the maximum size of `Path` on some larger 
repo?
Maybe using `llvm::SmallVector` with some well-thought (data-based) static 
number of elements would be better here, WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115064

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


[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

2021-12-03 Thread 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!




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:127
+// declared multiple times. The most common two cases are:
+// - Definition available in TU, only mark that one as usage. rest is
+//   likely to be unnecessary. this might result in false positives when an

i know it was me who didn't capitalize, but can you capitalize the first words 
:D
s/rest/Rest
s/this/This (line below)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114949

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


[PATCH] D113753: [Analyzer][Core] Better simplification in SimpleSValBuilder::evalBinOpNN

2021-12-03 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:372
   QualType resultTy)  {
   NonLoc InputLHS = lhs;
   NonLoc InputRHS = rhs;

steakhal wrote:
> @martong, you simplified the operands, but you overwrite only the lhs and 
> rhs. Shouldnt you overwite these as well?
> What is the purpose of these variables, do you know?
> @martong, you simplified the operands, but you overwrite only the lhs and 
> rhs. Shouldnt you overwite these as well?
> What is the purpose of these variables, do you know?

They are used exclusively to create `SymExpr`s with `makeSymExprValNN` when 
both lhs and rhs are symbols. If we were to simplify the `Input` variables then 
it might happen that we end up with a non symbol (i.e. a concrete int) and then 
`makeSymExprValNN` would fail miserably (would assert I guess). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113753

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


[PATCH] D114938: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

2021-12-03 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:51
+  // sub-trees and if a value is a constant we do the constant folding.
+  SVal simplifySValImpl(ProgramStateRef State, SVal V);
+

steakhal wrote:
> What about calling this `simplifySValOnce()`
Sounds better!



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1137
+  SVal SimplifiedVal = simplifySValImpl(State, Val);
+  // Do the simplification until we can.
+  while (SimplifiedVal != Val) {

steakhal wrote:
> You can remove this comment. Not really useful anyway.
Ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114938

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


[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

2021-12-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added a comment.

In D114949#3169074 , @kadircet wrote:

> thanks!

Aww, sorry, I capitalized/reformatted parts of them but missed the rest :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114949

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


[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

2021-12-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 391567.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Improve wording.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114949

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,10 +39,10 @@
   "class ^X;",
   "X *y;",
   },
-  // FIXME(kirillbobyrev): When definition is available, we don't need to
-  // mark forward declarations as used.
+  // When definition is available, we don't need to mark forward
+  // declarations as used.
   {
-  "class ^X {}; class ^X;",
+  "class ^X {}; class X;",
   "X *y;",
   },
   // We already have forward declaration in the main file, the other
@@ -51,17 +51,24 @@
   "class ^X {}; class X;",
   "class X; X *y;",
   },
+  // Nested class definition can occur outside of the parent class
+  // definition. Bar declaration should be visible to its definition but
+  // it will always be because we will mark Foo definition as used.
+  {
+  "class ^Foo { class Bar; };",
+  "class Foo::Bar {};",
+  },
   // TypedefType and UsingDecls
   {
   "using ^Integer = int;",
   "Integer x;",
   },
   {
-  "namespace ns { struct ^X; struct ^X {}; }",
-  "using ns::X;",
+  "namespace ns { void ^foo(); void ^foo() {} }",
+  "using ns::foo;",
   },
   {
-  "namespace ns { struct X; struct X {}; }",
+  "namespace ns { void foo(); void foo() {}; }",
   "using namespace ns;",
   },
   {
@@ -88,8 +95,8 @@
   },
   // Redecls
   {
-  "class ^X; class ^X{}; class ^X;",
-  "X *y;",
+  "void ^foo(); void ^foo() {} void ^foo();",
+  "void bar() { foo(); }",
   },
   // Constructor
   {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -122,16 +122,20 @@
   void add(const Decl *D) {
 if (!D || !isNew(D->getCanonicalDecl()))
   return;
-// If we see a declaration in the mainfile, skip all non-definition decls.
-// We only do this for classes because forward declarations are common and
-// we don't want to include every header that forward-declares the symbol
-// because they're often unrelated.
+// Special case RecordDecls, as it is common for them to be forward
+// declared multiple times. The most common cases are:
+// - Definition available in TU, only mark that one as usage. The rest is
+//   likely to be unnecessary. This might result in false positives when an
+//   internal definition is visible.
+// - There's a forward declaration in the main file, no need for other
+//   redecls.
 if (const auto *RD = llvm::dyn_cast(D)) {
-  if (SM.isInMainFile(RD->getMostRecentDecl()->getBeginLoc())) {
-if (const auto *Definition = RD->getMostRecentDecl()->getDefinition())
-  Result.insert(Definition->getLocation());
+  if (const auto *Definition = RD->getDefinition()) {
+Result.insert(Definition->getLocation());
 return;
   }
+  if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation()))
+return;
 }
 for (const Decl *Redecl : D->redecls())
   Result.insert(Redecl->getLocation());


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,10 +39,10 @@
   "class ^X;",
   "X *y;",
   },
-  // FIXME(kirillbobyrev): When definition is available, we don't need to
-  // mark forward declarations as used.
+  // When definition is available, we don't need to mark forward
+  // declarations as used.
   {
-  "class ^X {}; class ^X;",
+  "class ^X {}; class X;",
   "X *y;",
   },
   // We already have forward declaration in the main file, the other
@@ -51,17 +51,24 @@
   "class ^X {}; class X;",
   "class X; X *y;",
   },
+  // Nested class definition can occur outside of the parent class
+  // definition. Bar declaration should be visible to its definition but
+  // it will always be because we will mark Foo definition as used.
+  {
+  "class ^Foo { class Bar; 

[PATCH] D114317: [clang-tidy][WIP] Do not run perfect alias checks

2021-12-03 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

Kind ping to reviewers, I mostly want to know if you agree with the general 
direction so I can go ahead and implement the rest of the patch. It's a big 
change (replace all check registrations with the macro) so I'd like to avoid 
extra work if you are strongly against it.

@aaron.ballman I understand you are super busy and don't want to take any more 
time than needed from you. I'd just love to hear your thoughts about the two 
missing points that we discussed in the RFC. Nobody else seems to have raised 
similar concerns about it.

Really appreciate your time!


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

https://reviews.llvm.org/D114317

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


[PATCH] D112453: [Sema] When dereferencing a pointer of dependent type, infer the result type.

2021-12-03 Thread Clement Courbet via Phabricator via cfe-commits
courbet added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112453

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


[PATCH] D114968: [clang][deps] Avoid reading file for stat calls

2021-12-03 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 abandoned this revision.
jansvoboda11 added a comment.

Thanks for confirming!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114968

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


[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

2021-12-03 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 391566.
kbobyrev added a comment.

Fix typos.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114949

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,10 +39,10 @@
   "class ^X;",
   "X *y;",
   },
-  // FIXME(kirillbobyrev): When definition is available, we don't need to
-  // mark forward declarations as used.
+  // When definition is available, we don't need to mark forward
+  // declarations as used.
   {
-  "class ^X {}; class ^X;",
+  "class ^X {}; class X;",
   "X *y;",
   },
   // We already have forward declaration in the main file, the other
@@ -51,17 +51,24 @@
   "class ^X {}; class X;",
   "class X; X *y;",
   },
+  // Nested class definition can occur outside of the parent class
+  // definition. Bar declaration should be visible to its definition but
+  // it will always be because we will mark Foo definition as used.
+  {
+  "class ^Foo { class Bar; };",
+  "class Foo::Bar {};",
+  },
   // TypedefType and UsingDecls
   {
   "using ^Integer = int;",
   "Integer x;",
   },
   {
-  "namespace ns { struct ^X; struct ^X {}; }",
-  "using ns::X;",
+  "namespace ns { void ^foo(); void ^foo() {} }",
+  "using ns::foo;",
   },
   {
-  "namespace ns { struct X; struct X {}; }",
+  "namespace ns { void foo(); void foo() {}; }",
   "using namespace ns;",
   },
   {
@@ -88,8 +95,8 @@
   },
   // Redecls
   {
-  "class ^X; class ^X{}; class ^X;",
-  "X *y;",
+  "void ^foo(); void ^foo() {} void ^foo();",
+  "void bar() { foo(); }",
   },
   // Constructor
   {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -122,16 +122,20 @@
   void add(const Decl *D) {
 if (!D || !isNew(D->getCanonicalDecl()))
   return;
-// If we see a declaration in the mainfile, skip all non-definition decls.
-// We only do this for classes because forward declarations are common and
-// we don't want to include every header that forward-declares the symbol
-// because they're often unrelated.
+// Special case RecordDecls, as it is common for them to be forward
+// declared multiple times. The most common two cases are:
+// - Definition available in TU, only mark that one as usage. The rest is
+//   likely to be unnecessary. This might result in false positives when an
+//   internal definition is visible.
+// - There's a forward declaration in the main file, no need for other
+//   redecls.
 if (const auto *RD = llvm::dyn_cast(D)) {
-  if (SM.isInMainFile(RD->getMostRecentDecl()->getBeginLoc())) {
-if (const auto *Definition = RD->getMostRecentDecl()->getDefinition())
-  Result.insert(Definition->getLocation());
+  if (const auto *Definition = RD->getDefinition()) {
+Result.insert(Definition->getLocation());
 return;
   }
+  if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation()))
+return;
 }
 for (const Decl *Redecl : D->redecls())
   Result.insert(Redecl->getLocation());


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,10 +39,10 @@
   "class ^X;",
   "X *y;",
   },
-  // FIXME(kirillbobyrev): When definition is available, we don't need to
-  // mark forward declarations as used.
+  // When definition is available, we don't need to mark forward
+  // declarations as used.
   {
-  "class ^X {}; class ^X;",
+  "class ^X {}; class X;",
   "X *y;",
   },
   // We already have forward declaration in the main file, the other
@@ -51,17 +51,24 @@
   "class ^X {}; class X;",
   "class X; X *y;",
   },
+  // Nested class definition can occur outside of the parent class
+  // definition. Bar declaration should be visible to its definition but
+  // it will always be because we will mark Foo definition as used.
+  {
+  "class ^Foo { class Bar; };",
+  "class Foo::Bar {};",
+  

[PATCH] D114688: [Clang] Add __builtin_elementwise_ceil

2021-12-03 Thread Jun Zhang via Phabricator via cfe-commits
junaire updated this revision to Diff 391574.
junaire added a comment.

No more lambdas


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

https://reviews.llvm.org/D114688

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-elementwise-math.c
  clang/test/Sema/builtins-elementwise-math.c
  clang/test/SemaCXX/builtins-elementwise-math.cpp

Index: clang/test/SemaCXX/builtins-elementwise-math.cpp
===
--- clang/test/SemaCXX/builtins-elementwise-math.cpp
+++ clang/test/SemaCXX/builtins-elementwise-math.cpp
@@ -36,3 +36,10 @@
   static_assert(!is_const::value);
   static_assert(!is_const::value);
 }
+
+void test_builtin_elementwise_ceil() {
+  const float a = 42.0;
+  float b = 42.3;
+  static_assert(!is_const::value);
+  static_assert(!is_const::value);
+}
Index: clang/test/Sema/builtins-elementwise-math.c
===
--- clang/test/Sema/builtins-elementwise-math.c
+++ clang/test/Sema/builtins-elementwise-math.c
@@ -135,3 +135,24 @@
   c1 = __builtin_elementwise_min(c1, c2);
   // expected-error@-1 {{1st argument must be a vector, integer or floating point type (was '_Complex float')}}
 }
+
+void test_builtin_elementwise_ceil(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+
+  struct Foo s = __builtin_elementwise_ceil(f);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'float'}}
+
+  i = __builtin_elementwise_ceil();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_ceil(i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  i = __builtin_elementwise_ceil(f, f);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  u = __builtin_elementwise_ceil(u);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_ceil(uv);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
+}
Index: clang/test/CodeGen/builtins-elementwise-math.c
===
--- clang/test/CodeGen/builtins-elementwise-math.c
+++ clang/test/CodeGen/builtins-elementwise-math.c
@@ -189,3 +189,20 @@
   // CHECK-NEXT: call i32 @llvm.smin.i32(i32 [[IAS1]], i32 [[B]])
   int_as_one = __builtin_elementwise_min(int_as_one, b);
 }
+
+void test_builtin_elementwise_ceil(float f1, float f2, double d1, double d2,
+   float4 vf1, float4 vf2, si8 vi1, si8 vi2,
+   long long int i1, long long int i2, short si) {
+  // CHECK-LABEL: define void @test_builtin_elementwise_ceil(
+  // CHECK:  [[F1:%.+]] = load float, float* %f1.addr, align 4
+  // CHECK-NEXT:  call float @llvm.ceil.f32(float [[F1]])
+  f2 = __builtin_elementwise_ceil(f1);
+
+  // CHECK:  [[D1:%.+]] = load double, double* %d1.addr, align 8
+  // CHECK-NEXT: call double @llvm.ceil.f64(double [[D1]])
+  d2 = __builtin_elementwise_ceil(d1);
+
+  // CHECK:  [[VF1:%.+]] = load <4 x float>, <4 x float>* %vf1.addr, align 16
+  // CHECK-NEXT: call <4 x float> @llvm.ceil.v4f32(<4 x float> [[VF1]])
+  vf2 = __builtin_elementwise_ceil(vf1);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2098,10 +2098,47 @@
 break;
   }
 
-  case Builtin::BI__builtin_elementwise_abs:
-if (SemaBuiltinElementwiseMathOneArg(TheCall))
+  // __builtin_elementwise_abs estricts the element type to signed integers or
+  // floating point types only.
+  case Builtin::BI__builtin_elementwise_abs: {
+if (PrepareBuiltinElementwiseMathOneArgCall(TheCall))
   return ExprError();
+
+QualType ArgTy = TheCall->getArg(0)->getType();
+QualType EltTy = ArgTy;
+
+if (auto *VecTy = EltTy->getAs())
+  EltTy = VecTy->getElementType();
+if (EltTy->isUnsignedIntegerType()) {
+  Diag(TheCall->getArg(0)->getBeginLoc(),
+   diag::err_builtin_invalid_arg_type)
+  << 1 << /* signed integer or float ty*/ 3 << ArgTy;
+  return ExprError();
+}
+break;
+  }
+
+  // __builtin_elementwise_abs estricts the element type to floating point
+  // types only.
+  case Builtin::BI__builtin_elementwise_ceil: {
+if (PrepareBuiltinElementwiseMathOneArgCall(TheCall))
+  return ExprError();
+
+QualType ArgTy = TheCall->getArg(0)->getType();
+QualType EltTy = ArgTy;
+
+if (auto *VecTy = EltTy->getAs())
+  EltTy = VecTy->getElementType();
+if (!EltTy->isFloatingType()) {
+

[clang-tools-extra] bab7a30 - [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

2021-12-03 Thread Kirill Bobyrev via cfe-commits

Author: Kirill Bobyrev
Date: 2021-12-03T09:36:50+01:00
New Revision: bab7a30ab692059e5e9dc867a59b9ead47efd35c

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

LOG: [clangd] IncludeCleaner: Do not require forward declarations of 
RecordDecls when definition is available

This makes IncludeCleaner more useful in the presense of a large number of
forward declarations. If the definition is already in the Translation Unit and
visible to the Main File, forward declarations have no effect.

The original patch D112707 was split in two: D114864 and this one.

Reviewed By: kadircet

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

Added: 


Modified: 
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 76d5d626b9f88..0e44c080625af 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -122,16 +122,20 @@ class ReferencedLocationCrawler
   void add(const Decl *D) {
 if (!D || !isNew(D->getCanonicalDecl()))
   return;
-// If we see a declaration in the mainfile, skip all non-definition decls.
-// We only do this for classes because forward declarations are common and
-// we don't want to include every header that forward-declares the symbol
-// because they're often unrelated.
+// Special case RecordDecls, as it is common for them to be forward
+// declared multiple times. The most common cases are:
+// - Definition available in TU, only mark that one as usage. The rest is
+//   likely to be unnecessary. This might result in false positives when an
+//   internal definition is visible.
+// - There's a forward declaration in the main file, no need for other
+//   redecls.
 if (const auto *RD = llvm::dyn_cast(D)) {
-  if (SM.isInMainFile(RD->getMostRecentDecl()->getBeginLoc())) {
-if (const auto *Definition = RD->getMostRecentDecl()->getDefinition())
-  Result.insert(Definition->getLocation());
+  if (const auto *Definition = RD->getDefinition()) {
+Result.insert(Definition->getLocation());
 return;
   }
+  if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation()))
+return;
 }
 for (const Decl *Redecl : D->redecls())
   Result.insert(Redecl->getLocation());

diff  --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp 
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index ddc0a99d58356..1313485f481df 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,10 +39,10 @@ TEST(IncludeCleaner, ReferencedLocations) {
   "class ^X;",
   "X *y;",
   },
-  // FIXME(kirillbobyrev): When definition is available, we don't need to
-  // mark forward declarations as used.
+  // When definition is available, we don't need to mark forward
+  // declarations as used.
   {
-  "class ^X {}; class ^X;",
+  "class ^X {}; class X;",
   "X *y;",
   },
   // We already have forward declaration in the main file, the other
@@ -51,17 +51,24 @@ TEST(IncludeCleaner, ReferencedLocations) {
   "class ^X {}; class X;",
   "class X; X *y;",
   },
+  // Nested class definition can occur outside of the parent class
+  // definition. Bar declaration should be visible to its definition but
+  // it will always be because we will mark Foo definition as used.
+  {
+  "class ^Foo { class Bar; };",
+  "class Foo::Bar {};",
+  },
   // TypedefType and UsingDecls
   {
   "using ^Integer = int;",
   "Integer x;",
   },
   {
-  "namespace ns { struct ^X; struct ^X {}; }",
-  "using ns::X;",
+  "namespace ns { void ^foo(); void ^foo() {} }",
+  "using ns::foo;",
   },
   {
-  "namespace ns { struct X; struct X {}; }",
+  "namespace ns { void foo(); void foo() {}; }",
   "using namespace ns;",
   },
   {
@@ -88,8 +95,8 @@ TEST(IncludeCleaner, ReferencedLocations) {
   },
   // Redecls
   {
-  "class ^X; class ^X{}; class ^X;",
-  "X *y;",
+  "void ^foo(); void ^foo() {} void ^foo();",
+  "void bar() { foo(); }",
   },
   // Constructor
   {



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


[PATCH] D114949: [clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available

2021-12-03 Thread Kirill Bobyrev 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 rGbab7a30ab692: [clangd] IncludeCleaner: Do not require 
forward declarations of RecordDecls… (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114949

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,10 +39,10 @@
   "class ^X;",
   "X *y;",
   },
-  // FIXME(kirillbobyrev): When definition is available, we don't need to
-  // mark forward declarations as used.
+  // When definition is available, we don't need to mark forward
+  // declarations as used.
   {
-  "class ^X {}; class ^X;",
+  "class ^X {}; class X;",
   "X *y;",
   },
   // We already have forward declaration in the main file, the other
@@ -51,17 +51,24 @@
   "class ^X {}; class X;",
   "class X; X *y;",
   },
+  // Nested class definition can occur outside of the parent class
+  // definition. Bar declaration should be visible to its definition but
+  // it will always be because we will mark Foo definition as used.
+  {
+  "class ^Foo { class Bar; };",
+  "class Foo::Bar {};",
+  },
   // TypedefType and UsingDecls
   {
   "using ^Integer = int;",
   "Integer x;",
   },
   {
-  "namespace ns { struct ^X; struct ^X {}; }",
-  "using ns::X;",
+  "namespace ns { void ^foo(); void ^foo() {} }",
+  "using ns::foo;",
   },
   {
-  "namespace ns { struct X; struct X {}; }",
+  "namespace ns { void foo(); void foo() {}; }",
   "using namespace ns;",
   },
   {
@@ -88,8 +95,8 @@
   },
   // Redecls
   {
-  "class ^X; class ^X{}; class ^X;",
-  "X *y;",
+  "void ^foo(); void ^foo() {} void ^foo();",
+  "void bar() { foo(); }",
   },
   // Constructor
   {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -122,16 +122,20 @@
   void add(const Decl *D) {
 if (!D || !isNew(D->getCanonicalDecl()))
   return;
-// If we see a declaration in the mainfile, skip all non-definition decls.
-// We only do this for classes because forward declarations are common and
-// we don't want to include every header that forward-declares the symbol
-// because they're often unrelated.
+// Special case RecordDecls, as it is common for them to be forward
+// declared multiple times. The most common cases are:
+// - Definition available in TU, only mark that one as usage. The rest is
+//   likely to be unnecessary. This might result in false positives when an
+//   internal definition is visible.
+// - There's a forward declaration in the main file, no need for other
+//   redecls.
 if (const auto *RD = llvm::dyn_cast(D)) {
-  if (SM.isInMainFile(RD->getMostRecentDecl()->getBeginLoc())) {
-if (const auto *Definition = RD->getMostRecentDecl()->getDefinition())
-  Result.insert(Definition->getLocation());
+  if (const auto *Definition = RD->getDefinition()) {
+Result.insert(Definition->getLocation());
 return;
   }
+  if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation()))
+return;
 }
 for (const Decl *Redecl : D->redecls())
   Result.insert(Redecl->getLocation());


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,10 +39,10 @@
   "class ^X;",
   "X *y;",
   },
-  // FIXME(kirillbobyrev): When definition is available, we don't need to
-  // mark forward declarations as used.
+  // When definition is available, we don't need to mark forward
+  // declarations as used.
   {
-  "class ^X {}; class ^X;",
+  "class ^X {}; class X;",
   "X *y;",
   },
   // We already have forward declaration in the main file, the other
@@ -51,17 +51,24 @@
   "class ^X {}; class X;",
   "class X; X *y;",
   },
+  // Nested class definition can occur outside of the parent class
+  // definition. Bar declaration should be visible to its 

[PATCH] D102090: [CMake][ELF] Link libLLVM.so and libclang-cpp.so with -Bsymbolic-functions

2021-12-03 Thread Evgeniy via Phabricator via cfe-commits
ebrevnov added a comment.

While -Bsymbolic-funtions brings nice performance improvements it also changes 
symbol resolution order. That means we effectively disabled  preemption for 
functions and all references from inside libLLVM*.so will be resolved locally.  
But references to global data can still be interposed by external definitions. 
Do I understand correctly that main argument against using -Bsymbolic is 
potential issue with equality comparison of address of global? Or anything else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102090

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


[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-03 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray updated this revision to Diff 391600.
lichray added a comment.

Fix failed assertion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

Files:
  clang/lib/AST/APValue.cpp

Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -625,6 +625,97 @@
   return V.convertToDouble();
 }
 
+static bool TryPrintAsStringLiteral(raw_ostream , const ArrayType *ATy,
+const APValue *Data, size_t Size) {
+  if (Size == 0)
+return false;
+
+  QualType Ty = ATy->getElementType();
+  if (!Ty->isAnyCharacterType())
+return false;
+
+  // Nothing we can do about a sequence that is not null-terminated
+  if (!Data[--Size].getInt().isZero())
+return false;
+
+  constexpr size_t MaxN = 36;
+  char Buf[MaxN * 2 + 3] = {'"'}; // "At most 36 escaped chars" + \0
+  auto *pBuf = Buf + 1;
+
+  // Better than printing a two-digit sequence of 10 integers.
+  StringRef Ellipsis;
+  if (Size > MaxN) {
+Ellipsis = " [...]";
+Size = std::min(MaxN - Ellipsis.size(), Size);
+  }
+
+  auto writeEscape = [](char *Ptr, char Ch) {
+Ptr[0] = '\\';
+Ptr[1] = Ch;
+return Ptr + 2;
+  };
+
+  for (auto  : ArrayRef(Data, Size)) {
+auto Char64 = Val.getInt().getExtValue();
+if (Char64 > 0x7f)
+  return false; // Bye bye, see you in integers.
+switch (auto Ch = static_cast(Char64)) {
+default:
+  if (isPrintable(Ch)) {
+*pBuf++ = Ch;
+break;
+  }
+  return false;
+case '\\':
+case '\'': // The diagnostic message is 'quoted'
+  pBuf = writeEscape(pBuf, Ch);
+  break;
+case '\0':
+  pBuf = writeEscape(pBuf, '0');
+  break;
+case '\a':
+  pBuf = writeEscape(pBuf, 'a');
+  break;
+case '\b':
+  pBuf = writeEscape(pBuf, 'b');
+  break;
+case '\f':
+  pBuf = writeEscape(pBuf, 'f');
+  break;
+case '\n':
+  pBuf = writeEscape(pBuf, 'n');
+  break;
+case '\r':
+  pBuf = writeEscape(pBuf, 'r');
+  break;
+case '\t':
+  pBuf = writeEscape(pBuf, 't');
+  break;
+case '\v':
+  pBuf = writeEscape(pBuf, 'v');
+  break;
+}
+  }
+
+  if (!Ellipsis.empty()) {
+memcpy(pBuf, Ellipsis.data(), Ellipsis.size());
+pBuf += Ellipsis.size();
+  }
+  *pBuf++ = '"';
+
+  if (Ty->isWideCharType())
+Out << 'L';
+  else if (Ty->isChar8Type())
+Out << "u8";
+  else if (Ty->isChar16Type())
+Out << 'u';
+  else if (Ty->isChar32Type())
+Out << 'U';
+
+  Out << StringRef(Buf, pBuf - Buf);
+  return true;
+}
+
 void APValue::printPretty(raw_ostream , const ASTContext ,
   QualType Ty) const {
   printPretty(Out, Ctx.getPrintingPolicy(), Ty, );
@@ -795,17 +886,23 @@
   }
   case APValue::Array: {
 const ArrayType *AT = Ty->castAsArrayTypeUnsafe();
+unsigned N = getArrayInitializedElts();
+if (N != 0 &&
+TryPrintAsStringLiteral(Out, AT, (0), N))
+  return;
 QualType ElemTy = AT->getElementType();
 Out << '{';
-if (unsigned N = getArrayInitializedElts()) {
-  getArrayInitializedElt(0).printPretty(Out, Policy, ElemTy, Ctx);
-  for (unsigned I = 1; I != N; ++I) {
+unsigned I = 0;
+switch (N) {
+case 0:
+  for (; I != N; ++I) {
 Out << ", ";
 if (I == 10) {
-  // Avoid printing out the entire contents of large arrays.
-  Out << "...";
-  break;
+  Out << "...}";
+  return;
 }
+LLVM_FALLTHROUGH;
+  default:
 getArrayInitializedElt(I).printPretty(Out, Policy, ElemTy, Ctx);
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   >