[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:409
+
+namespace std {
+

flx wrote:
> gribozavr2 wrote:
> > Could you add a nested inline namespace to better imitate what declarations 
> > look like in libc++?
> I'm not sure I follow.  I looked through the other tests that declare a std 
> function and copied the declaration from modernize-avoid-bind.cpp.
libc++ declarations look like this:

```
namespace std {
inline namespace __1 {
template<...> struct function...
} // __1
} // std
```

The inline namespace in the middle often trips up declaration matching in 
checkers. And yes, many other tests don't imitate this pattern, and are often 
broken with libc++. Those tests should be improved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89332

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

[PATCH] D87830: [clang-tidy][test] Allow empty checks in check_clang_tidy.py

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Probably not quite as verbose but should do the job

  // RUN: clang-tidy %s --checks=-*,my-check-to-test --warnings-as-errors=* -- 
-std=c++11


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87830

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

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

How does this type alias and typedef, In theory that should also be handled.

  using Functor = std::function<...>;
  Functor Copy = Orig; // No warning.




Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:39-44
+AST_MATCHER(NamedDecl, isStdFunction) {
+  // First use the fast getName() method to avoid unnecessary calls to the
+  // slow getQualifiedNameAsString().
+  return Node.getName() == "function" &&
+ Node.getQualifiedNameAsString() == "std::function";
+}

It's better to use `node.isInStdNamespace()` instead of checking the qualified 
name as its designed for that very purpose. Is should probably take a 
`CXXRecordDecl` instead of a `NamedDecl` aswell.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89332

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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D66324#2336186 , @phosek wrote:

> I apologize for the late response, I somehow missed the earlier responses. We 
> have successfully used this feature in Fuchsia and found it useful, but I 
> agree that the issues raised need to be addressed. Unfortunately @paulkirth 
> is no longer working on this project. I hope that someone from our team can 
> take a look but it might take a few weeks. If you prefer, we could revert 
> this change and then reland an improved version in the future?

I would very much prefer *NOT* not revert if someone is going to step up to 
work on these problems soon (within next 4 weeks?).

That being said, in light of that bug, my original doubts about the underlying 
data type (a novel `MD_misexpect`,
with structure different from `MD_prof`) have reappeared with double strength. 
I really think they should share underlying type.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324

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

[PATCH] D89612: NFC: Fix -Wsign-compare warnings on 32-bit builds

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast created this revision.
hubert.reinterpretcast added reviewers: dantrushin, hokein, daltenty, stevewan, 
jasonliu.
Herald added a subscriber: hiraditya.
Herald added projects: clang, LLVM.
hubert.reinterpretcast requested review of this revision.

Comparing 32-bit `ptrdiff_t` against 32-bit `unsigned` results in 
`-Wsign-compare` warnings for both GCC and Clang.

The warning for the cases in question appear to identify an issue where the 
`ptrdiff_t` value would be mutated via conversion to an unsigned type.

The warning is resolved by using the usual arithmetic conversions to safely 
preserve the value of the `unsigned` operand while trying to convert to a 
signed type. Host platforms where `unsigned` has the same width as `unsigned 
long long` will need to make a different change, but using an explicit cast has 
disadvantages that can be avoided for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89612

Files:
  clang/lib/Serialization/ASTReaderStmt.cpp
  llvm/lib/CodeGen/StackMaps.cpp


Index: llvm/lib/CodeGen/StackMaps.cpp
===
--- llvm/lib/CodeGen/StackMaps.cpp
+++ llvm/lib/CodeGen/StackMaps.cpp
@@ -401,7 +401,7 @@
 SmallVector GCPtrIndices;
 unsigned GCPtrIdx = (unsigned)SO.getFirstGCPtrIdx();
 assert((int)GCPtrIdx != -1);
-assert(MOI - MI.operands_begin() == GCPtrIdx);
+assert(MOI - MI.operands_begin() == GCPtrIdx + 0LL);
 while (NumGCPointers--) {
   GCPtrIndices.push_back(GCPtrIdx);
   GCPtrIdx = StackMaps::getNextMetaArgIdx(&MI, GCPtrIdx);
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2186,9 +2186,9 @@
   unsigned NumArgs = Record.readInt();
   E->BeginLoc = readSourceLocation();
   E->EndLoc = readSourceLocation();
-  assert(
-  (NumArgs == std::distance(E->children().begin(), E->children().end())) &&
-  "Wrong NumArgs!");
+  assert((NumArgs + 0LL ==
+  std::distance(E->children().begin(), E->children().end())) &&
+ "Wrong NumArgs!");
   (void)NumArgs;
   for (Stmt *&Child : E->children())
 Child = Record.readSubStmt();


Index: llvm/lib/CodeGen/StackMaps.cpp
===
--- llvm/lib/CodeGen/StackMaps.cpp
+++ llvm/lib/CodeGen/StackMaps.cpp
@@ -401,7 +401,7 @@
 SmallVector GCPtrIndices;
 unsigned GCPtrIdx = (unsigned)SO.getFirstGCPtrIdx();
 assert((int)GCPtrIdx != -1);
-assert(MOI - MI.operands_begin() == GCPtrIdx);
+assert(MOI - MI.operands_begin() == GCPtrIdx + 0LL);
 while (NumGCPointers--) {
   GCPtrIndices.push_back(GCPtrIdx);
   GCPtrIdx = StackMaps::getNextMetaArgIdx(&MI, GCPtrIdx);
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -2186,9 +2186,9 @@
   unsigned NumArgs = Record.readInt();
   E->BeginLoc = readSourceLocation();
   E->EndLoc = readSourceLocation();
-  assert(
-  (NumArgs == std::distance(E->children().begin(), E->children().end())) &&
-  "Wrong NumArgs!");
+  assert((NumArgs + 0LL ==
+  std::distance(E->children().begin(), E->children().end())) &&
+ "Wrong NumArgs!");
   (void)NumArgs;
   for (Stmt *&Child : E->children())
 Child = Record.readSubStmt();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

LLVM buildbot will be restarted soon

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be restarted in the nearest hour.

Thanks

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

[PATCH] D58559: emit '(assertions enabled)' in the version string for a build of clang with assertions enabled

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.

Marking as "Request changes" to get this off my queue; are you still interested 
in pursuing this?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58559

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

[PATCH] D89608: Make sure Objective-C category support in IncludeSorter handles top-level imports

[Joe Turner via Phabricator via cfe-commits]

compositeprimes created this revision.
compositeprimes added reviewers: alexfh, gribozavr.
compositeprimes added a project: clang-tools-extra.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
compositeprimes requested review of this revision.

Currently, this would not correctly associate a category with the related 
include if it was top-level (i.e. no slashes in the path). This ensures that we 
explicitly think about that case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89608

Files:
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp


Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -45,8 +45,12 @@
 
 // Objective-C categories have a `+suffix` format, but should be grouped
 // with the file they are a category of.
+size_t start_index = Canonical.find_last_of('/');
+if (start_index == StringRef::npos) {
+  start_index = 0;
+}
 return Canonical.substr(
-0, Canonical.find_first_of('+', Canonical.find_last_of('/')));
+0, Canonical.find_first_of('+', start_index));
   }
   return RemoveFirstSuffix(
   RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", ".hpp"}),


Index: clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
===
--- clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
+++ clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
@@ -45,8 +45,12 @@
 
 // Objective-C categories have a `+suffix` format, but should be grouped
 // with the file they are a category of.
+size_t start_index = Canonical.find_last_of('/');
+if (start_index == StringRef::npos) {
+  start_index = 0;
+}
 return Canonical.substr(
-0, Canonical.find_first_of('+', Canonical.find_last_of('/')));
+0, Canonical.find_first_of('+', start_index));
   }
   return RemoveFirstSuffix(
   RemoveFirstSuffix(Str, {".cc", ".cpp", ".c", ".h", ".hpp"}),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

We can't have it always match, that would require us to reject the conversion 
to a bare function-pointer type, which is required by the standard.

I'm not sure if MSVC's multiple conversions hack is conformant or not, but it's 
at least more conformant than that.


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

https://reviews.llvm.org/D89559

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

[PATCH] D89372: [OpenCL] Remove unused extensions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D89372#2335627 , @jvesely wrote:

> In D89372#2335027 , @Anastasia wrote:
>
>> In D89372#2334728 , @jvesely wrote:
>>
>>> In D89372#2334225 , @Anastasia 
>>> wrote:
>>>
> 

 Ok, so the pragma is supposed to control the pointer dereferencing which 
 seems like a valid case but however, it is not implemented now. Do we know 
 of any immediate plans from anyone to implement this? If not I would 
 prefer to remove the pragma for now? If someone decided to implement this 
 functionality later fully it is absolutely fine. Pragma will be very easy 
 to add. There is no need for everyone to pay the price for something that 
 can't be used at the moment.
>>>
>>> I though the current behaviour of 'you can use #pragma, but the 
>>> dereferences are always legal' was intentional for backward compatibility.
>>> I don't think there are programs that would set it to 'disabled' and expect 
>>> compilation failure.
>>
>> The initial state of the pragma is disabled, so if disabling the extension 
>> isn't supposed to do anything then I don't know why would anyone ever enable 
>> it?
>>
>>> My concern is that legacy apps would set '#pragma enabled' before using 
>>> char/short. such behavior would produce warning/error if with this change 
>>> applied.
>>
>> Correct, it will compile with a warning but not fail to compile. I am 
>> willing to introduce a -W option (if not present already ) to suppress that 
>> warning if it helps with the clean up and backward compatibility. It might 
>> also open up opportunities for a wider clean up  - removing pragma in 
>> extensions that currently require pragma for no good reason.  I have written 
>> more details on this in 
>> https://github.com/KhronosGroup/OpenCL-Docs/pull/355#issuecomment-662679499
>
> Adding a new option cannot be a solution as legacy applications were written 
> before its introduction.
> CL1.x applications need to continue to work without changes, anything else 
> breaks backward compatibility.

Yes, we don't break backward compatibility of the specified behavior. This is 
never the intent.

> The same arguments also apply to `cles_khr_in64`. It's illegal to use 
> int64 types in embedded profile unless you first enable the extensions. 
> Rather than removing it, `cles_khr_2d_image_array_writes` should be added 
> to the extension list.

 I don't think clang currently support embedded profile. Adding extension 
 into the OpenCLOptions is pointless if it's not used. Do you plan to add 
 any functionality for it? Defining macros can be done easily outside of 
 clang source code or if it is preferable to do inside there is 
 `MacroBuilder::defineMacro`  available in the target hooks. Currently for 
 every extension added into `OpenCLExtensions.def` there is a macro, a 
 pragma and a field in `OpenCLOptions` added. This is often more than what 
 is necessary. Plus Khronos has very many extensions if we assume that all 
 of them are added in clang it will become scalability and maintanance 
 nightmare. Most of the extensions only add functions so if we provide a 
 way to add macros for those outside of clang code base it will keep the 
 common code clean and vendors can be more flexible in adding the 
 extensions without the need to modify upstream code if they need to. I see 
 it as an opportunity to improve common code and out of tree 
 implementations too. It just need a little bit of restructuring.
>>>
>>> My understanding is that both the macro and working #pragma directive is 
>>> necessary.
>>
>> I don't believe this is the correct interpretation. If you look at the 
>> extension spec s9.1 it says:
>>
>> `Every extension which affects the OpenCL language semantics, syntax or adds 
>> built-in functions to the language must create a preprocessor #define that 
>> matches the extension name string.  This #define would be available in the 
>> language if and only if the extension is supported on a given 
>> implementation.`
>
> This is only one part indicating the availability of extension. It's not 
> describing what's necessary to observe extended behaviour. The specs state 
> that the initial state of the compiler is as if `#pragma OPENCL EXTENSION all 
> : disable` was issued.
> This means that functions introduced by extensions cannot be present, and the 
> symbol names are available for application use unless such symbols were 
> previously reserved.

This is not written in  the spec. The statement you quote only says that there 
is implicit `#pragma OPENCL EXTENSION all : disable` and this is all. It says 
nothing about functions!

>> It does not say that the pragma is needed, it only says that macro is 
>> needed. That perfectly makes sense because the

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaLambda.cpp:1278
+  if (CallOpCC == DefaultMember)
+return DefaultFree;
+  return CallOpCC;

rsmith wrote:
> rsmith wrote:
> > erichkeane wrote:
> > > rjmccall wrote:
> > > > ...I made this comment in my first review, but Phabricator threw it 
> > > > away.
> > > > 
> > > > The attributes let you explicitly request the default method CC, right? 
> > > >  I think you need to check for an explicit attribute rather than just 
> > > > checking CC identity.  There should be an AttributedType in the sugar.
> > > They do, but I can't seem to find a way to find them.  The calling 
> > > convention is already merged into the functiontype by the time we get 
> > > here, the AttributedType isn't accessible.
> > > 
> > > So it seems we don't distinguish between "modified by attribute", 
> > > "modified-default by command line", and "modified-default by TargetInfo."
> > > 
> > > That said, I somewhat think this is the right thing to do anyway.  If 
> > > you're on a platform where the default call convention is different 
> > > between a free-function and member-function, I'd think that this is what 
> > > you MEAN...
> > The `AttributedType` should be present in the type on the `TypeSourceInfo` 
> > for the call operator. It might not be present on the type retrieved by 
> > `getType()`, though.
> > 
> > Concretely, what targets have different calling conventions for member 
> > versus non-member functions, and what do those calling conventions do 
> > differently? (For example, if the default member calling convention treats 
> > `this` differently, it doesn't seem reasonable to apply that to the static 
> > invoker...)
> Answering my own question: the only case where the default member calling 
> convention is different from the default non-member calling convention is for 
> MinGW on 32-bit x86, where members use `thiscall` by default (which passes 
> the first parameter in `%ecx`).
> 
> Is it reasonable for `[] [[clang::thiscall]] {}` to result in a static 
> invoker using the `thiscall` calling convention? Intuitively I'd say no, but 
> there's not really anything specific to member functions in `thiscall` -- it 
> just means that we pass the first argument in a register. (However, the first 
> argument of the lambda and the first argument of its static invoker are 
> different, so it's still somewhat unclear whether inheriting `thiscall` is 
> appropriate. But usually for any given lambda only one of the two is actually 
> used.)
> 
> But there's another quirk here: the default non-member calling convention can 
> be set on the command line with `-fdefault-calling-conv=` (and a few other 
> flags such as `-mrtd`). This doesn't affect member functions by default. So 
> what should happen if this is compiled with `-fdefault-calling-conv=stdcall` 
> (assuming the default calling convention is otherwise `cdecl`):
> 
> ```
> auto *p0 = [] [[clang::stdcal]] {};
> auto *p1 = [] {};
> auto *p2 = [] [[clang::cdecl]] {};
> ```
> 
> `p0` seems easy: the default non-member calling convention and the explicit 
> calling convention are the same. The invoker should be `stdcall`.
> 
> For `p1`, the default member calling convention is `cdecl` but the default 
> non-member calling convention is `stdcall`. In this case, conformance 
> requires us to use `stdcall` for the pointer type, because `p1` is required 
> to have type `void (*)()`, which is a `stdcall` function pointer in this 
> configuration.
> 
> For `p2`, the call operator's calling convention has been explicitly set to 
> the default member calling convention. I think in this case I'd expect a 
> `cdecl` static invoker.
> 
> So I think I'm inclined to say we should always apply an explicit calling 
> convention to both the call operator and the static invoker -- that seems 
> like the simplest and easiest to explain rule, and probably matches user 
> expectations most of the time, especially given the observation that you're 
> usually writing a lambda only for one or the other of the call operator and 
> the static invoker, so if you explicitly write a calling convention 
> attribute, you presumably want it to apply to the part of the lambda's 
> interface that you're using.
> Answering my own question: the only case where the default member calling 
> convention is different from the default non-member calling convention is for 
> MinGW on 32-bit x86, where members use `thiscall` by default (which passes 
> the first parameter in `%ecx`).
> 
> Is it reasonable for `[] [[clang::thiscall]] {}` to result in a static 
> invoker using the `thiscall` calling convention? Intuitively I'd say no, but 
> there's not really anything specific to member functions in `thiscall` -- it 
> just means that we pass the first argument in a register. (However, the first 
> argument of the lambda and the first argument of its static invoker are 
> different, so it's still somewhat uncle

[clang] d4aac67 - Make the check for whether we should memset(0) an aggregate

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-10-16T16:48:22-07:00
New Revision: d4aac67859640bdb7e8ed3123a00c3f200f89b9c

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

LOG: Make the check for whether we should memset(0) an aggregate
initialization a little smarter.

Look through casts that preserve zero-ness when determining if an
initializer is zero, so that we can handle cases like an {0} initializer
whose corresponding field is a type other than 'int'.

Added: 


Modified: 
clang/lib/CodeGen/CGExprAgg.cpp
clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 625b9116ff25..bf7c1adb6810 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -1372,11 +1372,109 @@ void 
AggExprEmitter::VisitImplicitValueInitExpr(ImplicitValueInitExpr *E) {
   EmitNullInitializationToLValue(CGF.MakeAddrLValue(Slot.getAddress(), T));
 }
 
+/// Determine whether the given cast kind is known to always convert values
+/// with all zero bits in their value representation to values with all zero
+/// bits in their value representation.
+static bool castPreservesZero(const CastExpr *CE) {
+  switch (CE->getCastKind()) {
+// No-ops.
+  case CK_NoOp:
+  case CK_UserDefinedConversion:
+  case CK_ConstructorConversion:
+  case CK_BitCast:
+  case CK_ToUnion:
+  case CK_ToVoid:
+// Conversions between (possibly-complex) integral, (possibly-complex)
+// floating-point, and bool.
+  case CK_BooleanToSignedIntegral:
+  case CK_FloatingCast:
+  case CK_FloatingComplexCast:
+  case CK_FloatingComplexToBoolean:
+  case CK_FloatingComplexToIntegralComplex:
+  case CK_FloatingComplexToReal:
+  case CK_FloatingRealToComplex:
+  case CK_FloatingToBoolean:
+  case CK_FloatingToIntegral:
+  case CK_IntegralCast:
+  case CK_IntegralComplexCast:
+  case CK_IntegralComplexToBoolean:
+  case CK_IntegralComplexToFloatingComplex:
+  case CK_IntegralComplexToReal:
+  case CK_IntegralRealToComplex:
+  case CK_IntegralToBoolean:
+  case CK_IntegralToFloating:
+// Reinterpreting integers as pointers and vice versa.
+  case CK_IntegralToPointer:
+  case CK_PointerToIntegral:
+// Language extensions.
+  case CK_VectorSplat:
+  case CK_NonAtomicToAtomic:
+  case CK_AtomicToNonAtomic:
+return true;
+
+  case CK_BaseToDerivedMemberPointer:
+  case CK_DerivedToBaseMemberPointer:
+  case CK_MemberPointerToBoolean:
+  case CK_NullToMemberPointer:
+  case CK_ReinterpretMemberPointer:
+// FIXME: ABI-dependent.
+return false;
+
+  case CK_AnyPointerToBlockPointerCast:
+  case CK_BlockPointerToObjCPointerCast:
+  case CK_CPointerToObjCPointerCast:
+  case CK_ObjCObjectLValueCast:
+  case CK_IntToOCLSampler:
+  case CK_ZeroToOCLOpaqueType:
+// FIXME: Check these.
+return false;
+
+  case CK_FixedPointCast:
+  case CK_FixedPointToBoolean:
+  case CK_FixedPointToFloating:
+  case CK_FixedPointToIntegral:
+  case CK_FloatingToFixedPoint:
+  case CK_IntegralToFixedPoint:
+// FIXME: Do all fixed-point types represent zero as all 0 bits?
+return false;
+
+  case CK_AddressSpaceConversion:
+  case CK_BaseToDerived:
+  case CK_DerivedToBase:
+  case CK_Dynamic:
+  case CK_NullToPointer:
+  case CK_PointerToBoolean:
+// FIXME: Preserves zeroes only if zero pointers and null pointers have the
+// same representation in all involved address spaces.
+return false;
+
+  case CK_ARCConsumeObject:
+  case CK_ARCExtendBlockObject:
+  case CK_ARCProduceObject:
+  case CK_ARCReclaimReturnedObject:
+  case CK_CopyAndAutoreleaseBlockObject:
+  case CK_ArrayToPointerDecay:
+  case CK_FunctionToPointerDecay:
+  case CK_BuiltinFnToFnPtr:
+  case CK_Dependent:
+  case CK_LValueBitCast:
+  case CK_LValueToRValue:
+  case CK_LValueToRValueBitCast:
+  case CK_UncheckedDerivedToBase:
+return false;
+  }
+}
+
 /// isSimpleZero - If emitting this value will obviously just cause a store of
 /// zero to memory, return true.  This can return false if uncertain, so it 
just
 /// handles simple cases.
 static bool isSimpleZero(const Expr *E, CodeGenFunction &CGF) {
   E = E->IgnoreParens();
+  while (auto *CE = dyn_cast(E)) {
+if (!castPreservesZero(CE))
+  break;
+E = CE->getSubExpr()->IgnoreParens();
+  }
 
   // 0
   if (const IntegerLiteral *IL = dyn_cast(E))

diff  --git a/clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp 
b/clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
index 3e9d936a16f8..fb7e82d929d6 100644
--- a/clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ b/clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -139,4 +139,35 @@ namespace ZeroInit {
   // CHECK-NOT }
   // CHECK: call {{.*}}memset
   Largeish largeish4() { return (Largeish){}

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

[Alexandre Ganea via Phabricator via cfe-commits]

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



Comment at: lld/Common/ErrorHandler.cpp:78
   }
-  _exit(val);
+  llvm::sys::Process::Exit(val);
 }

rnk wrote:
> This appears to have regressed shutdown. `sys::Process::Exit` calls `exit`, 
> not `_exit`, so now atexit destructors are run. That's unintended, right?
Yes, the fix is in D88348!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

I apologize for the late response, I somehow missed the earlier responses. We 
have successfully used this feature in Fuchsia and found it useful, but I agree 
that the issues raised need to be addressed. Unfortunately @paulkirth is no 
longer working on this project. I hope that someone from our team can take a 
look but it might take a few weeks. If you prefer, we could revert this change 
and then reland an improved version in the future?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324

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

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

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

rsmith added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:2445
+   FPO.getFPExceptionMode() != LangOptions::FPE_Ignore ||
+   FPO.getAllowFEnvAccess()) && Info.Ctx.getLangOpts().CPlusPlus) {
+Info.FFDiag(E, diag::note_constexpr_float_arithmetic_strict);

I don't think we need the `CPlusPlus` check here; we bail out of this function 
in constant contexts in all language modes, and in non-constant contexts we 
want to abort evaluation under this condition in C too.



Comment at: clang/lib/AST/ExprConstant.cpp:2496-2498
+  if (llvm::APFloatBase::opOK != Result.convertFromAPInt(Value,
+   Value.isSigned(),
+   APFloat::rmNearestTiesToEven) &&

Following D89360, we should skip this check if `Info.InConstantContext`.



Comment at: clang/lib/AST/ExprConstant.cpp:12302
+llvm::APFloatBase::cmpResult CmpResult = LHS.compare(RHS);
+if (CmpResult == APFloat::cmpUnordered &&
+E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {

Skip this check if `Info.InConstantContext`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87528

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

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: lld/Common/ErrorHandler.cpp:78
   }
-  _exit(val);
+  llvm::sys::Process::Exit(val);
 }

This appears to have regressed shutdown. `sys::Process::Exit` calls `exit`, not 
`_exit`, so now atexit destructors are run. That's unintended, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

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

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

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

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaLambda.cpp:1278
+  if (CallOpCC == DefaultMember)
+return DefaultFree;
+  return CallOpCC;

rsmith wrote:
> erichkeane wrote:
> > rjmccall wrote:
> > > ...I made this comment in my first review, but Phabricator threw it away.
> > > 
> > > The attributes let you explicitly request the default method CC, right?  
> > > I think you need to check for an explicit attribute rather than just 
> > > checking CC identity.  There should be an AttributedType in the sugar.
> > They do, but I can't seem to find a way to find them.  The calling 
> > convention is already merged into the functiontype by the time we get here, 
> > the AttributedType isn't accessible.
> > 
> > So it seems we don't distinguish between "modified by attribute", 
> > "modified-default by command line", and "modified-default by TargetInfo."
> > 
> > That said, I somewhat think this is the right thing to do anyway.  If 
> > you're on a platform where the default call convention is different between 
> > a free-function and member-function, I'd think that this is what you MEAN...
> The `AttributedType` should be present in the type on the `TypeSourceInfo` 
> for the call operator. It might not be present on the type retrieved by 
> `getType()`, though.
> 
> Concretely, what targets have different calling conventions for member versus 
> non-member functions, and what do those calling conventions do differently? 
> (For example, if the default member calling convention treats `this` 
> differently, it doesn't seem reasonable to apply that to the static 
> invoker...)
Answering my own question: the only case where the default member calling 
convention is different from the default non-member calling convention is for 
MinGW on 32-bit x86, where members use `thiscall` by default (which passes the 
first parameter in `%ecx`).

Is it reasonable for `[] [[clang::thiscall]] {}` to result in a static invoker 
using the `thiscall` calling convention? Intuitively I'd say no, but there's 
not really anything specific to member functions in `thiscall` -- it just means 
that we pass the first argument in a register. (However, the first argument of 
the lambda and the first argument of its static invoker are different, so it's 
still somewhat unclear whether inheriting `thiscall` is appropriate. But 
usually for any given lambda only one of the two is actually used.)

But there's another quirk here: the default non-member calling convention can 
be set on the command line with `-fdefault-calling-conv=` (and a few other 
flags such as `-mrtd`). This doesn't affect member functions by default. So 
what should happen if this is compiled with `-fdefault-calling-conv=stdcall` 
(assuming the default calling convention is otherwise `cdecl`):

```
auto *p0 = [] [[clang::stdcal]] {};
auto *p1 = [] {};
auto *p2 = [] [[clang::cdecl]] {};
```

`p0` seems easy: the default non-member calling convention and the explicit 
calling convention are the same. The invoker should be `stdcall`.

For `p1`, the default member calling convention is `cdecl` but the default 
non-member calling convention is `stdcall`. In this case, conformance requires 
us to use `stdcall` for the pointer type, because `p1` is required to have type 
`void (*)()`, which is a `stdcall` function pointer in this configuration.

For `p2`, the call operator's calling convention has been explicitly set to the 
default member calling convention. I think in this case I'd expect a `cdecl` 
static invoker.

So I think I'm inclined to say we should always apply an explicit calling 
convention to both the call operator and the static invoker -- that seems like 
the simplest and easiest to explain rule, and probably matches user 
expectations most of the time, especially given the observation that you're 
usually writing a lambda only for one or the other of the call operator and the 
static invoker, so if you explicitly write a calling convention attribute, you 
presumably want it to apply to the part of the lambda's interface that you're 
using.


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

https://reviews.llvm.org/D89559

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

[PATCH] D86819: [PowerPC][Power10] Implementation of 128-bit Binary Vector Rotate builtins

[Albion Fung via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd30155feaa9c: [PowerPC] Implementation of 128-bit Binary 
Vector Rotate builtins (authored by Conanap).

Changed prior to commit:
  https://reviews.llvm.org/D86819?vs=295393&id=298770#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86819

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vector-rotate.ll

Index: llvm/test/CodeGen/PowerPC/p10-vector-rotate.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/p10-vector-rotate.ll
@@ -0,0 +1,85 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
+; RUN:   FileCheck %s -check-prefixes=CHECK-LE,CHECK
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
+; RUN:   FileCheck %s -check-prefixes=CHECK-BE,CHECK
+
+; This test case aims to test the builtins for vector rotate instructions
+; on Power10.
+
+
+define <1 x i128> @test_vrlq(<1 x i128> %x, <1 x i128> %y) {
+; CHECK-LABEL: test_vrlq:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vrlq v2, v3, v2
+; CHECK-NEXT:blr
+  %shl.i = shl <1 x i128> %y, %x
+  %sub.i = sub <1 x i128> , %x
+  %lshr.i = lshr <1 x i128> %y, %sub.i
+  %tmp = or <1 x i128> %shl.i, %lshr.i
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vrlq_cost_mult8(<1 x i128> %x) {
+; CHECK-LABEL: test_vrlq_cost_mult8:
+; CHECK: # %bb.0:
+; CHECK: vrlq v2, v3, v2
+; CHECK-NEXT: blr
+  %shl.i = shl <1 x i128> , %x
+  %sub.i = sub <1 x i128> , %x
+  %lshr.i = lshr <1 x i128> , %sub.i
+  %tmp = or <1 x i128> %shl.i, %lshr.i
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vrlq_cost_non_mult8(<1 x i128> %x) {
+; CHECK-LABEL: test_vrlq_cost_non_mult8:
+; CHECK: # %bb.0:
+; CHECK: vrlq v2, v3, v2
+; CHECK-NEXT: blr
+  %shl.i = shl <1 x i128> , %x
+  %sub.i = sub <1 x i128> , %x
+  %lshr.i = lshr <1 x i128> , %sub.i
+  %tmp = or <1 x i128> %shl.i, %lshr.i
+  ret <1 x i128> %tmp
+}
+
+; Function Attrs: nounwind readnone
+define <1 x i128> @test_vrlqmi(<1 x i128> %a, <1 x i128> %b, <1 x i128> %c) {
+; CHECK-LABEL: test_vrlqmi:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:vrlqmi v3, v2, v4
+; CHECK-NEXT:vmr v2, v3
+; CHECK-NEXT:blr
+entry:
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vrlqmi(<1 x i128> %a, <1 x i128> %c, <1 x i128> %b)
+  ret <1 x i128> %tmp
+}
+
+; Function Attrs: nounwind readnone
+define <1 x i128> @test_vrlqnm(<1 x i128> %a, <1 x i128> %b, <1 x i128> %c) {
+; CHECK-LABEL: test_vrlqnm:
+; CHECK:# %bb.0: # %entry
+; CHECK-BE: lxvx v5
+; CHECK-BE-NEXT:vperm v3, v3, v4, v5
+; CHECK-LE-NEXT:plxv v5
+; CHECK-LE-NEXT:vperm v3, v4, v3, v5
+; CHECK-NEXT:   vrlqnm v2, v2, v3
+; CHECK-NEXT:   blr
+entry:
+  %0 = bitcast <1 x i128> %b to <16 x i8>
+  %1 = bitcast <1 x i128> %c to <16 x i8>
+  %shuffle.i = shufflevector <16 x i8> %0, <16 x i8> %1, <16 x i32> 
+  %d = bitcast <16 x i8> %shuffle.i to <1 x i128>
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vrlqnm(<1 x i128> %a, <1 x i128> %d)
+  ret <1 x i128> %tmp
+}
+
+; Function Attrs: nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vrlqmi(<1 x i128>, <1 x i128>, <1 x i128>)
+
+; Function Attrs: nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vrlqnm(<1 x i128>, <1 x i128>)
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -2111,10 +2111,16 @@
"vcmpuq $BF, $vA, $vB", IIC_VecGeneral, []>;
   def VCMPSQ : VXForm_BF3_VAB5<321, (outs crrc:$BF), (ins vrrc:$vA, vrrc:$vB),
"vcmpsq $BF, $vA, $vB", IIC_VecGeneral, []>;
-  def VRLQNM : VX1_VT5_VA5_VB5<325, "vrlqnm", []>;
+  def VRLQNM : VX1_VT5_VA5_VB5<325, "vrlqnm",
+   [(set v1i128:$vD,
+   (int_ppc_altivec_vrlqnm v1i128:$vA,
+   v1i128:$vB))]>;
   def VRLQMI : VXForm_1<69, (outs vrrc:$vD),
 (ins vrrc:$vA, vrrc:$vB, vrrc:$vDi),
-"vrlqmi $vD, $vA, $vB", IIC_VecFP, []>,
+"vrlqmi $vD, $vA, $vB", IIC_VecFP,
+[(set v1i128:$vD,
+  (int_ppc_altivec_vrlqmi v1i128:$vA, v1i128:$vB,
+ 

[clang] d30155f - [PowerPC] Implementation of 128-bit Binary Vector Rotate builtins

[Albion Fung via cfe-commits]

Author: Albion Fung
Date: 2020-10-16T18:03:22-04:00
New Revision: d30155feaa9c4ddd09cb115fb30ea5810f63af9c

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

LOG: [PowerPC] Implementation of 128-bit Binary Vector Rotate builtins

This patch implements 128-bit Binary Vector Rotate builtins for PowerPC10.

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

Added: 
llvm/test/CodeGen/PowerPC/p10-vector-rotate.ll

Modified: 
clang/include/clang/Basic/BuiltinsPPC.def
clang/lib/Headers/altivec.h
clang/test/CodeGen/builtins-ppc-p10vector.c
llvm/include/llvm/IR/IntrinsicsPowerPC.td
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
llvm/lib/Target/PowerPC/PPCInstrPrefix.td

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsPPC.def 
b/clang/include/clang/Basic/BuiltinsPPC.def
index 015411abc508..fe20c9418e8d 100644
--- a/clang/include/clang/Basic/BuiltinsPPC.def
+++ b/clang/include/clang/Basic/BuiltinsPPC.def
@@ -422,6 +422,10 @@ BUILTIN(__builtin_altivec_vextduwvrx, "V2ULLiV4UiV4UiUi", 
"")
 BUILTIN(__builtin_altivec_vextddvlx, "V2ULLiV2ULLiV2ULLiUi", "")
 BUILTIN(__builtin_altivec_vextddvrx, "V2ULLiV2ULLiV2ULLiUi", "")
 
+// P10 Vector rotate built-ins.
+BUILTIN(__builtin_altivec_vrlqmi, "V1ULLLiV1ULLLiV1ULLLiV1ULLLi", "")
+BUILTIN(__builtin_altivec_vrlqnm, "V1ULLLiV1ULLLiV1ULLLi", "")
+
 // VSX built-ins.
 
 BUILTIN(__builtin_vsx_lxvd2x, "V2divC*", "")

diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index 1d7bc201d330..2df420d640f1 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -7927,6 +7927,18 @@ vec_rl(vector unsigned long long __a, vector unsigned 
long long __b) {
 }
 #endif
 
+#ifdef __POWER10_VECTOR__
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_rl(vector signed __int128 __a, vector unsigned __int128 __b) {
+  return (__b << __a)|(__b >> ((__CHAR_BIT__ * sizeof(vector signed __int128)) 
- __a));
+}
+
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_rl(vector unsigned __int128 __a, vector unsigned __int128 __b) {
+  return (__b << __a)|(__b >> ((__CHAR_BIT__ * sizeof(vector unsigned 
__int128)) - __a));
+}
+#endif
+
 /* vec_rlmi */
 #ifdef __POWER9_VECTOR__
 static __inline__ vector unsigned int __ATTRS_o_ai
@@ -7940,8 +7952,24 @@ vec_rlmi(vector unsigned long long __a, vector unsigned 
long long __b,
  vector unsigned long long __c) {
   return __builtin_altivec_vrldmi(__a, __c, __b);
 }
+#endif
+
+#ifdef __POWER10_VECTOR__
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_rlmi(vector unsigned __int128 __a, vector unsigned __int128 __b,
+ vector unsigned __int128 __c) {
+  return __builtin_altivec_vrlqmi(__a, __c, __b);
+}
+
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_rlmi(vector signed __int128 __a, vector signed __int128 __b,
+ vector signed __int128 __c) {
+  return __builtin_altivec_vrlqmi(__a, __c, __b);
+}
+#endif
 
 /* vec_rlnm */
+#ifdef __POWER9_VECTOR__
 static __inline__ vector unsigned int __ATTRS_o_ai
 vec_rlnm(vector unsigned int __a, vector unsigned int __b,
  vector unsigned int __c) {
@@ -7957,6 +7985,42 @@ vec_rlnm(vector unsigned long long __a, vector unsigned 
long long __b,
 }
 #endif
 
+#ifdef __POWER10_VECTOR__
+static __inline__ vector unsigned __int128 __ATTRS_o_ai
+vec_rlnm(vector unsigned __int128 __a, vector unsigned __int128 __b,
+ vector unsigned __int128 __c) {
+  // Merge __b and __c using an appropriate shuffle.
+  vector unsigned char TmpB = (vector unsigned char)__b;
+  vector unsigned char TmpC = (vector unsigned char)__c;
+  vector unsigned char MaskAndShift =
+#ifdef __LITTLE_ENDIAN__
+  __builtin_shufflevector(TmpB, TmpC, -1, -1, -1, -1, -1, -1, -1, -1, 16, 
0,
+  1, -1, -1, -1, -1, -1);
+#else
+  __builtin_shufflevector(TmpB, TmpC, -1, -1, -1, -1, -1, 31, 30, 15, -1,
+  -1, -1, -1, -1, -1, -1, -1);
+#endif
+   return __builtin_altivec_vrlqnm(__a, (vector unsigned __int128) 
MaskAndShift);
+}
+
+static __inline__ vector signed __int128 __ATTRS_o_ai
+vec_rlnm(vector signed __int128 __a, vector signed __int128 __b,
+ vector signed __int128 __c) {
+  // Merge __b and __c using an appropriate shuffle.
+  vector unsigned char TmpB = (vector unsigned char)__b;
+  vector unsigned char TmpC = (vector unsigned char)__c;
+  vector unsigned char MaskAndShift =
+#ifdef __LITTLE_ENDIAN__
+  __builtin_shufflevector(TmpB, TmpC, -1, -1, -1, -1, -1, -1, -1, -1, 16, 
0,
+  1, -1, -1, -1, -1, -1);
+#else
+  __builtin_shufflevector(TmpB, TmpC, -1, -1, -1, -1, -1, 31, 30, 15, -1,
+  -1, -1, -1, -1, -1, -1, -1);
+#endif
+  return __builtin_altivec_vrl

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

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

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaLambda.cpp:1278
+  if (CallOpCC == DefaultMember)
+return DefaultFree;
+  return CallOpCC;

erichkeane wrote:
> rjmccall wrote:
> > ...I made this comment in my first review, but Phabricator threw it away.
> > 
> > The attributes let you explicitly request the default method CC, right?  I 
> > think you need to check for an explicit attribute rather than just checking 
> > CC identity.  There should be an AttributedType in the sugar.
> They do, but I can't seem to find a way to find them.  The calling convention 
> is already merged into the functiontype by the time we get here, the 
> AttributedType isn't accessible.
> 
> So it seems we don't distinguish between "modified by attribute", 
> "modified-default by command line", and "modified-default by TargetInfo."
> 
> That said, I somewhat think this is the right thing to do anyway.  If you're 
> on a platform where the default call convention is different between a 
> free-function and member-function, I'd think that this is what you MEAN...
The `AttributedType` should be present in the type on the `TypeSourceInfo` for 
the call operator. It might not be present on the type retrieved by 
`getType()`, though.

Concretely, what targets have different calling conventions for member versus 
non-member functions, and what do those calling conventions do differently? 
(For example, if the default member calling convention treats `this` 
differently, it doesn't seem reasonable to apply that to the static invoker...)


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

https://reviews.llvm.org/D89559

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

[PATCH] D89520: Don't permit array bound constant folding in OpenCL.

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

rsmith updated this revision to Diff 298764.
rsmith added a comment.

Rebase on D89523 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89520

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
  clang/test/SemaOpenCL/address-spaces.cl

Index: clang/test/SemaOpenCL/address-spaces.cl
===
--- clang/test/SemaOpenCL/address-spaces.cl
+++ clang/test/SemaOpenCL/address-spaces.cl
@@ -4,6 +4,13 @@
 
 __constant int ci = 1;
 
+// __constant ints are allowed in constant expressions.
+enum use_ci_in_enum { enumerator = ci };
+typedef int use_ci_in_array_bound[ci];
+
+// general constant folding of array bounds is not permitted
+typedef int folding_in_array_bounds[&ci + 3 - &ci]; // expected-error-re variable length arrays are not supported in OpenCL|array size is not a constant expression expected-note {{cannot refer to element 3}}
+
 __kernel void foo(__global int *gip) {
   __local int li;
   __local int lj = 2; // expected-error {{'__local' variable cannot have an initializer}}
Index: clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
+++ clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -104,7 +104,7 @@
 // NOOPT: @test_static_var_private.sp2 = internal addrspace(1) global i8 addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), align 4
 // NOOPT: @test_static_var_private.sp3 = internal addrspace(1) global i8 addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), align 4
 // NOOPT: @test_static_var_private.sp4 = internal addrspace(1) global i8 addrspace(5)* null, align 4
-// NOOPT: @test_static_var_private.sp5 = internal addrspace(1) global i8 addrspace(5)* null, align 4
+// NOOPT: @test_static_var_private.sp5 = internal addrspace(1) global i8 addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), align 4
 // NOOPT: @test_static_var_private.SS1 = internal addrspace(1) global %struct.StructTy1 { i8 addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*), i8 addrspace(4)* null, i8 addrspace(1)* null, i8* null }, align 8
 // NOOPT: @test_static_var_private.SS2 = internal addrspace(1) global %struct.StructTy2 zeroinitializer, align 8
 
@@ -123,7 +123,7 @@
 // NOOPT: @test_static_var_local.sp2 = internal addrspace(1) global i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*), align 4
 // NOOPT: @test_static_var_local.sp3 = internal addrspace(1) global i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*), align 4
 // NOOPT: @test_static_var_local.sp4 = internal addrspace(1) global i8 addrspace(3)* null, align 4
-// NOOPT: @test_static_var_local.sp5 = internal addrspace(1) global i8 addrspace(3)* null, align 4
+// NOOPT: @test_static_var_local.sp5 = internal addrspace(1) global i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*), align 4
 // NOOPT: @test_static_var_local.SS1 = internal addrspace(1) global %struct.StructTy1 { i8 addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*), i8 addrspace(4)* null, i8 addrspace(1)* null, i8* null }, align 8
 // NOOPT: @test_static_var_local.SS2 = internal addrspace(1) global %struct.StructTy2 zeroinitializer, align 8
 void test_static_var_local(void) {
@@ -142,7 +142,7 @@
 // NOOPT: store i8 addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), i8 addrspace(5)* addrspace(5)* %sp1, align 4
 // NOOPT: store i8 addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), i8 addrspace(5)* addrspace(5)* %sp2, align 4
 // NOOPT: store i8 addrspace(5)* null, i8 addrspace(5)* addrspace(5)* %sp3, align 4
-// NOOPT: store i8 addrspace(5)* null, i8 addrspace(5)* addrspace(5)* %sp4, align 4
+// NOOPT: store i8 addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*), i8 addrspace(5)* addrspace(5)* %sp4, align 4
 // NOOPT: %[[SS1:.*]] = bitcast %struct.StructTy1 addrspace(5)* %SS1 to i8 addrspace(5)*
 // NOOPT: call void @llvm.memcpy.p5i8.p4i8.i64(i8 addrspace(5)* align 8 %[[SS1]], i8 addrspace(4)* align 8 bitcast (%struct.StructTy1 addrspace(4)* @__const.test_func_scope_var_private.SS1 to i8 addrspace(4)*), i64 32, i1 false)
 // NOOPT: %[[SS2:.*]] = bitcast %struct.StructTy2 addrspace(5)* %SS2 to i8 addrspace(5)*
@@ -162,7 +162,7 @@
 // NOOPT: store i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*), i8 addrspace(3)* addrspace(5)* %sp1, align 4
 // NOOPT: store i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*), i8 addrspace(3)* addrspace(5)* %sp2, align 4
 // NOOPT: store i8 addrspace(3)* null, i8 addrspace(3)* addrspace(5)* %sp3, align 4
-// NOOPT: store i8 addrspace(3)* null, i8 addrspace(3)* addrspace(5)* %sp4, align 4
+// NOOPT: sto

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

[Richard Smith - zygoloid 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 rG552c6c232872: PR44406: Follow behavior of array bound 
constant folding in more recent… (authored by rsmith).

Changed prior to commit:
  https://reviews.llvm.org/D89523?vs=298705&id=298761#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89523

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/basic/basic.types/p10.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CodeGen/vla.c
  clang/test/Misc/warning-flags.c
  clang/test/PCH/cxx-constexpr.cpp
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  clang/test/Sema/builtin-assume.c
  clang/test/Sema/builtins.c
  clang/test/Sema/complex-int.c
  clang/test/Sema/const-eval-64.c
  clang/test/Sema/const-eval.c
  clang/test/Sema/darwin-align-cast.c
  clang/test/Sema/decl-in-prototype.c
  clang/test/Sema/gnu-flags.c
  clang/test/Sema/i-c-e.c
  clang/test/Sema/offsetof-64.c
  clang/test/Sema/rounding-math.c
  clang/test/Sema/struct-decl.c
  clang/test/Sema/typedef-variable-type.c
  clang/test/Sema/vla.c
  clang/test/SemaCXX/anonymous-struct.cpp
  clang/test/SemaCXX/constant-expression.cpp
  clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
  clang/test/SemaCXX/i-c-e-cxx.cpp
  clang/test/SemaObjC/gcc-cast-ext.m

Index: clang/test/SemaObjC/gcc-cast-ext.m
===
--- clang/test/SemaObjC/gcc-cast-ext.m
+++ clang/test/SemaObjC/gcc-cast-ext.m
@@ -11,7 +11,7 @@
 
 // GCC allows pointer expressions in integer constant expressions.
 struct {
-  char control[((int)(char *)2)];
+  char control[((int)(char *)2)]; // expected-warning {{extension}}
 } xx;
 
 @implementation PBXDocBookmark // expected-warning {{method definition for 'autorelease' not found}}\
Index: clang/test/SemaCXX/i-c-e-cxx.cpp
===
--- clang/test/SemaCXX/i-c-e-cxx.cpp
+++ clang/test/SemaCXX/i-c-e-cxx.cpp
@@ -80,7 +80,7 @@
 #endif
 
 int PR8836test[(__typeof(sizeof(int)))&reinterpret_castPR8836*)0)->a))];
-// expected-warning@-1 {{folded to constant array as an extension}}
+// expected-warning@-1 0-1{{C99 feature}} expected-warning@-1 {{folded to constant array as an extension}}
 // expected-note@-2 {{cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression}}
 
 const int nonconst = 1.0;
@@ -89,7 +89,7 @@
 #endif
 int arr[nonconst];
 #if __cplusplus <= 199711L
-// expected-warning@-2 {{folded to constant array as an extension}}
+// expected-warning@-2 0-1{{C99 feature}} expected-warning@-2 {{folded to constant array as an extension}}
 // expected-note@-3 {{initializer of 'nonconst' is not a constant expression}}
 #endif
 
Index: clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
===
--- clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
+++ clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
@@ -120,7 +120,7 @@
   extern "C" int strncmp(const char *, const char *, decltype(sizeof(0))) noexcept;
 
   // Check we recognized both as builtins.
-  typedef int arr[strcmp("bar", "foo") + 4 * strncmp("foo", "bar", 4)];
+  typedef int arr[strcmp("bar", "foo") + 4 * strncmp("foo", "bar", 4)]; // expected-warning {{variable length array}}
   typedef int arr[3];
 }
 
Index: clang/test/SemaCXX/constant-expression.cpp
===
--- clang/test/SemaCXX/constant-expression.cpp
+++ clang/test/SemaCXX/constant-expression.cpp
@@ -110,7 +110,7 @@
 const int recurse2 = recurse1; // expected-note {{here}}
 const int recurse1 = 1;
 int array1[recurse1]; // ok
-int array2[recurse2]; // expected-warning {{variable length array}} expected-warning {{integer constant expression}} expected-note {{initializer of 'recurse2' is not a constant expression}}
+int array2[recurse2]; // expected-warning 2{{variable length array}} expected-note {{initializer of 'recurse2' is not a constant expression}}
 
 namespace FloatConvert {
   typedef int a[(int)42.3];
Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -131,6 +131,9 @@
   typedef struct { // expected-error {{unsupported}}
 enum X {};
 int arr[&f ? 1 : 2];
+#if __cplusplus < 201103L
+// expected-warning@-2 {{folded to constant}}
+#endif
   } C; // expected-note {{by this typedef}}
 }
 
Index: clang/test/Sema/vla.c
==

[clang] 552c6c2 - PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-10-16T14:34:35-07:00
New Revision: 552c6c2328723a248c2b4d2765f75d49129dff20

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

LOG: PR44406: Follow behavior of array bound constant folding in more recent 
versions of GCC.

Old GCC used to aggressively fold VLAs to constant-bound arrays at block
scope in GNU mode. That's non-conforming, and more modern versions of
GCC only do this at file scope. Update Clang to do the same.

Also promote the warning for this from off-by-default to on-by-default
in all cases; more recent versions of GCC likewise warn on this by
default.

This is still slightly more permissive than GCC, as pointed out in
PR44406, as we still fold VLAs to constant arrays in structs, but that
seems justifiable given that we don't support VLA-in-struct (and don't
intend to ever support it), but GCC does.

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

Added: 


Modified: 
clang/docs/UsersManual.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaType.cpp
clang/test/CXX/basic/basic.types/p10.cpp
clang/test/CXX/drs/dr3xx.cpp
clang/test/CodeGen/vla.c
clang/test/Misc/warning-flags.c
clang/test/PCH/cxx-constexpr.cpp
clang/test/Profile/misexpect-switch-default.c
clang/test/Profile/misexpect-switch-nonconst.c
clang/test/Profile/misexpect-switch-only-default-case.c
clang/test/Profile/misexpect-switch.c
clang/test/Sema/builtin-assume.c
clang/test/Sema/builtins.c
clang/test/Sema/complex-int.c
clang/test/Sema/const-eval-64.c
clang/test/Sema/const-eval.c
clang/test/Sema/darwin-align-cast.c
clang/test/Sema/decl-in-prototype.c
clang/test/Sema/gnu-flags.c
clang/test/Sema/i-c-e.c
clang/test/Sema/offsetof-64.c
clang/test/Sema/rounding-math.c
clang/test/Sema/struct-decl.c
clang/test/Sema/typedef-variable-type.c
clang/test/Sema/vla.c
clang/test/SemaCXX/anonymous-struct.cpp
clang/test/SemaCXX/constant-expression.cpp
clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
clang/test/SemaCXX/i-c-e-cxx.cpp
clang/test/SemaObjC/gcc-cast-ext.m

Removed: 




diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 473fbb6d8d04..9726a25f7f63 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -2502,10 +2502,6 @@ Differences between all ``c*`` and ``gnu*`` modes:
 -  The Apple "blocks" extension is recognized by default in ``gnu*`` modes
on some platforms; it can be enabled in any mode with the ``-fblocks``
option.
--  Arrays that are VLA's according to the standard, but which can be
-   constant folded by the frontend are treated as fixed size arrays.
-   This occurs for things like "int X[(1, 2)];", which is technically a
-   VLA. ``c*`` modes are strictly compliant and treat these as VLAs.
 
 Differences between ``*89`` and ``*94`` modes:
 
@@ -2594,10 +2590,12 @@ Intentionally unsupported GCC extensions
the extension appears to be rarely used. Note that clang *does*
support flexible array members (arrays with a zero or unspecified
size at the end of a structure).
--  clang does not have an equivalent to gcc's "fold"; this means that
-   clang doesn't accept some constructs gcc might accept in contexts
-   where a constant expression is required, like "x-x" where x is a
-   variable.
+-  GCC accepts many expression forms that are not valid integer constant
+   expressions in bit-field widths, enumerator constants, case labels,
+   and in array bounds at global scope. Clang also accepts additional
+   expression forms in these contexts, but constructs that GCC accepts due to
+   simplifications GCC performs while parsing, such as ``x - x`` (where ``x`` 
is a
+   variable) will likely never be accepted by Clang.
 -  clang does not support ``__builtin_apply`` and friends; this extension
is extremely obscure and 
diff icult to implement reliably.
 

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b3b3bc723863..90a48ad7e93c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -137,8 +137,9 @@ def err_vla_decl_has_static_storage : Error<
   "variable length array declaration cannot have 'static' storage duration">;
 def err_vla_decl_has_extern_linkage : Error<
   "variable length array declaration cannot have 'extern' linkage">;
-def ext_vla_folded_to_constant : Extension<
-  "variable length array folded to constant array as an extension">, 
InGroup;
+def ext_vla_folded_to_constant : ExtWarn<
+  "variable length array folded to constant array as an extension">,
+  InGroup;
 def err_vla_un

[PATCH] D89520: Don't permit array bound constant folding in OpenCL.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

I am OK with the changes regarding null pointer. I guess people seldom set 
pointer to zero address in OpenCL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89520

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

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 298759.
dgoldman added a comment.

Fix merge


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89579

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -356,6 +356,22 @@
 )cpp",
   "DeclRefExpr"},
 
+  // Objective-C nullability attributes.
+  {
+  R"cpp(
+@interface I{}
+@property(nullable) [[^I]] *x;
+@end
+  )cpp",
+  "ObjCInterfaceTypeLoc"},
+  {
+  R"cpp(
+@interface I{}
+- (void)doSomething:(nonnull [[i^d]])argument;
+@end
+  )cpp",
+  "TypedefTypeLoc"},
+
   // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
   // Need to traverse the contents of the OpaqueValueExpr to the POE,
   // and ensure we traverse only the syntactic form of the PseudoObjectExpr.
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1991,6 +1991,91 @@
 HI.NamespaceScope = "ObjC::"; // FIXME: fix it
 HI.Definition = "char data";
   }},
+  {
+  R"cpp(
+  @interface MYObject
+  @end
+  @interface Interface
+  @property(retain) [[MYOb^ject]] *x;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "MYObject";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "";
+HI.Definition = "@interface MYObject\n@end";
+  }},
+  // Should work even with nullability attribute.
+  {
+  R"cpp(
+  @interface MYObject
+  @end
+  @interface Interface
+  @property(nonnull) [[MYOb^ject]] *x;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "MYObject";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "";
+HI.Definition = "@interface MYObject\n@end";
+  }},
+  {
+  R"cpp(
+  @interface MYObject
+  @end
+  @interface Interface
+  - (void)doWith:([[MYOb^ject]] *)object;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "MYObject";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "";
+HI.Definition = "@interface MYObject\n@end";
+  }},
+  {
+  R"cpp(
+  @interface MYObject
+  @end
+  @interface Interface
+  - (void)doWith:(nullable [[MYOb^ject]] *)object;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "MYObject";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "";
+HI.Definition = "@interface MYObject\n@end";
+  }},
+  {
+  R"cpp(
+  @class ForwardDeclared;
+  @interface Interface
+  @property(retain) [[Forward^Declared]] *x;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "ForwardDeclared";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "";
+HI.Definition = "@class ForwardDeclared;";
+  }},
+  {
+  R"cpp(
+  @protocol Fooey
+  - (void)foo;
+  @end
+  @interface Interface
+  @property(retain) id<[[Fo^oey]]> fooey;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Fooey";
+HI.Kind = index::SymbolKind::Protocol;
+HI.NamespaceScope = "";
+HI.Definition = "@protocol Fooey\n@end";
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -820,6 +820,24 @@
   EXPECT_DECLS("ObjCPropertyRefExpr",
"@property(atomic, retain, readwrite) I *x");
 
+  Code = R"cpp(
+@interface MYObject
+@end
+@interface Interface
+@property(retain) [[MYObject]] *x;
+@end
+  )cpp";
+  EXPECT_DECLS("ObjCInterfaceTypeLoc", "@interface MYObject");
+
+  Code = R"cpp(
+   

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 298757.
dgoldman added a comment.

Lint fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89579

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2005,8 +2005,8 @@
 HI.NamespaceScope = "";
 HI.Definition = "@interface MYObject\n@end";
   }},
+  // Should work even with nullability attribute.
   {
-  // Should work even with nullability attribute.
   R"cpp(
   @interface MYObject
   @end
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -827,8 +827,7 @@
 @property(retain) [[MYObject]] *x;
 @end
   )cpp";
-  EXPECT_DECLS("ObjCInterfaceTypeLoc",
-   "@interface MYObject");
+  EXPECT_DECLS("ObjCInterfaceTypeLoc", "@interface MYObject");
 
   Code = R"cpp(
 @interface MYObject2
@@ -837,8 +836,7 @@
 @property(retain, nonnull) [[MYObject2]] *x;
 @end
   )cpp";
-  EXPECT_DECLS("ObjCInterfaceTypeLoc",
-   "@interface MYObject2");
+  EXPECT_DECLS("ObjCInterfaceTypeLoc", "@interface MYObject2");
 
   Code = R"cpp(
 @protocol Foo
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -619,7 +619,8 @@
   // AttributeTypeLoc points to the attribute's range, NOT the modified
   // type's range.
   if (auto AT = TL->getAs())
-S = AT.getModifiedLoc().getSourceRange();
+S = AT.getModifiedLoc()
+.getSourceRange(); // should we just return false?
 }
 if (!SelChecker.mayHit(S)) {
   dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2005,8 +2005,8 @@
 HI.NamespaceScope = "";
 HI.Definition = "@interface MYObject\n@end";
   }},
+  // Should work even with nullability attribute.
   {
-  // Should work even with nullability attribute.
   R"cpp(
   @interface MYObject
   @end
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -827,8 +827,7 @@
 @property(retain) [[MYObject]] *x;
 @end
   )cpp";
-  EXPECT_DECLS("ObjCInterfaceTypeLoc",
-   "@interface MYObject");
+  EXPECT_DECLS("ObjCInterfaceTypeLoc", "@interface MYObject");
 
   Code = R"cpp(
 @interface MYObject2
@@ -837,8 +836,7 @@
 @property(retain, nonnull) [[MYObject2]] *x;
 @end
   )cpp";
-  EXPECT_DECLS("ObjCInterfaceTypeLoc",
-   "@interface MYObject2");
+  EXPECT_DECLS("ObjCInterfaceTypeLoc", "@interface MYObject2");
 
   Code = R"cpp(
 @protocol Foo
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -619,7 +619,8 @@
   // AttributeTypeLoc points to the attribute's range, NOT the modified
   // type's range.
   if (auto AT = TL->getAs())
-S = AT.getModifiedLoc().getSourceRange();
+S = AT.getModifiedLoc()
+.getSourceRange(); // should we just return false?
 }
 if (!SelChecker.mayHit(S)) {
   dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89496: [Format/ObjC] Correctly handle base class with lightweight generics and protocol

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG24b5266892c3: [Format/ObjC] Correctly handle base class with 
lightweight generics and protocol (authored by benhamilton).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89496

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -328,6 +328,18 @@
"+ (id)init;\n"
"@end");
 
+  verifyFormat("@interface Foo> : Xyzzy   {\n"
+   "  int _i;\n"
+   "}\n"
+   "+ (id)init;\n"
+   "@end");
+
+  verifyFormat("@interface Foo : Bar  \n"
+   "@end");
+
+  verifyFormat("@interface Foo : Bar  \n"
+   "@end");
+
   verifyFormat("@interface Foo (HackStuff) {\n"
"  int _i;\n"
"}\n"
@@ -413,6 +425,10 @@
"f,\n"
"f> {\n"
"}");
+  verifyFormat("@interface g\n"
+   ": g \n"
+   "  \n"
+   "@end");
 }
 
 TEST_F(FormatTestObjC, FormatObjCImplementation) {
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -118,6 +118,7 @@
   // parses the record as a child block, i.e. if the class declaration is an
   // expression.
   void parseRecord(bool ParseAsExpr = false);
+  void parseObjCLightweightGenerics();
   void parseObjCMethod();
   void parseObjCProtocolList();
   void parseObjCUntilAtEnd();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2612,32 +2612,15 @@
   // @interface can be followed by a lightweight generic
   // specialization list, then either a base class or a category.
   if (FormatTok->Tok.is(tok::less)) {
-// Unlike protocol lists, generic parameterizations support
-// nested angles:
-//
-// @interface Foo> :
-// NSObject 
-//
-// so we need to count how many open angles we have left.
-unsigned NumOpenAngles = 1;
-do {
-  nextToken();
-  // Early exit in case someone forgot a close angle.
-  if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
-  FormatTok->Tok.isObjCAtKeyword(tok::objc_end))
-break;
-  if (FormatTok->Tok.is(tok::less))
-++NumOpenAngles;
-  else if (FormatTok->Tok.is(tok::greater)) {
-assert(NumOpenAngles > 0 && "'>' makes NumOpenAngles negative");
---NumOpenAngles;
-  }
-} while (!eof() && NumOpenAngles != 0);
-nextToken(); // Skip '>'.
+parseObjCLightweightGenerics();
   }
   if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
 nextToken(); // base class name
+// The base class can also have lightweight generics applied to it.
+if (FormatTok->Tok.is(tok::less)) {
+  parseObjCLightweightGenerics();
+}
   } else if (FormatTok->Tok.is(tok::l_paren))
 // Skip category, if present.
 parseParens();
@@ -2658,6 +2641,32 @@
   parseObjCUntilAtEnd();
 }
 
+void UnwrappedLineParser::parseObjCLightweightGenerics() {
+  assert(FormatTok->Tok.is(tok::less));
+  // Unlike protocol lists, generic parameterizations support
+  // nested angles:
+  //
+  // @interface Foo> :
+  // NSObject 
+  //
+  // so we need to count how many open angles we have left.
+  unsigned NumOpenAngles = 1;
+  do {
+nextToken();
+// Early exit in case someone forgot a close angle.
+if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
+FormatTok->Tok.isObjCAtKeyword(tok::objc_end))
+  break;
+if (FormatTok->Tok.is(tok::less))
+  ++NumOpenAngles;
+else if (FormatTok->Tok.is(tok::greater)) {
+  assert(NumOpenAngles > 0 && "'>' makes NumOpenAngles negative");
+  --NumOpenAngles;
+}
+  } while (!eof() && NumOpenAngles != 0);
+  nextToken(); // Skip '>'.
+}
+
 // Returns true for the declaration/definition form of @protocol,
 // false for the expression form.
 bool UnwrappedLineParser::parseObjCProtocol() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 24b5266 - [Format/ObjC] Correctly handle base class with lightweight generics and protocol

[Ben Hamilton via cfe-commits]

Author: Ben Hamilton
Date: 2020-10-16T15:12:25-06:00
New Revision: 24b5266892c30e2c9cb6ea28c2631e988a5754b6

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

LOG: [Format/ObjC] Correctly handle base class with lightweight generics and 
protocol

ClangFormat does not correctly handle an Objective-C interface declaration
with both lightweight generics and a protocol conformance.

This simple example:

```
@interface Foo : Bar  

@end
```

means `Foo` extends `Bar` (a lightweight generic class whose type
parameter is `Baz`) and also conforms to the protocol `Blech`.

ClangFormat should not apply any changes to the above example, but
instead it currently formats it quite poorly:

```
@interface Foo : Bar 


@end
```

The bug is that `UnwrappedLineParser` assumes an open-angle bracket
after a base class name is a protocol list, but it can also be a
lightweight generic specification.

This diff fixes the bug by factoring out the logic to parse
lightweight generics so it can apply both to the declared class
as well as the base class.

Test Plan: New tests added. Ran tests with:
  % ninja FormatTests && ./tools/clang/unittests/Format/FormatTests
  Confirmed tests failed before diff and passed after diff.

Reviewed By: sammccall, MyDeveloperDay

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h
clang/unittests/Format/FormatTestObjC.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 7075a6fe33f7..ab1f647481d0 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2612,32 +2612,15 @@ void 
UnwrappedLineParser::parseObjCInterfaceOrImplementation() {
   // @interface can be followed by a lightweight generic
   // specialization list, then either a base class or a category.
   if (FormatTok->Tok.is(tok::less)) {
-// Unlike protocol lists, generic parameterizations support
-// nested angles:
-//
-// @interface Foo> :
-// NSObject 
-//
-// so we need to count how many open angles we have left.
-unsigned NumOpenAngles = 1;
-do {
-  nextToken();
-  // Early exit in case someone forgot a close angle.
-  if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
-  FormatTok->Tok.isObjCAtKeyword(tok::objc_end))
-break;
-  if (FormatTok->Tok.is(tok::less))
-++NumOpenAngles;
-  else if (FormatTok->Tok.is(tok::greater)) {
-assert(NumOpenAngles > 0 && "'>' makes NumOpenAngles negative");
---NumOpenAngles;
-  }
-} while (!eof() && NumOpenAngles != 0);
-nextToken(); // Skip '>'.
+parseObjCLightweightGenerics();
   }
   if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
 nextToken(); // base class name
+// The base class can also have lightweight generics applied to it.
+if (FormatTok->Tok.is(tok::less)) {
+  parseObjCLightweightGenerics();
+}
   } else if (FormatTok->Tok.is(tok::l_paren))
 // Skip category, if present.
 parseParens();
@@ -2658,6 +2641,32 @@ void 
UnwrappedLineParser::parseObjCInterfaceOrImplementation() {
   parseObjCUntilAtEnd();
 }
 
+void UnwrappedLineParser::parseObjCLightweightGenerics() {
+  assert(FormatTok->Tok.is(tok::less));
+  // Unlike protocol lists, generic parameterizations support
+  // nested angles:
+  //
+  // @interface Foo> :
+  // NSObject 
+  //
+  // so we need to count how many open angles we have left.
+  unsigned NumOpenAngles = 1;
+  do {
+nextToken();
+// Early exit in case someone forgot a close angle.
+if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
+FormatTok->Tok.isObjCAtKeyword(tok::objc_end))
+  break;
+if (FormatTok->Tok.is(tok::less))
+  ++NumOpenAngles;
+else if (FormatTok->Tok.is(tok::greater)) {
+  assert(NumOpenAngles > 0 && "'>' makes NumOpenAngles negative");
+  --NumOpenAngles;
+}
+  } while (!eof() && NumOpenAngles != 0);
+  nextToken(); // Skip '>'.
+}
+
 // Returns true for the declaration/definition form of @protocol,
 // false for the expression form.
 bool UnwrappedLineParser::parseObjCProtocol() {

diff  --git a/clang/lib/Format/UnwrappedLineParser.h 
b/clang/lib/Format/UnwrappedLineParser.h
index 8b3aa4c84edb..d682b4d6acd8 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -118,6 +118,7 @@ class UnwrappedLineParser {
   // parses the record as a child block, i.e. if the class declaration is an
   // expression.
   void parseRecord(bool ParseAsExpr = false);
+  void parseObjCLightweightGenerics();
   void parseObjCMethod();
   void parseObjCProtocolList();

[PATCH] D89573: [Driver] Incorporate -mfloat-abi in the computed triple on ARM

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/Driver/ToolChain.cpp:807
+default:
+  break;
+}

Do we want to error in the "default" case?  Not that anyone is likely to use 
-mfloat-abi on those targets, but I'd prefer not to silently miscompile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89573

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

[PATCH] D87590: Backport D79219, D85820, D86134 to 10.0 branch

[Brian J. Cardiff via Phabricator via cfe-commits]

bcardiff abandoned this revision.
bcardiff added a comment.

Closing in favor of D84563 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87590

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

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89523

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

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

[Felix Berger via Phabricator via cfe-commits]

flx added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:52
   AllowedTypes(
   utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
 

lebedev.ri wrote:
> flx wrote:
> > lebedev.ri wrote:
> > > Just put it here?
> > I tried this first, but this list  is substring matched against only on the 
> > non-qualified type name, so std::function would not match anything and if 
> > we added "function" here it would match many other types that contain the 
> > word function.
> I would personally say it's a bug, especially because i personally don't like 
> hardcoded "choices" that are impossible to change.
> 
Are you referring to how the matching of "AllowedTypes" works or the fact that 
std::function causes false positives?

The latter is currently a bug and by excluding it now its severity is 
downgraded from bug to missing feature. Is your concern that you would like to 
have std::function be matched even if it is a false positive?

Regarding "AllowedTypes" matching I'm also surprised to find it matches 
substrings without namespaces as the name doesn't communicate this and makes it 
an imprecise net to cast. Changing it could break existing users that have type 
substrings configured though.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:409
+
+namespace std {
+

gribozavr2 wrote:
> Could you add a nested inline namespace to better imitate what declarations 
> look like in libc++?
I'm not sure I follow.  I looked through the other tests that declare a std 
function and copied the declaration from modernize-avoid-bind.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89332

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

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

[Felix Berger via Phabricator via cfe-commits]

flx updated this revision to Diff 298745.
flx added a comment.

Add more complete fake version of std::function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89332

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -405,3 +405,36 @@
   ExpensiveToCopyType Orig;
   const ExpensiveToCopyType Copy = freeFunctionWithDefaultArg(&Orig);
 }
+
+// Let's fake a minimal std::function-like facility.
+namespace std {
+template 
+_Tp declval();
+
+template 
+struct __res {
+  template 
+  static decltype(declval<_Functor>()(_Args()...)) _S_test(int);
+
+  template 
+  static void _S_test(...);
+
+  using type = decltype(_S_test<_ArgTypes...>(0));
+};
+
+template 
+struct function;
+
+template 
+struct function {
+  template ::type>
+  function(_Functor) {}
+};
+} // namespace std
+
+void negativeStdFunction() {
+  std::function Orig;
+  std::function Copy = Orig;
+  int i = Orig();
+}
Index: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -19,6 +19,10 @@
 namespace performance {
 namespace {
 
+using namespace ::clang::ast_matchers;
+using ::clang::ast_matchers::internal::Matcher;
+using utils::decl_ref_expr::isOnlyUsedAsConst;
+
 void recordFixes(const VarDecl &Var, ASTContext &Context,
  DiagnosticBuilder &Diagnostic) {
   Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context);
@@ -29,10 +33,17 @@
   }
 }
 
-} // namespace
+// Always allow std::function to be copied. Since its call operator is const 
but
+// can modify the state of the underlying functor we cannot ell whether the 
copy
+// is unnecessary.
+AST_MATCHER(NamedDecl, isStdFunction) {
+  // First use the fast getName() method to avoid unnecessary calls to the
+  // slow getQualifiedNameAsString().
+  return Node.getName() == "function" &&
+ Node.getQualifiedNameAsString() == "std::function";
+}
 
-using namespace ::clang::ast_matchers;
-using utils::decl_ref_expr::isOnlyUsedAsConst;
+} // namespace
 
 UnnecessaryCopyInitialization::UnnecessaryCopyInitialization(
 StringRef Name, ClangTidyContext *Context)
@@ -57,7 +68,7 @@
unless(callee(cxxMethodDecl(
   .bind("initFunctionCall");
 
-  auto localVarCopiedFrom = [this](const internal::Matcher &CopyCtorArg) 
{
+  auto localVarCopiedFrom = [this](const Matcher &CopyCtorArg) {
 return compoundStmt(
forEachDescendant(
declStmt(
@@ -66,8 +77,9 @@
hasCanonicalType(
matchers::isExpensiveToCopy()),
unless(hasDeclaration(namedDecl(
-   matchers::matchesAnyListedName(
-   AllowedTypes)),
+   
anyOf(matchers::matchesAnyListedName(
+ AllowedTypes),
+ isStdFunction())),
unless(isImplicit()),
hasInitializer(traverse(
ast_type_traits::TK_AsIs,


Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -405,3 +405,36 @@
   ExpensiveToCopyType Orig;
   const ExpensiveToCopyType Copy = freeFunctionWithDefaultArg(&Orig);
 }
+
+// Let's fake a minimal std::function-like facility.
+namespace std {
+template 
+_Tp declval();
+
+template 
+struct __res {
+  template 
+  static decltype(declval<_Functor>()(_Args()...)) _S_test(int);
+
+  template 
+  static void _S_test(...);
+
+  using type = decltype(_S_test<_ArgTypes...>(0));
+};
+
+template 
+struct function;
+
+template 
+struct function {
+  template ::type>
+  function(_Functor) {}
+};
+} // namespace std
+
+void negativeStdFunction() {
+  std::function Orig;
+  std::function Copy = Orig;
+  

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[Richard Smith - zygoloid 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 rG7e801ca0efa9: Treat constant contexts as being in the 
default rounding mode. (authored by rsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89360

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/Sema/rounding-math.c
  clang/test/SemaCXX/rounding-math.cpp


Index: clang/test/SemaCXX/rounding-math.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+enum Enum { enum_a = f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : f(3);
+};
+
+void f(Bitfield &b) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 
0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performed during
+  //   translation or during program execution.
+  const int k = 3 * (1.0 / 3.0);
+  static_assert(k == 1, "");
+}
+
+int *h() {
+  return new int[int(-3 * (1.0 / 3.0))]; // norounding-error {{too large}}
+}
Index: clang/test/Sema/rounding-math.c
===
--- /dev/null
+++ clang/test/Sema/rounding-math.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -std=c17 -verify=rounding-std %s 
-frounding-math
+// RUN: %clang_cc1 -triple x86_64-linux -std=gnu17 -verify=rounding-gnu %s 
-frounding-math
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+double a = 1.0 / 3.0;
+
+#define f(n) ((n) * (1.0 / 3.0))
+_Static_assert(fold((int)f(3)) == 1, "");
+
+typedef int T[fold((int)f(3))];
+typedef int T[1];
+
+enum Enum { enum_a = (int)f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : fold((int)f(3));
+};
+
+void bitfield(struct Bitfield *b) {
+  b->n = (int)(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 
to 0}}
+}
+
+void vlas() {
+  // Under -frounding-math, this is a VLA.
+  // FIXME: Due to PR44406, in GNU mode we constant-fold the initializer 
resulting in a non-VLA.
+  typedef int vla[(int)(-3 * (1.0 / 3.0))]; // norounding-error {{negative 
size}} rounding-gnu-error {{negative size}}
+  struct X { vla v; }; // rounding-std-error {{fields must have a constant 
size}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2528,6 +2528,11 @@
 /// Check if the given evaluation result is allowed for constant evaluation.
 static bool checkFloatingPointResult(EvalInfo &Info, const Expr *E,
  APFloat::opStatus St) {
+  // In a constant context, assume that any dynamic rounding mode or FP
+  // exception state matches the default floating-point environment.
+  if (Info.InConstantContext)
+return true;
+
   FPOptions FPO = E->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
   if ((St & APFloat::opInexact) &&
   FPO.getRoundingMode() == llvm::RoundingMode::Dynamic) {


Index: clang/test/SemaCXX/rounding-math.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+enum Enum { enum_a = f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : f(3);
+};
+
+void f(Bitfield &b) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performe

[clang] 7e801ca - Treat constant contexts as being in the default rounding mode.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-10-16T13:26:15-07:00
New Revision: 7e801ca0efa99f7cec7a2aea30513ad282030b51

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

LOG: Treat constant contexts as being in the default rounding mode.

This addresses a regression where pretty much all C++ compilations using
-frounding-math now fail, due to rounding being performed in constexpr
function definitions in the standard library.

This follows the "manifestly constant evaluated" approach described in
https://reviews.llvm.org/D87528#2270676 -- evaluations that are required
to succeed at compile time are permitted even in regions with dynamic
rounding modes, as are (unfortunately) the evaluation of the
initializers of local variables of const integral types.

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

Added: 
clang/test/Sema/rounding-math.c
clang/test/SemaCXX/rounding-math.cpp

Modified: 
clang/lib/AST/ExprConstant.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fda565354b92..7afc44de 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2528,6 +2528,11 @@ static llvm::RoundingMode getActiveRoundingMode(EvalInfo 
&Info, const Expr *E,
 /// Check if the given evaluation result is allowed for constant evaluation.
 static bool checkFloatingPointResult(EvalInfo &Info, const Expr *E,
  APFloat::opStatus St) {
+  // In a constant context, assume that any dynamic rounding mode or FP
+  // exception state matches the default floating-point environment.
+  if (Info.InConstantContext)
+return true;
+
   FPOptions FPO = E->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
   if ((St & APFloat::opInexact) &&
   FPO.getRoundingMode() == llvm::RoundingMode::Dynamic) {

diff  --git a/clang/test/Sema/rounding-math.c b/clang/test/Sema/rounding-math.c
new file mode 100644
index ..3cb5d034b143
--- /dev/null
+++ b/clang/test/Sema/rounding-math.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -std=c17 -verify=rounding-std %s 
-frounding-math
+// RUN: %clang_cc1 -triple x86_64-linux -std=gnu17 -verify=rounding-gnu %s 
-frounding-math
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+double a = 1.0 / 3.0;
+
+#define f(n) ((n) * (1.0 / 3.0))
+_Static_assert(fold((int)f(3)) == 1, "");
+
+typedef int T[fold((int)f(3))];
+typedef int T[1];
+
+enum Enum { enum_a = (int)f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : fold((int)f(3));
+};
+
+void bitfield(struct Bitfield *b) {
+  b->n = (int)(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 
to 0}}
+}
+
+void vlas() {
+  // Under -frounding-math, this is a VLA.
+  // FIXME: Due to PR44406, in GNU mode we constant-fold the initializer 
resulting in a non-VLA.
+  typedef int vla[(int)(-3 * (1.0 / 3.0))]; // norounding-error {{negative 
size}} rounding-gnu-error {{negative size}}
+  struct X { vla v; }; // rounding-std-error {{fields must have a constant 
size}}
+}

diff  --git a/clang/test/SemaCXX/rounding-math.cpp 
b/clang/test/SemaCXX/rounding-math.cpp
new file mode 100644
index ..3577d2fc0756
--- /dev/null
+++ b/clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+enum Enum { enum_a = f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : f(3);
+};
+
+void f(Bitfield &b) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 
0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performed during
+  //   translation or during program execution.
+  const int k = 3 * (1.0 / 3.0);
+  static_assert(k == 1, "");
+}
+
+int *h() {
+  return new int[int(-3 * (1.0 / 3.0))]; // norounding-error {{too large}}
+}



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

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

[Atmn Patel via Phabricator via cfe-commits]

atmnpatel updated this revision to Diff 298740.
atmnpatel added a comment.

fixed splice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86844

Files:
  clang/test/Misc/loop-opt-setup.c
  llvm/include/llvm/Transforms/Utils/LoopUtils.h
  llvm/lib/Transforms/Scalar/LoopDeletion.cpp
  llvm/lib/Transforms/Utils/LoopUtils.cpp
  llvm/test/Other/loop-deletion-printer.ll
  llvm/test/Other/loop-pm-invalidation.ll
  llvm/test/Transforms/LICM/2003-02-27-PreheaderProblem.ll
  llvm/test/Transforms/LoopDeletion/2017-07-11-incremental-dt.ll
  llvm/test/Transforms/LoopDeletion/basic-remark.ll
  llvm/test/Transforms/LoopDeletion/diundef.ll
  llvm/test/Transforms/LoopDeletion/invalidation.ll
  llvm/test/Transforms/LoopDeletion/multiple-exit-conditions.ll
  llvm/test/Transforms/LoopDeletion/multiple-exits.ll
  llvm/test/Transforms/LoopDeletion/mustprogress.ll
  llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
  llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
  llvm/test/Transforms/SCCP/calltest.ll
  llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll

Index: llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
===
--- llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
+++ llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
@@ -7,7 +7,7 @@
 
 target triple = "x86_64-unknown-linux-gnu"
 
-define void @pr37888() {
+define void @pr37888() willreturn {
 ; CHECK-LABEL: define void @pr37888()
 entry:
   %tobool = icmp ne i16 undef, 0
Index: llvm/test/Transforms/SCCP/calltest.ll
===
--- llvm/test/Transforms/SCCP/calltest.ll
+++ llvm/test/Transforms/SCCP/calltest.ll
@@ -4,7 +4,7 @@
 %empty = type {}
 declare %empty @has_side_effects()
 
-define double @test_0(i32 %param) {
+define double @test_0(i32 %param) willreturn {
 ; CHECK-LABEL: @test_0(
 ; CHECK-NOT: br
 entry:
Index: llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
===
--- llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
+++ llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
@@ -3,7 +3,7 @@
 ; Checking that possible users of instruction from the loop in
 ; unreachable blocks are handled.
 
-define i64 @foo() {
+define i64 @foo() willreturn {
 entry:
   br label %invloop
 ; CHECK-LABEL-NOT: invloop
Index: llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
===
--- llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
+++ llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
@@ -216,7 +216,7 @@
 ; Show recursive deletion of loops. Since we start with subloops and progress outward 
 ; to parent loop, we first delete the loop L2. Now loop L1 becomes a non-loop since it's backedge
 ; from L2's preheader to L1's exit block is never taken. So, L1 gets deleted as well.
-define void @test8(i64 %n) {
+define void @test8(i64 %n) #0 {
 ; CHECK-LABEL: test8
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT: br label %exit
@@ -318,7 +318,7 @@
 ; deleted.
 ; In the next iteration, since L2 is never executed and has no subloops, we delete
 ; L2 as well. Finally, the outermost loop L1 is deleted.
-define void @test11(i64 %n) {
+define void @test11(i64 %n) #0 {
 ; CHECK-LABEL: test11
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT: br label %exit
@@ -355,7 +355,7 @@
 
 
 ; 2 edges from a single exiting block to the exit block.
-define i64 @test12(i64 %n){
+define i64 @test12(i64 %n) #0 {
 ;CHECK-LABEL: @test12
 ; CHECK-NOT: L1:
 ; CHECK-NOT: L1Latch:
@@ -392,7 +392,7 @@
 }
 
 ; multiple edges to exit block from the same exiting blocks
-define i64 @test13(i64 %n) {
+define i64 @test13(i64 %n) #0 {
 ; CHECK-LABEL: @test13
 ; CHECK-NOT: L1:
 ; CHECK-NOT: L1Latch:
@@ -433,3 +433,5 @@
   %y.phi = phi i64 [ 10, %L1Block ], [ 10, %L1Block ], [ %y.next, %L1 ], [ 30, %L1Latch ], [ 30, %L1Latch ]
   ret i64 %y.phi
 }
+
+attributes #0 = { willreturn }
Index: llvm/test/Transforms/LoopDeletion/mustprogress.ll
===
--- /dev/null
+++ llvm/test/Transforms/LoopDeletion/mustprogress.ll
@@ -0,0 +1,241 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --check-attributes
+; RUN: opt < %s -loop-deletion -S | FileCheck %s
+
+;; Original C Code:
+;;  void unknown_tripcount_mustprogress_attr_mustprogress_loopmd(int a, int b) {
+;;for (; a < b;) ;
+;;for (;;) ;
+;;  }
+
+define void @unknown_tripcount_mustprogress_attr_mustprogress_loopmd(i32 %a, i32 %b) #0 {
+; CHECK: Function Attrs: mustprogress
+; CHECK-LABEL: define {{[^@]+}}@unknown_tripcount_mustprogress_attr_mustprogress_loopmd
+; CHECK-SAME: (i32 [[A:%.*]], i32 [[B:%.*]]) [[ATTR0:#.*]] {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:br label [[FOR_END:%.*]]
+; CHECK:   for.end:
+; CHECK-N

[clang] 48c70c1 - Extend memset-to-zero optimization to C++11 aggregate functional casts

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-10-16T13:21:08-07:00
New Revision: 48c70c1664aa4512cb7e08352dd8eb33dde4807c

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

LOG: Extend memset-to-zero optimization to C++11 aggregate functional casts
Aggr{...}.

We previously missed these cases due to not stepping over the additional
AST nodes representing their syntactic form.

Added: 


Modified: 
clang/lib/CodeGen/CGExprAgg.cpp
clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 43e23d986ae0..625b9116ff25 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -1759,7 +1759,9 @@ void 
AggExprEmitter::VisitDesignatedInitUpdateExpr(DesignatedInitUpdateExpr *E)
 /// non-zero bytes that will be stored when outputting the initializer for the
 /// specified initializer expression.
 static CharUnits GetNumNonZeroBytesInInit(const Expr *E, CodeGenFunction &CGF) 
{
-  E = E->IgnoreParens();
+  if (auto *MTE = dyn_cast(E))
+E = MTE->getSubExpr();
+  E = E->IgnoreParenNoopCasts(CGF.getContext());
 
   // 0 and 0.0 won't require any non-zero stores!
   if (isSimpleZero(E, CGF)) return CharUnits::Zero();
@@ -1808,7 +1810,7 @@ static CharUnits GetNumNonZeroBytesInInit(const Expr *E, 
CodeGenFunction &CGF) {
 }
   }
 
-
+  // FIXME: This overestimates the number of non-zero bytes for bit-fields.
   CharUnits NumNonZeroBytes = CharUnits::Zero();
   for (unsigned i = 0, e = ILE->getNumInits(); i != e; ++i)
 NumNonZeroBytes += GetNumNonZeroBytesInInit(ILE->getInit(i), CGF);

diff  --git a/clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp 
b/clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
index 6b10efd34f92..3e9d936a16f8 100644
--- a/clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ b/clang/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -S -emit-llvm -o - %s -triple x86_64-linux-gnu | 
FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -S -emit-llvm -o - %s -triple x86_64-linux-gnu | 
FileCheck %s
 
 struct A { int a, b; int f(); };
 
@@ -118,4 +119,24 @@ namespace ZeroInit {
   // This variable must be initialized elementwise.
   Filler data_e1[1024] = {};
   // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
+
+  struct Largeish {
+long a, b, c;
+  };
+  // CHECK: define {{.*}}@_ZN8ZeroInit9largeish1Ev(
+  // CHECK-NOT }
+  // CHECK: call {{.*}}memset
+  Largeish largeish1() { return {}; }
+  // CHECK: define {{.*}}@_ZN8ZeroInit9largeish2Ev(
+  // CHECK-NOT }
+  // CHECK: call {{.*}}memset
+  Largeish largeish2() { return Largeish(); }
+  // CHECK: define {{.*}}@_ZN8ZeroInit9largeish3Ev(
+  // CHECK-NOT }
+  // CHECK: call {{.*}}memset
+  Largeish largeish3() { return Largeish{}; }
+  // CHECK: define {{.*}}@_ZN8ZeroInit9largeish4Ev(
+  // CHECK-NOT }
+  // CHECK: call {{.*}}memset
+  Largeish largeish4() { return (Largeish){}; }
 }



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

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

[Atmn Patel via Phabricator via cfe-commits]

atmnpatel updated this revision to Diff 298736.
atmnpatel added a comment.

nit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86844

Files:
  clang/test/Misc/loop-opt-setup.c
  llvm/include/llvm/Transforms/Utils/LoopUtils.h
  llvm/lib/Transforms/Scalar/LoopDeletion.cpp
  llvm/lib/Transforms/Utils/LoopUtils.cpp
  llvm/test/Other/loop-deletion-printer.ll
  llvm/test/Other/loop-pm-invalidation.ll
  llvm/test/Transforms/LICM/2003-02-27-PreheaderProblem.ll
  llvm/test/Transforms/LoopDeletion/2017-07-11-incremental-dt.ll
  llvm/test/Transforms/LoopDeletion/basic-remark.ll
  llvm/test/Transforms/LoopDeletion/diundef.ll
  llvm/test/Transforms/LoopDeletion/invalidation.ll
  llvm/test/Transforms/LoopDeletion/multiple-exit-conditions.ll
  llvm/test/Transforms/LoopDeletion/multiple-exits.ll
  llvm/test/Transforms/LoopDeletion/mustprogress.ll
  llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
  llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
  llvm/test/Transforms/SCCP/calltest.ll
  llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll

Index: llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
===
--- llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
+++ llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
@@ -7,7 +7,7 @@
 
 target triple = "x86_64-unknown-linux-gnu"
 
-define void @pr37888() {
+define void @pr37888() willreturn {
 ; CHECK-LABEL: define void @pr37888()
 entry:
   %tobool = icmp ne i16 undef, 0
Index: llvm/test/Transforms/SCCP/calltest.ll
===
--- llvm/test/Transforms/SCCP/calltest.ll
+++ llvm/test/Transforms/SCCP/calltest.ll
@@ -4,7 +4,7 @@
 %empty = type {}
 declare %empty @has_side_effects()
 
-define double @test_0(i32 %param) {
+define double @test_0(i32 %param) willreturn {
 ; CHECK-LABEL: @test_0(
 ; CHECK-NOT: br
 entry:
Index: llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
===
--- llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
+++ llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
@@ -3,7 +3,7 @@
 ; Checking that possible users of instruction from the loop in
 ; unreachable blocks are handled.
 
-define i64 @foo() {
+define i64 @foo() willreturn {
 entry:
   br label %invloop
 ; CHECK-LABEL-NOT: invloop
Index: llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
===
--- llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
+++ llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
@@ -216,7 +216,7 @@
 ; Show recursive deletion of loops. Since we start with subloops and progress outward 
 ; to parent loop, we first delete the loop L2. Now loop L1 becomes a non-loop since it's backedge
 ; from L2's preheader to L1's exit block is never taken. So, L1 gets deleted as well.
-define void @test8(i64 %n) {
+define void @test8(i64 %n) #0 {
 ; CHECK-LABEL: test8
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT: br label %exit
@@ -318,7 +318,7 @@
 ; deleted.
 ; In the next iteration, since L2 is never executed and has no subloops, we delete
 ; L2 as well. Finally, the outermost loop L1 is deleted.
-define void @test11(i64 %n) {
+define void @test11(i64 %n) #0 {
 ; CHECK-LABEL: test11
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT: br label %exit
@@ -355,7 +355,7 @@
 
 
 ; 2 edges from a single exiting block to the exit block.
-define i64 @test12(i64 %n){
+define i64 @test12(i64 %n) #0 {
 ;CHECK-LABEL: @test12
 ; CHECK-NOT: L1:
 ; CHECK-NOT: L1Latch:
@@ -392,7 +392,7 @@
 }
 
 ; multiple edges to exit block from the same exiting blocks
-define i64 @test13(i64 %n) {
+define i64 @test13(i64 %n) #0 {
 ; CHECK-LABEL: @test13
 ; CHECK-NOT: L1:
 ; CHECK-NOT: L1Latch:
@@ -433,3 +433,5 @@
   %y.phi = phi i64 [ 10, %L1Block ], [ 10, %L1Block ], [ %y.next, %L1 ], [ 30, %L1Latch ], [ 30, %L1Latch ]
   ret i64 %y.phi
 }
+
+attributes #0 = { willreturn }
Index: llvm/test/Transforms/LoopDeletion/mustprogress.ll
===
--- llvm/test/Transforms/LoopDeletion/mustprogress.ll
+++ llvm/test/Transforms/LoopDeletion/mustprogress.ll
@@ -14,7 +14,9 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:br label [[FOR_END:%.*]]
 ; CHECK:   for.end:
-; CHECK-NEXT:unreachable
+; CHECK-NEXT:br label [[FOR_COND1:%.*]]
+; CHECK:   for.cond1:
+; CHECK-NEXT:br label [[FOR_COND1]]
 ;
 entry:
   br label %for.cond
@@ -43,7 +45,9 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:br label [[FOR_END:%.*]]
 ; CHECK:   for.end:
-; CHECK-NEXT:unreachable
+; CHECK-NEXT:br label [[FOR_COND1:%.*]]
+; CHECK:   for.cond1:
+; CHECK-NEXT:br label [[FOR_COND1]]
 ;
 entry:
   br label %for.cond
Index: llvm/test/Transforms/LoopDeletion/multiple-exits.ll
===
---

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

I don't like the %clang_bin substitution - imho it's unclear for the test 
authors when to use it instead of %clang, but I can't come up with a better 
idea.

Is it true that %clang_bin is only necessary when the automatically added 
-disable-noundef-analysis flag triggers an unused argument warning, or are 
there other reasons?

I wonder it can be avoided by

- disable noundef analysis by default in cc1
- always add -enable-noundef-analysis in the driver when invoking cc1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85788

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

[PATCH] D86844: [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

[Atmn Patel via Phabricator via cfe-commits]

atmnpatel updated this revision to Diff 298734.
atmnpatel added a comment.

Reverted to prior diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86844

Files:
  clang/test/Misc/loop-opt-setup.c
  llvm/include/llvm/Transforms/Utils/LoopUtils.h
  llvm/lib/Transforms/Scalar/LoopDeletion.cpp
  llvm/lib/Transforms/Utils/LoopUtils.cpp
  llvm/test/Other/loop-deletion-printer.ll
  llvm/test/Other/loop-pm-invalidation.ll
  llvm/test/Transforms/LICM/2003-02-27-PreheaderProblem.ll
  llvm/test/Transforms/LoopDeletion/2017-07-11-incremental-dt.ll
  llvm/test/Transforms/LoopDeletion/basic-remark.ll
  llvm/test/Transforms/LoopDeletion/diundef.ll
  llvm/test/Transforms/LoopDeletion/invalidation.ll
  llvm/test/Transforms/LoopDeletion/multiple-exit-conditions.ll
  llvm/test/Transforms/LoopDeletion/multiple-exits.ll
  llvm/test/Transforms/LoopDeletion/mustprogress.ll
  llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
  llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
  llvm/test/Transforms/SCCP/calltest.ll
  llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll

Index: llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
===
--- llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
+++ llvm/test/Transforms/SimpleLoopUnswitch/pr37888.ll
@@ -7,7 +7,7 @@
 
 target triple = "x86_64-unknown-linux-gnu"
 
-define void @pr37888() {
+define void @pr37888() willreturn {
 ; CHECK-LABEL: define void @pr37888()
 entry:
   %tobool = icmp ne i16 undef, 0
Index: llvm/test/Transforms/SCCP/calltest.ll
===
--- llvm/test/Transforms/SCCP/calltest.ll
+++ llvm/test/Transforms/SCCP/calltest.ll
@@ -4,7 +4,7 @@
 %empty = type {}
 declare %empty @has_side_effects()
 
-define double @test_0(i32 %param) {
+define double @test_0(i32 %param) willreturn {
 ; CHECK-LABEL: @test_0(
 ; CHECK-NOT: br
 entry:
Index: llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
===
--- llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
+++ llvm/test/Transforms/LoopDeletion/use-in-unreachable.ll
@@ -3,7 +3,7 @@
 ; Checking that possible users of instruction from the loop in
 ; unreachable blocks are handled.
 
-define i64 @foo() {
+define i64 @foo() willreturn {
 entry:
   br label %invloop
 ; CHECK-LABEL-NOT: invloop
Index: llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
===
--- llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
+++ llvm/test/Transforms/LoopDeletion/unreachable-loops.ll
@@ -216,7 +216,7 @@
 ; Show recursive deletion of loops. Since we start with subloops and progress outward 
 ; to parent loop, we first delete the loop L2. Now loop L1 becomes a non-loop since it's backedge
 ; from L2's preheader to L1's exit block is never taken. So, L1 gets deleted as well.
-define void @test8(i64 %n) {
+define void @test8(i64 %n) #0 {
 ; CHECK-LABEL: test8
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT: br label %exit
@@ -318,7 +318,7 @@
 ; deleted.
 ; In the next iteration, since L2 is never executed and has no subloops, we delete
 ; L2 as well. Finally, the outermost loop L1 is deleted.
-define void @test11(i64 %n) {
+define void @test11(i64 %n) #0 {
 ; CHECK-LABEL: test11
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT: br label %exit
@@ -355,7 +355,7 @@
 
 
 ; 2 edges from a single exiting block to the exit block.
-define i64 @test12(i64 %n){
+define i64 @test12(i64 %n) #0 {
 ;CHECK-LABEL: @test12
 ; CHECK-NOT: L1:
 ; CHECK-NOT: L1Latch:
@@ -392,7 +392,7 @@
 }
 
 ; multiple edges to exit block from the same exiting blocks
-define i64 @test13(i64 %n) {
+define i64 @test13(i64 %n) #0 {
 ; CHECK-LABEL: @test13
 ; CHECK-NOT: L1:
 ; CHECK-NOT: L1Latch:
@@ -433,3 +433,5 @@
   %y.phi = phi i64 [ 10, %L1Block ], [ 10, %L1Block ], [ %y.next, %L1 ], [ 30, %L1Latch ], [ 30, %L1Latch ]
   ret i64 %y.phi
 }
+
+attributes #0 = { willreturn }
Index: llvm/test/Transforms/LoopDeletion/mustprogress.ll
===
--- llvm/test/Transforms/LoopDeletion/mustprogress.ll
+++ llvm/test/Transforms/LoopDeletion/mustprogress.ll
@@ -14,7 +14,9 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:br label [[FOR_END:%.*]]
 ; CHECK:   for.end:
-; CHECK-NEXT:unreachable
+; CHECK-NEXT:br label [[FOR_COND1:%.*]]
+; CHECK:   for.cond1:
+; CHECK-NEXT:br label [[FOR_COND1]]
 ;
 entry:
   br label %for.cond
@@ -43,7 +45,9 @@
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:br label [[FOR_END:%.*]]
 ; CHECK:   for.end:
-; CHECK-NEXT:unreachable
+; CHECK-NEXT:br label [[FOR_COND1:%.*]]
+; CHECK:   for.cond1:
+; CHECK-NEXT:br label [[FOR_COND1]]
 ;
 entry:
   br label %for.cond
Index: llvm/test/Transforms/LoopDeletion/multiple-exits.ll

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

[Gui Andrade via Phabricator via cfe-commits]

guiand added a comment.

Did we decide that we wanted this change then? I remember there being 
discussion around whether it's the right approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85788

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

[PATCH] D69840: [Basic] Make SourceLocation usable as key in hash maps, NFCI

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: sstefan1.

If you're still interested in pursuing this, I'm happy to review it.

A general comment is that there are a couple of functional changes in this 
patch, where hash values are changing and data structures are being changed, 
but they're buried in the noise of the refactoring that's going on at the same 
time. I suggest splitting this up at least into two, where most of the NFC 
changes are in a different commit.




Comment at: 
clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h:35
 private:
-  std::unordered_set MatchedTemplateLocations;
+  llvm::DenseSet MatchedTemplateLocations;
 };

Changing the data structure (`DenseSet` <= `std::unordered_set`) seems 
unrelated to the rest of the patch. If it's necessary / useful, then please do 
it separately. It'll be important to confirm that the users of 
`MatchedTemplateLocations` don't rely on the values having stable addresses.



Comment at: clang/include/clang/Basic/SourceLocation.h:178
 
+  unsigned getHashValue() const {
+return ID * 37U;

I suggest reusing the hash function for `unsigned`.


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

https://reviews.llvm.org/D69840

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

[PATCH] D89429: clang/Basic: Replace SourceManager::getMemoryBufferForFile, NFC

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.



Comment at: clang/lib/Basic/SourceManager.cpp:705
+llvm::Optional
+SourceManager::getMemoryBufferForFileOrNone(const FileEntry *File) {
   const SrcMgr::ContentCache *IR = getOrCreateContentCache(File);

Would it make sense to rename this to the slightly less verbose 
`getMemoryBufferOrNone` now that it takes just the File as an argument? Or 
maybe even `getBufferOrNone` like the method in `ContentCache`. 


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

https://reviews.llvm.org/D89429

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

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

[John McCall via Phabricator via cfe-commits]

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

Okay, thanks.  This LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89360

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

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

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

rsmith added a comment.

In D89360#2329167 , @rjmccall wrote:

> Okay.  I can accept the fairly unfortunate thing with `const int` 
> initializers.  When is `IsConstantContext` set in C, just static initializers?

This affects static initializers, enumerators, bit-widths, array bounds, case 
labels, and indexes in array designators. For code not relying on extensions, 
only static initializers are affected, but as an extension we do permit 
floating-point math in other contexts. I've added some tests for the C side of 
things to demonstrate.

The C tests reveal one issue: block-scope array bounds get constant-folded in 
GNU mode even if they contain floating-point operations. That's PR44406; D89523 
 and D89520  
have a fix for that which should hopefully land soon. I don't think we need to 
block on it.

In D89360#2334038 , @sepavloff wrote:

> Probably there is an issue with code generation. The source:
>
>   constexpr float func_01(float x, float y) {
> return x + y;
>   }
>   
>   float V1 = func_01(1.0F, 0x0.01p0F);
>
> compiled with '-frounding-math' must produce dynamic initializer. It however 
> is evaluated at compile time.

Evaluating it at compile time is in line with the plan from 
https://reviews.llvm.org/D87528#2270676. The C++ rule is that initializers for 
static storage duration variables are first evaluated during translation 
(therefore, per our plan, in the default rounding mode), and only evaluated at 
runtime (and therefore in the runtime rounding mode) if the compile-time 
evaluation fails. This is in line with the C rules; C11 F.8.5 says: "All 
computation for automatic initialization is done (as if) at execution time; 
thus, it is affected by any operative modes and raises floating-point 
exceptions as required by IEC 60559 (provided the state for the FENV_ACCESS 
pragma is ‘‘on’’). All computation for initialization of objects that have 
static or thread storage duration is done (as if) at translation time." C++ 
generalizes this by adding another phase of initialization (at runtime) if the 
translation-time initialization fails, but the translation-time evaluation of 
the initializer of `V1` succeeds so it should be treated as a constant 
initializer.

And just a side note: we're currently blocked by this. C++ builds using 
`-frounding-math` have not worked at all since D87822 
 landed. If we can't come to an agreement on a 
fix very soon, I think we need to revert D87822 
 while we figure it out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89360

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

[PATCH] D89582: clang/AMDGPU: Apply workgroup related attributes to all functions

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

In D89582#2335704 , @arsenm wrote:

> In D89582#2335671 , @rampitec wrote:
>
>> In D89582#2335619 , @arsenm wrote:
>>
>>> In D89582#2335574 , @yaxunl wrote:
>>>
 What if a device function is called by kernels with different work group 
 sizes, will caller's work group size override callee's work group size?
>>>
>>> It's user error to call a function with a larger range than the caller
>>
>> The problem is that user can override default on a kernel with the 
>> attribute, but cannot do so on function. So a module can be compiled with a 
>> default smaller than requested on one of the kernels.
>
>
>
>> Then if default is maximum 1024 and can only be overridden with the 
>> --gpu-max-threads-per-block option it would not be problem, if not the 
>> description of the option:
>>
>>   LANGOPT(GPUMaxThreadsPerBlock, 32, 256, "default max threads per block for 
>> kernel launch bounds for HIP")
>>
>> I.e. it says about the "default", so it should be perfectly legal to set a 
>> higher limits on a specific kernel. Should the option say it restricts the 
>> maximum it would be legal to apply it to functions as well.
>
> The current backend default ends up greatly restricting the registers used in 
> the functions, and increasing the spilling.

I know the problem, but it should be better to use AMDGPUPropagateAttributes 
for this. It will clone functions if needed.


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

https://reviews.llvm.org/D89582

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

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

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

rsmith updated this revision to Diff 298724.
rsmith added a comment.

- Add more testcases to demonstrate behavior in C.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89360

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/Sema/rounding-math.c
  clang/test/SemaCXX/rounding-math.cpp


Index: clang/test/SemaCXX/rounding-math.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+enum Enum { enum_a = f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : f(3);
+};
+
+void f(Bitfield &b) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 
0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performed during
+  //   translation or during program execution.
+  const int k = 3 * (1.0 / 3.0);
+  static_assert(k == 1, "");
+}
+
+int *h() {
+  return new int[int(-3 * (1.0 / 3.0))]; // norounding-error {{too large}}
+}
Index: clang/test/Sema/rounding-math.c
===
--- /dev/null
+++ clang/test/Sema/rounding-math.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -std=c17 -verify=rounding-std %s 
-frounding-math
+// RUN: %clang_cc1 -triple x86_64-linux -std=gnu17 -verify=rounding-gnu %s 
-frounding-math
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+double a = 1.0 / 3.0;
+
+#define f(n) ((n) * (1.0 / 3.0))
+_Static_assert(fold((int)f(3)) == 1, "");
+
+typedef int T[fold((int)f(3))];
+typedef int T[1];
+
+enum Enum { enum_a = (int)f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : fold((int)f(3));
+};
+
+void bitfield(struct Bitfield *b) {
+  b->n = (int)(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 
to 0}}
+}
+
+void vlas() {
+  // Under -frounding-math, this is a VLA.
+  // FIXME: Due to PR44406, in GNU mode we constant-fold the initializer 
resulting in a non-VLA.
+  typedef int vla[(int)(-3 * (1.0 / 3.0))]; // norounding-error {{negative 
size}} rounding-gnu-error {{negative size}}
+  struct X { vla v; }; // rounding-std-error {{fields must have a constant 
size}}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2528,6 +2528,11 @@
 /// Check if the given evaluation result is allowed for constant evaluation.
 static bool checkFloatingPointResult(EvalInfo &Info, const Expr *E,
  APFloat::opStatus St) {
+  // In a constant context, assume that any dynamic rounding mode or FP
+  // exception state matches the default floating-point environment.
+  if (Info.InConstantContext)
+return true;
+
   FPOptions FPO = E->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
   if ((St & APFloat::opInexact) &&
   FPO.getRoundingMode() == llvm::RoundingMode::Dynamic) {


Index: clang/test/SemaCXX/rounding-math.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/rounding-math.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -triple x86_64-linux -verify=norounding %s
+// RUN: %clang_cc1 -triple x86_64-linux -verify=rounding %s -frounding-math
+// rounding-no-diagnostics
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+
+constexpr double a = 1.0 / 3.0;
+
+constexpr int f(int n) { return int(n * (1.0 / 3.0)); }
+
+using T = int[f(3)];
+using T = int[1];
+
+enum Enum { enum_a = f(3) };
+
+struct Bitfield {
+  unsigned int n : 1;
+  unsigned int m : f(3);
+};
+
+void f(Bitfield &b) {
+  b.n = int(6 * (1.0 / 3.0)); // norounding-warning {{changes value from 2 to 0}}
+}
+
+const int k = 3 * (1.0 / 3.0);
+static_assert(k == 1, "");
+
+void g() {
+  // FIXME: Constant-evaluating this initializer is surprising, and violates
+  // the recommended practice in C++ [expr.const]p12:
+  //
+  //   Implementations should provide consistent results of floating-point
+  //   evaluations, irrespective of whether the evaluation is performed during
+  //   translation or during program execution.
+  const int k = 3 * (1.0 / 3.0);
+  static_assert(k == 1, "");
+}
+

[PATCH] D89582: clang/AMDGPU: Apply workgroup related attributes to all functions

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D89582#2335671 , @rampitec wrote:

> In D89582#2335619 , @arsenm wrote:
>
>> In D89582#2335574 , @yaxunl wrote:
>>
>>> What if a device function is called by kernels with different work group 
>>> sizes, will caller's work group size override callee's work group size?
>>
>> It's user error to call a function with a larger range than the caller
>
> The problem is that user can override default on a kernel with the attribute, 
> but cannot do so on function. So a module can be compiled with a default 
> smaller than requested on one of the kernels.



> Then if default is maximum 1024 and can only be overridden with the 
> --gpu-max-threads-per-block option it would not be problem, if not the 
> description of the option:
>
>   LANGOPT(GPUMaxThreadsPerBlock, 32, 256, "default max threads per block for 
> kernel launch bounds for HIP")
>
> I.e. it says about the "default", so it should be perfectly legal to set a 
> higher limits on a specific kernel. Should the option say it restricts the 
> maximum it would be legal to apply it to functions as well.

The current backend default ends up greatly restricting the registers used in 
the functions, and increasing the spilling.


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

https://reviews.llvm.org/D89582

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

[PATCH] D89582: clang/AMDGPU: Apply workgroup related attributes to all functions

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

In D89582#2335619 , @arsenm wrote:

> In D89582#2335574 , @yaxunl wrote:
>
>> What if a device function is called by kernels with different work group 
>> sizes, will caller's work group size override callee's work group size?
>
> It's user error to call a function with a larger range than the caller

The problem is that user can override default on a kernel with the attribute, 
but cannot do so on function. So a module can be compiled with a default 
smaller than requested on one of the kernels.

Then if default is maximum 1024 and can only be overridden with the 
--gpu-max-threads-per-block option it would not be problem, if not the 
description of the option:

  LANGOPT(GPUMaxThreadsPerBlock, 32, 256, "default max threads per block for 
kernel launch bounds for HIP")

I.e. it says about the "default", so it should be perfectly legal to set a 
higher limits on a specific kernel. Should the option say it restricts the 
maximum it would be legal to apply it to functions as well.


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

https://reviews.llvm.org/D89582

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

[PATCH] D89372: [OpenCL] Remove unused extensions

[Jan Vesely via Phabricator via cfe-commits]

jvesely added a comment.

In D89372#2335027 , @Anastasia wrote:

> In D89372#2334728 , @jvesely wrote:
>
>> In D89372#2334225 , @Anastasia 
>> wrote:
>>
 
>>>
>>> Ok, so the pragma is supposed to control the pointer dereferencing which 
>>> seems like a valid case but however, it is not implemented now. Do we know 
>>> of any immediate plans from anyone to implement this? If not I would prefer 
>>> to remove the pragma for now? If someone decided to implement this 
>>> functionality later fully it is absolutely fine. Pragma will be very easy 
>>> to add. There is no need for everyone to pay the price for something that 
>>> can't be used at the moment.
>>
>> I though the current behaviour of 'you can use #pragma, but the dereferences 
>> are always legal' was intentional for backward compatibility.
>> I don't think there are programs that would set it to 'disabled' and expect 
>> compilation failure.
>
> The initial state of the pragma is disabled, so if disabling the extension 
> isn't supposed to do anything then I don't know why would anyone ever enable 
> it?
>
>> My concern is that legacy apps would set '#pragma enabled' before using 
>> char/short. such behavior would produce warning/error if with this change 
>> applied.
>
> Correct, it will compile with a warning but not fail to compile. I am willing 
> to introduce a -W option (if not present already ) to suppress that warning 
> if it helps with the clean up and backward compatibility. It might also open 
> up opportunities for a wider clean up  - removing pragma in extensions that 
> currently require pragma for no good reason.  I have written more details on 
> this in 
> https://github.com/KhronosGroup/OpenCL-Docs/pull/355#issuecomment-662679499

Adding a new option cannot be a solution as legacy applications were written 
before its introduction.
CL1.x applications need to continue to work without changes, anything else 
breaks backward compatibility.

 The same arguments also apply to `cles_khr_in64`. It's illegal to use 
 int64 types in embedded profile unless you first enable the extensions. 
 Rather than removing it, `cles_khr_2d_image_array_writes` should be added 
 to the extension list.
>>>
>>> I don't think clang currently support embedded profile. Adding extension 
>>> into the OpenCLOptions is pointless if it's not used. Do you plan to add 
>>> any functionality for it? Defining macros can be done easily outside of 
>>> clang source code or if it is preferable to do inside there is 
>>> `MacroBuilder::defineMacro`  available in the target hooks. Currently for 
>>> every extension added into `OpenCLExtensions.def` there is a macro, a 
>>> pragma and a field in `OpenCLOptions` added. This is often more than what 
>>> is necessary. Plus Khronos has very many extensions if we assume that all 
>>> of them are added in clang it will become scalability and maintanance 
>>> nightmare. Most of the extensions only add functions so if we provide a way 
>>> to add macros for those outside of clang code base it will keep the common 
>>> code clean and vendors can be more flexible in adding the extensions 
>>> without the need to modify upstream code if they need to. I see it as an 
>>> opportunity to improve common code and out of tree implementations too. It 
>>> just need a little bit of restructuring.
>>
>> My understanding is that both the macro and working #pragma directive is 
>> necessary.
>
> I don't believe this is the correct interpretation. If you look at the 
> extension spec s9.1 it says:
>
> `Every extension which affects the OpenCL language semantics, syntax or adds 
> built-in functions to the language must create a preprocessor #define that 
> matches the extension name string.  This #define would be available in the 
> language if and only if the extension is supported on a given implementation.`

This is only one part indicating the availability of extension. It's not 
describing what's necessary to observe extended behaviour. The specs state that 
the initial state of the compiler is as if `#pragma OPENCL EXTENSION all : 
disable` was issued.
This means that functions introduced by extensions cannot be present, and the 
symbol names are available for application use unless such symbols were 
previously reserved.

> It does not say that the pragma is needed, it only says that macro is needed. 
> That perfectly makes sense because the macro allows to check that the 
> extension is present to implement certain functionality conditionally.

This cited line only talks about extensions availability, not extension use. 
One example of newly introduced functions is in cl_khr_in64_base_atomics. The 
specs say:

  The optional extensions cl_khr_int64_base_atomics and 
cl_khr_int64_extended_atomics
  implement atomic operations on 64-bit signed and unsigned integers to 
locations in __global
  an

[PATCH] D89520: Don't permit array bound constant folding in OpenCL.

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

rsmith added a comment.

In D89520#2334477 , @Anastasia wrote:

>> I couldn't find any spec justification for accepting the kinds of cases
>> that D20090  accepts, so a reference to 
>> where in the OpenCL specification
>> this is permitted would be useful.
>
> Thanks for fixing this. I agree that the original change was not compliant 
> with the spec. OpenCL indeed doesn't allow constant folding for array bounds. 
> The idea of the change was to allow using expressions that are compile time 
> constant in the array bound because this doesn't result in VLA.
>
> Regarding the spec reference, I think we can refer to the section 6.5.3 
> describing variables in the `__constant` address space:
>
>   These variables  are  required  to  be  initialized  and  the  values  used 
>  to  initialize  these  variables  must  be  a compile time constant. Writing 
> to such a variable results in a compile-time error.
>
> I.e. the `__constant` address space variables are semantically similar to 
> `constexpr` in C++.

I don't see any way to read this wording that way. That talks about how the 
variables are initialized and says they're immutable, but I can't find anything 
in the OpenCL specification that says they can be used in integer constant 
expressions. The C definition of "integral constant expression" does not allow 
the use of variables:

C11 6.6/6: "An integer constant expression shall have integer type and shall 
only have operands that are integer constants, enumeration constants, character 
constants, sizeof expressions whose results are integer constants, _Alignof 
expressions, and floating constants that are the immediate operands of casts."

... and absent a modification to that, nor would OpenCL. That said, I'm happy 
to take your word for it that the OpenCL specification //intends// for such 
variable uses to be permitted in constant expressions.

>> Note that this change also affects the code generation in one test:
>> because after 'const int n = 0' we now treat 'n' as a constant
>> expression with value 0, it's now a null pointer, so '(local int *)n'
>> forms a null pointer rather than a zero pointer.
>
> I am slightly confused about this case because technically the value of 'n' 
> can be overwritten by casting away const.

That would result in undefined behavior. And if not, the same consideration 
would apply to array bounds :)

> However, I suspect this is compliant C99 behavior?

Not exactly, because C99 doesn't permit usage of variables in integer constant 
expressions; this is a consequence of the presumptive OpenCL rule that does 
permit such things. In C, the closest analogue would be something like:

  enum { e = 0 };
  int *p = (int*)0; // null pointer constant, not a bit-cast of zero to a 
pointer



> This example:
>
>   void test(void) {
> const int x = 0;
> const int* ptrx = &x;
> int* ptry = (int*)ptrx;
> *ptry = 100; 
> int *sp5 = ( int*)x;
>   }
>
> shows that the IR produced is indeed supposed to have NULL despite of the 
> fact that the value of initializing variable is not 0 at the time of the last 
> initialization.

This example has undefined behavior due to writing to a const variable. Also, 
in C it initializes `sp5` to a zero pointer, not necessarily a null pointer, 
because `x` is not an integer constant expression -- though you can't see the 
difference here because we're in an address space where they're the same. Under 
the presumptive OpenCL rule, `x` would be an integer constant expression, so 
the above example would initialize `sp5` to a null pointer instead of a zero 
pointer, just like in C++98.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89520

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

[PATCH] D89582: clang/AMDGPU: Apply workgroup related attributes to all functions

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D89582#2335574 , @yaxunl wrote:

> What if a device function is called by kernels with different work group 
> sizes, will caller's work group size override callee's work group size?

It's user error to call a function with a larger range than the caller


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

https://reviews.llvm.org/D89582

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

[PATCH] D89582: clang/AMDGPU: Apply workgroup related attributes to all functions

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

What if a device function is called by kernels with different work group sizes, 
will caller's work group size override callee's work group size?


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

https://reviews.llvm.org/D89582

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

[clang] dd4e8a5 - [docs] Fix some out-of-date / inaccurate text and missing formatting in the User's Manual.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-10-16T12:00:13-07:00
New Revision: dd4e8a54b2ba9cec78441ba537b94f4eaa5acd84

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

LOG: [docs] Fix some out-of-date / inaccurate text and missing formatting in 
the User's Manual.

Added: 


Modified: 
clang/docs/UsersManual.rst

Removed: 




diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index f313ce72d8ed..473fbb6d8d04 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -2479,8 +2479,8 @@ Differences between various standard modes
 --
 
 clang supports the -std option, which changes what language mode clang uses.
-The supported modes for C are c89, gnu89, c99, gnu99, c11, gnu11, c17, gnu17,
-c2x, gnu2x, and various aliases for those modes. If no -std option is
+The supported modes for C are c89, gnu89, c94, c99, gnu99, c11, gnu11, c17,
+gnu17, c2x, gnu2x, and various aliases for those modes. If no -std option is
 specified, clang defaults to gnu17 mode. Many C99 and C11 features are
 supported in earlier modes as a conforming extension, with a warning. Use
 ``-pedantic-errors`` to request an error if a feature from a later standard
@@ -2489,34 +2489,40 @@ revision is used in an earlier mode.
 Differences between all ``c*`` and ``gnu*`` modes:
 
 -  ``c*`` modes define "``__STRICT_ANSI__``".
--  Target-specific defines not prefixed by underscores, like "linux",
+-  Target-specific defines not prefixed by underscores, like ``linux``,
are defined in ``gnu*`` modes.
--  Trigraphs default to being off in ``gnu*`` modes; they can be enabled by
-   the -trigraphs option.
--  The parser recognizes "asm" and "typeof" as keywords in ``gnu*`` modes;
-   the variants "``__asm__``" and "``__typeof__``" are recognized in all
+-  Trigraphs default to being off in ``gnu*`` modes; they can be enabled
+   by the ``-trigraphs`` option.
+-  The parser recognizes ``asm`` and ``typeof`` as keywords in ``gnu*`` modes;
+   the variants ``__asm__`` and ``__typeof__`` are recognized in all modes.
+-  The parser recognizes ``inline`` as a keyword in ``gnu*`` mode, in
+   addition to recognizing it in the ``*99`` and later modes for which it is
+   part of the ISO C standard. The variant ``__inline__`` is recognized in all
modes.
 -  The Apple "blocks" extension is recognized by default in ``gnu*`` modes
-   on some platforms; it can be enabled in any mode with the "-fblocks"
+   on some platforms; it can be enabled in any mode with the ``-fblocks``
option.
 -  Arrays that are VLA's according to the standard, but which can be
constant folded by the frontend are treated as fixed size arrays.
This occurs for things like "int X[(1, 2)];", which is technically a
VLA. ``c*`` modes are strictly compliant and treat these as VLAs.
 
-Differences between ``*89`` and ``*99`` modes:
+Differences between ``*89`` and ``*94`` modes:
 
--  The ``*99`` modes default to implementing "inline" as specified in C99,
-   while the ``*89`` modes implement the GNU version. This can be
-   overridden for individual functions with the ``__gnu_inline__``
-   attribute.
 -  Digraphs are not recognized in c89 mode.
--  The scope of names defined inside a "for", "if", "switch", "while",
-   or "do" statement is 
diff erent. (example: "``if ((struct x {int
-   x;}*)0) {}``".)
+
+Differences between ``*94`` and ``*99`` modes:
+
+-  The ``*99`` modes default to implementing ``inline`` / ``__inline__``
+   as specified in C99, while the ``*89`` modes implement the GNU version.
+   This can be overridden for individual functions with the ``__gnu_inline__``
+   attribute.
+-  The scope of names defined inside a ``for``, ``if``, ``switch``, ``while``,
+  or ``do`` statement is 
diff erent. (example: ``if ((struct x {int x;}*)0)
+  {}``.)
 -  ``__STDC_VERSION__`` is not defined in ``*89`` modes.
--  "inline" is not recognized as a keyword in c89 mode.
--  "restrict" is not recognized as a keyword in ``*89`` modes.
+-  ``inline`` is not recognized as a keyword in ``c89`` mode.
+-  ``restrict`` is not recognized as a keyword in ``*89`` modes.
 -  Commas are allowed in integer constant expressions in ``*99`` modes.
 -  Arrays which are not lvalues are not implicitly promoted to pointers
in ``*89`` modes.
@@ -2538,9 +2544,7 @@ clang tries to be compatible with gcc as much as 
possible, but some gcc
 extensions are not implemented yet:
 
 -  clang does not support decimal floating point types (``_Decimal32`` and
-   friends) or fixed-point types (``_Fract`` and friends); nobody has
-   expressed interest in these features yet, so it's hard to say when
-   they will be implemented.
+   friends) yet.
 -  clang does not support nested functions; this is a comp

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

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

rsmith added a comment.

In D89523#2333939 , @efriedma wrote:

> Also update the documentation?  See 
> https://clang.llvm.org/docs/UsersManual.html#differences-between-various-standard-modes
>  .

Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89523

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

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

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

rsmith updated this revision to Diff 298705.
rsmith added a comment.

- Update User's Manual to more accurately describe GNU mode and


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89523

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/basic/basic.types/p10.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CodeGen/vla.c
  clang/test/Misc/warning-flags.c
  clang/test/PCH/cxx-constexpr.cpp
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  clang/test/Sema/builtin-assume.c
  clang/test/Sema/builtins.c
  clang/test/Sema/complex-int.c
  clang/test/Sema/const-eval-64.c
  clang/test/Sema/const-eval.c
  clang/test/Sema/darwin-align-cast.c
  clang/test/Sema/decl-in-prototype.c
  clang/test/Sema/gnu-flags.c
  clang/test/Sema/i-c-e.c
  clang/test/Sema/offsetof-64.c
  clang/test/Sema/struct-decl.c
  clang/test/Sema/typedef-variable-type.c
  clang/test/Sema/vla.c
  clang/test/SemaCXX/anonymous-struct.cpp
  clang/test/SemaCXX/constant-expression.cpp
  clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
  clang/test/SemaCXX/i-c-e-cxx.cpp
  clang/test/SemaObjC/gcc-cast-ext.m

Index: clang/test/SemaObjC/gcc-cast-ext.m
===
--- clang/test/SemaObjC/gcc-cast-ext.m
+++ clang/test/SemaObjC/gcc-cast-ext.m
@@ -11,7 +11,7 @@
 
 // GCC allows pointer expressions in integer constant expressions.
 struct {
-  char control[((int)(char *)2)];
+  char control[((int)(char *)2)]; // expected-warning {{extension}}
 } xx;
 
 @implementation PBXDocBookmark // expected-warning {{method definition for 'autorelease' not found}}\
Index: clang/test/SemaCXX/i-c-e-cxx.cpp
===
--- clang/test/SemaCXX/i-c-e-cxx.cpp
+++ clang/test/SemaCXX/i-c-e-cxx.cpp
@@ -80,7 +80,7 @@
 #endif
 
 int PR8836test[(__typeof(sizeof(int)))&reinterpret_castPR8836*)0)->a))];
-// expected-warning@-1 {{folded to constant array as an extension}}
+// expected-warning@-1 0-1{{C99 feature}} expected-warning@-1 {{folded to constant array as an extension}}
 // expected-note@-2 {{cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression}}
 
 const int nonconst = 1.0;
@@ -89,7 +89,7 @@
 #endif
 int arr[nonconst];
 #if __cplusplus <= 199711L
-// expected-warning@-2 {{folded to constant array as an extension}}
+// expected-warning@-2 0-1{{C99 feature}} expected-warning@-2 {{folded to constant array as an extension}}
 // expected-note@-3 {{initializer of 'nonconst' is not a constant expression}}
 #endif
 
Index: clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
===
--- clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
+++ clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
@@ -120,7 +120,7 @@
   extern "C" int strncmp(const char *, const char *, decltype(sizeof(0))) noexcept;
 
   // Check we recognized both as builtins.
-  typedef int arr[strcmp("bar", "foo") + 4 * strncmp("foo", "bar", 4)];
+  typedef int arr[strcmp("bar", "foo") + 4 * strncmp("foo", "bar", 4)]; // expected-warning {{variable length array}}
   typedef int arr[3];
 }
 
Index: clang/test/SemaCXX/constant-expression.cpp
===
--- clang/test/SemaCXX/constant-expression.cpp
+++ clang/test/SemaCXX/constant-expression.cpp
@@ -110,7 +110,7 @@
 const int recurse2 = recurse1; // expected-note {{here}}
 const int recurse1 = 1;
 int array1[recurse1]; // ok
-int array2[recurse2]; // expected-warning {{variable length array}} expected-warning {{integer constant expression}} expected-note {{initializer of 'recurse2' is not a constant expression}}
+int array2[recurse2]; // expected-warning 2{{variable length array}} expected-note {{initializer of 'recurse2' is not a constant expression}}
 
 namespace FloatConvert {
   typedef int a[(int)42.3];
Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -131,6 +131,9 @@
   typedef struct { // expected-error {{unsupported}}
 enum X {};
 int arr[&f ? 1 : 2];
+#if __cplusplus < 201103L
+// expected-warning@-2 {{folded to constant}}
+#endif
   } C; // expected-note {{by this typedef}}
 }
 
Index: clang/test/Sema/vla.c
===
--- clang/test/Sema/vla.c
+++ clang/test/Sema/vla.c
@@ -53,7 +53,7 @@
 int (*pr2044c(void))[pr2044b]; // expected-error {{variably modified type}}
 
 const int f5_ci = 1;
-void f5() { char a[][f5_ci] = {""}; } // expecte

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Perhaps a better example:

(gdb) p CallOperator->getType()->dump()
FunctionProtoType 0x1177edf0 'void (void) __attribute__((vectorcall)) const' 
const vectorcall
`-AutoType 0x1177a0d0 'void' sugar

  `-BuiltinType 0x11716dc0 'void'


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

https://reviews.llvm.org/D89559

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

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D89559#2335557 , @rjmccall wrote:

> No, if you put a calling convention on a lambda and then convert it to a 
> function pointer, you almost certainly want that CC to be honored.
>
> Is the `AttributedType` still present in `CallOperator->getType()`?

No, it is not at that point:

(gdb) p CallOperator->getType()->dump()
FunctionProtoType 0x1177a160 'void (void) __attribute__((thiscall)) const' 
const thiscall
`-AutoType 0x1177a0d0 'void' sugar

  `-BuiltinType 0x11716dc0 'void'


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

https://reviews.llvm.org/D89559

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

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

No, if you put a calling convention on a lambda and then convert it to a 
function pointer, you almost certainly want that CC to be honored.

Is the `AttributedType` still present in `CallOperator->getType()`?


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

https://reviews.llvm.org/D89559

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

[PATCH] D89582: clang/AMDGPU: Apply workgroup related attributes to all functions

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: yaxunl, rampitec.
Herald added subscribers: kerbowa, t-tye, tpr, dstuttard, nhaehnle, jvesely, 
kzhuravl.
arsenm requested review of this revision.
Herald added a subscriber: wdng.

When the default flat work group size is 256, it should also apply to
callable functions.


https://reviews.llvm.org/D89582

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/amdgpu-kernel-attrs.cu
  clang/test/CodeGenOpenCL/amdgpu-attrs.cl


Index: clang/test/CodeGenOpenCL/amdgpu-attrs.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-attrs.cl
+++ clang/test/CodeGenOpenCL/amdgpu-attrs.cl
@@ -190,5 +190,5 @@
 // CHECK-DAG: attributes 
[[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_NUM_SGPR_32_NUM_VGPR_64]] = {{.*}} 
"amdgpu-flat-work-group-size"="32,64" "amdgpu-implicitarg-num-bytes"="56" 
"amdgpu-num-sgpr"="32" "amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2"
 // CHECK-DAG: attributes 
[[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_4_NUM_SGPR_32_NUM_VGPR_64]] = 
{{.*}} "amdgpu-flat-work-group-size"="32,64" 
"amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-sgpr"="32" 
"amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2,4"
 
-// CHECK-DAG: attributes [[A_FUNCTION]] = {{.*}}
+// CHECK-DAG: attributes [[A_FUNCTION]] = {{.*}} 
"amdgpu-flat-work-group-size"="1,256"
 // CHECK-DAG: attributes [[DEFAULT_KERNEL_ATTRS]] = {{.*}} 
"amdgpu-flat-work-group-size"="1,256" "amdgpu-implicitarg-num-bytes"="56"
Index: clang/test/CodeGenCUDA/amdgpu-kernel-attrs.cu
===
--- clang/test/CodeGenCUDA/amdgpu-kernel-attrs.cu
+++ clang/test/CodeGenCUDA/amdgpu-kernel-attrs.cu
@@ -16,6 +16,10 @@
 // CHECK: define amdgpu_kernel void @_Z28flat_work_group_size_defaultv() 
[[FLAT_WORK_GROUP_SIZE_DEFAULT:#[0-9]+]]
 }
 
+__device__ void func_flat_work_group_size_default() {
+// CHECK: define void @_Z33func_flat_work_group_size_defaultv() 
[[FLAT_WORK_GROUP_SIZE_DEFAULT_FUNC:#[0-9]+]]
+}
+
 __attribute__((amdgpu_flat_work_group_size(32, 64))) // expected-no-diagnostics
 __global__ void flat_work_group_size_32_64() {
 // CHECK: define amdgpu_kernel void @_Z26flat_work_group_size_32_64v() 
[[FLAT_WORK_GROUP_SIZE_32_64:#[0-9]+]]
@@ -40,7 +44,11 @@
 // NAMD-NOT: "amdgpu-num-sgpr"
 
 // DEFAULT-DAG: attributes [[FLAT_WORK_GROUP_SIZE_DEFAULT]] = 
{{.*}}"amdgpu-flat-work-group-size"="1,256"{{.*}}"uniform-work-group-size"="true"
+// DEFAULT-DAG: attributes [[FLAT_WORK_GROUP_SIZE_DEFAULT_FUNC]] = 
{{.*}}"amdgpu-flat-work-group-size"="1,256"{{.*}}"uniform-work-group-size"="true"
+
 // MAX1024-DAG: attributes [[FLAT_WORK_GROUP_SIZE_DEFAULT]] = 
{{.*}}"amdgpu-flat-work-group-size"="1,1024"
+// MAX1024-DAG: attributes [[FLAT_WORK_GROUP_SIZE_DEFAULT_FUNC]] = 
{{.*}}"amdgpu-flat-work-group-size"="1,1024"
+
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64]] = 
{{.*}}"amdgpu-flat-work-group-size"="32,64"
 // CHECK-DAG: attributes [[WAVES_PER_EU_2]] = {{.*}}"amdgpu-waves-per-eu"="2"
 // CHECK-DAG: attributes [[NUM_SGPR_32]] = {{.*}}"amdgpu-num-sgpr"="32"
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -9031,7 +9031,7 @@
   (M.getTriple().getOS() == llvm::Triple::AMDHSA))
 F->addFnAttr("amdgpu-implicitarg-num-bytes", "56");
 
-  if (IsHIPKernel)
+  if (M.getLangOpts().HIP)
 F->addFnAttr("uniform-work-group-size", "true");
 
 
@@ -9057,7 +9057,7 @@
   F->addFnAttr("amdgpu-flat-work-group-size", AttrVal);
 } else
   assert(Max == 0 && "Max must be zero");
-  } else if (IsOpenCLKernel || IsHIPKernel) {
+  } else {
 // By default, restrict the maximum size to a value specified by
 // --gpu-max-threads-per-block=n or its default value.
 std::string AttrVal =


Index: clang/test/CodeGenOpenCL/amdgpu-attrs.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-attrs.cl
+++ clang/test/CodeGenOpenCL/amdgpu-attrs.cl
@@ -190,5 +190,5 @@
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_NUM_SGPR_32_NUM_VGPR_64]] = {{.*}} "amdgpu-flat-work-group-size"="32,64" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-sgpr"="32" "amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2"
 // CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_4_NUM_SGPR_32_NUM_VGPR_64]] = {{.*}} "amdgpu-flat-work-group-size"="32,64" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-sgpr"="32" "amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2,4"
 
-// CHECK-DAG: attributes [[A_FUNCTION]] = {{.*}}
+// CHECK-DAG: attributes [[A_FUNCTION]] = {{.*}} "amdgpu-flat-work-group-size"="1,256"
 // CHECK-DAG: attributes [[DEFAULT_KERNEL_ATTRS]] = {{.*}} "amdgpu-flat-work-group-size"="1,256" "amdgpu-implicitarg-num-bytes"="56"
Index: clang/test/CodeGenCUDA/amdgpu-kernel-attrs.cu
=

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

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

rsmith updated this revision to Diff 298702.
rsmith marked 2 inline comments as done.
rsmith added a comment.

- Update User's Manual to more accurately describe GNU mode and


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89523

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/basic/basic.types/p10.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CodeGen/vla.c
  clang/test/Misc/warning-flags.c
  clang/test/PCH/cxx-constexpr.cpp
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  clang/test/Sema/builtin-assume.c
  clang/test/Sema/builtins.c
  clang/test/Sema/complex-int.c
  clang/test/Sema/const-eval-64.c
  clang/test/Sema/const-eval.c
  clang/test/Sema/darwin-align-cast.c
  clang/test/Sema/decl-in-prototype.c
  clang/test/Sema/gnu-flags.c
  clang/test/Sema/i-c-e.c
  clang/test/Sema/offsetof-64.c
  clang/test/Sema/struct-decl.c
  clang/test/Sema/typedef-variable-type.c
  clang/test/Sema/vla.c
  clang/test/SemaCXX/anonymous-struct.cpp
  clang/test/SemaCXX/constant-expression.cpp
  clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
  clang/test/SemaCXX/i-c-e-cxx.cpp
  clang/test/SemaObjC/gcc-cast-ext.m

Index: clang/test/SemaObjC/gcc-cast-ext.m
===
--- clang/test/SemaObjC/gcc-cast-ext.m
+++ clang/test/SemaObjC/gcc-cast-ext.m
@@ -11,7 +11,7 @@
 
 // GCC allows pointer expressions in integer constant expressions.
 struct {
-  char control[((int)(char *)2)];
+  char control[((int)(char *)2)]; // expected-warning {{extension}}
 } xx;
 
 @implementation PBXDocBookmark // expected-warning {{method definition for 'autorelease' not found}}\
Index: clang/test/SemaCXX/i-c-e-cxx.cpp
===
--- clang/test/SemaCXX/i-c-e-cxx.cpp
+++ clang/test/SemaCXX/i-c-e-cxx.cpp
@@ -80,7 +80,7 @@
 #endif
 
 int PR8836test[(__typeof(sizeof(int)))&reinterpret_castPR8836*)0)->a))];
-// expected-warning@-1 {{folded to constant array as an extension}}
+// expected-warning@-1 0-1{{C99 feature}} expected-warning@-1 {{folded to constant array as an extension}}
 // expected-note@-2 {{cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression}}
 
 const int nonconst = 1.0;
@@ -89,7 +89,7 @@
 #endif
 int arr[nonconst];
 #if __cplusplus <= 199711L
-// expected-warning@-2 {{folded to constant array as an extension}}
+// expected-warning@-2 0-1{{C99 feature}} expected-warning@-2 {{folded to constant array as an extension}}
 // expected-note@-3 {{initializer of 'nonconst' is not a constant expression}}
 #endif
 
Index: clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
===
--- clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
+++ clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
@@ -120,7 +120,7 @@
   extern "C" int strncmp(const char *, const char *, decltype(sizeof(0))) noexcept;
 
   // Check we recognized both as builtins.
-  typedef int arr[strcmp("bar", "foo") + 4 * strncmp("foo", "bar", 4)];
+  typedef int arr[strcmp("bar", "foo") + 4 * strncmp("foo", "bar", 4)]; // expected-warning {{variable length array}}
   typedef int arr[3];
 }
 
Index: clang/test/SemaCXX/constant-expression.cpp
===
--- clang/test/SemaCXX/constant-expression.cpp
+++ clang/test/SemaCXX/constant-expression.cpp
@@ -110,7 +110,7 @@
 const int recurse2 = recurse1; // expected-note {{here}}
 const int recurse1 = 1;
 int array1[recurse1]; // ok
-int array2[recurse2]; // expected-warning {{variable length array}} expected-warning {{integer constant expression}} expected-note {{initializer of 'recurse2' is not a constant expression}}
+int array2[recurse2]; // expected-warning 2{{variable length array}} expected-note {{initializer of 'recurse2' is not a constant expression}}
 
 namespace FloatConvert {
   typedef int a[(int)42.3];
Index: clang/test/SemaCXX/anonymous-struct.cpp
===
--- clang/test/SemaCXX/anonymous-struct.cpp
+++ clang/test/SemaCXX/anonymous-struct.cpp
@@ -131,6 +131,9 @@
   typedef struct { // expected-error {{unsupported}}
 enum X {};
 int arr[&f ? 1 : 2];
+#if __cplusplus < 201103L
+// expected-warning@-2 {{folded to constant}}
+#endif
   } C; // expected-note {{by this typedef}}
 }
 
Index: clang/test/Sema/vla.c
===
--- clang/test/Sema/vla.c
+++ clang/test/Sema/vla.c
@@ -53,7 +53,7 @@
 int (*pr2044c(void))[pr2044b]; // expected-error {{variably modified type}}
 
 const int f5_ci = 1;
-void f5

[PATCH] D89580: SourceManager: Fix an SLocEntry memory regression introduced with FileEntryRef

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added reviewers: arphaman, Bigcheese.
Herald added a subscriber: ributzka.
dexonsmith requested review of this revision.

4dc5573acc0d2e7c59d8bac2543eb25cb4b32984 added `FileEntryRef` in order to
help enable sharing of a `FileManager` between `CompilerInstance`s.

It also added a `StringRef` with the filename on `FileInfo`. This
doubled `sizeof(FileInfo)`, bloating `sizeof(SLocEntry)`, of which we
have one for each (loaded and unloaded) file and macro expansion. This
causes a memory regression in modules builds.

Move the filename down into the `ContentCache`, which is a side data
structure for `FileInfo` that does not impact `sizeof(SLocEntry)`. Once
`FileEntryRef` is used for `ContentCache::OrigEntry` this can go away.


https://reviews.llvm.org/D89580

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -1748,7 +1748,7 @@
   // Predefined header doesn't have a valid include location in main
   // file, but any files created by it should still be skipped when
   // computing macro args expanded in the main file.
-  (FID == MainFileID && Entry.getFile().Filename == "");
+  (FID == MainFileID && Entry.getFile().getName() == "");
   if (IncludedInFID) {
 // Skip the files/macros of the #include'd file, we only care about
 // macros that lexed macro arguments from our file.
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -105,6 +105,8 @@
 ///
 /// It is possible for this to be NULL if the ContentCache encapsulates
 /// an imaginary text buffer.
+///
+/// FIXME: Turn this into a FileEntryRef and remove Filename.
 const FileEntry *OrigEntry;
 
 /// References the file which the contents were actually loaded from.
@@ -113,6 +115,11 @@
 /// with the contents of another file.
 const FileEntry *ContentsEntry;
 
+/// The filename that is used to access OrigEntry.
+///
+/// FIXME: Remove this once OrigEntry is a FileEntryRef with a stable name.
+StringRef Filename;
+
 /// A bump pointer allocated array of offsets for each source line.
 ///
 /// This is lazily computed.  This is owned by the SourceManager
@@ -244,7 +251,11 @@
   /// from. This information encodes the \#include chain that a token was
   /// expanded from. The main include file has an invalid IncludeLoc.
   ///
-  /// FileInfos contain a "ContentCache *", with the contents of the file.
+  /// FileInfo should not grow larger than ExpansionInfo. Doing so will
+  /// cause memory to bloat in compilations with many unloaded macro
+  /// expansions, since the two data structurs are stored in a union in
+  /// SLocEntry. Extra fields should instead go in "ContentCache *", which
+  /// stores file contents and other bits on the side.
   ///
   class FileInfo {
 friend class clang::SourceManager;
@@ -269,10 +280,6 @@
 llvm::PointerIntPair
 ContentAndKind;
 
-/// The filename that is used to access the file entry represented by the
-/// content cache.
-StringRef Filename;
-
   public:
 /// Return a FileInfo object.
 static FileInfo get(SourceLocation IL, const ContentCache &Con,
@@ -283,7 +290,7 @@
   X.HasLineDirectives = false;
   X.ContentAndKind.setPointer(&Con);
   X.ContentAndKind.setInt(FileCharacter);
-  X.Filename = Filename;
+  const_cast(Con).Filename = Filename;
   return X;
 }
 
@@ -311,7 +318,7 @@
 
 /// Returns the name of the file that was used when the file was loaded 
from
 /// the underlying file system.
-StringRef getName() const { return Filename; }
+StringRef getName() const { return getContentCache().Filename; }
   };
 
   /// Each ExpansionInfo encodes the expansion location - where
@@ -432,6 +439,13 @@
 }
   };
 
+  // Assert that the \c FileInfo objects are no bigger than \c ExpansionInfo
+  // objects. This controls the size of \c SLocEntry, of which we have one for
+  // each macro expansion. The number of (unloaded) macro expansions can be
+  // very large. Any other fields needed in FileInfo should go in ContentCache.
+  static_assert(sizeof(FileInfo) <= sizeof(ExpansionInfo),
+"FileInfo must be no larger than ExpansionInfo.");
+
   /// This is a discriminated union of FileInfo and ExpansionInfo.
   ///
   /// SourceManager keeps an array of these objects, and they are uniquely


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -1748,7 +1748,7 @@
   

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

[David Goldman via Phabricator via cfe-commits]

dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/Selection.cpp:621-623
+  if (auto AT = TL->getAs())
+S = AT.getModifiedLoc()
+.getSourceRange(); // should we just return false?

Let me know if you think it would be better to return false here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89579

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

[PATCH] D89579: [clangd][ObjC] Support nullability annotations

[David Goldman via Phabricator via cfe-commits]

dgoldman created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman.
Herald added a project: clang.
dgoldman requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Nullability annotations are implmented using attributes; previusly
clangd would skip over AttributedTypeLoc since their location
points to the attribute instead of the modified type.

Also add quite a few test cases for this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89579

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -356,6 +356,22 @@
 )cpp",
   "DeclRefExpr"},
 
+  // Objective-C nullability attributes.
+  {
+  R"cpp(
+@interface I{}
+@property(nullable) [[^I]] *x;
+@end
+  )cpp",
+  "ObjCInterfaceTypeLoc"},
+  {
+  R"cpp(
+@interface I{}
+- (void)doSomething:(nonnull [[i^d]])argument;
+@end
+  )cpp",
+  "TypedefTypeLoc"},
+
   // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
   // Need to traverse the contents of the OpaqueValueExpr to the POE,
   // and ensure we traverse only the syntactic form of the PseudoObjectExpr.
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1991,6 +1991,91 @@
 HI.NamespaceScope = "ObjC::"; // FIXME: fix it
 HI.Definition = "char data";
   }},
+  {
+  R"cpp(
+  @interface MYObject
+  @end
+  @interface Interface
+  @property(retain) [[MYOb^ject]] *x;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "MYObject";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "";
+HI.Definition = "@interface MYObject\n@end";
+  }},
+  {
+  // Should work even with nullability attribute.
+  R"cpp(
+  @interface MYObject
+  @end
+  @interface Interface
+  @property(nonnull) [[MYOb^ject]] *x;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "MYObject";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "";
+HI.Definition = "@interface MYObject\n@end";
+  }},
+  {
+  R"cpp(
+  @interface MYObject
+  @end
+  @interface Interface
+  - (void)doWith:([[MYOb^ject]] *)object;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "MYObject";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "";
+HI.Definition = "@interface MYObject\n@end";
+  }},
+  {
+  R"cpp(
+  @interface MYObject
+  @end
+  @interface Interface
+  - (void)doWith:(nullable [[MYOb^ject]] *)object;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "MYObject";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "";
+HI.Definition = "@interface MYObject\n@end";
+  }},
+  {
+  R"cpp(
+  @class ForwardDeclared;
+  @interface Interface
+  @property(retain) [[Forward^Declared]] *x;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "ForwardDeclared";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "";
+HI.Definition = "@class ForwardDeclared;";
+  }},
+  {
+  R"cpp(
+  @protocol Fooey
+  - (void)foo;
+  @end
+  @interface Interface
+  @property(retain) id<[[Fo^oey]]> fooey;
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Fooey";
+HI.Kind = index::SymbolKind::Protocol;
+HI.NamespaceScope = "";
+HI.Definition = "@protocol Fooey\n@end";
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -820,6 +820,26 @@
   EXPECT_DECLS("Obj

[PATCH] D85788: [Clang test] Update to allow passing extra default clang arguments in use_clang

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis accepted this revision.
eugenis 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/D85788/new/

https://reviews.llvm.org/D85788

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

[PATCH] D63473: Support -fclang-abi-compat=8.0 to keep old ABI behavior

[Nico Weber via Phabricator via cfe-commits]

thakis resigned from this revision.
thakis added a comment.

hans and compnerd got this, no need for so many reviewers :)


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

https://reviews.llvm.org/D63473

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

[PATCH] D87279: [clang] Fix handling of physical registers in inline assembly operands.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers requested changes to this revision.
nickdesaulniers added a comment.
This revision now requires changes to proceed.

Thanks for updating the patch.

Please add a test case for the earlyclobber case. In particular, if none of the 
in-tree tests failed for the initial version of this patch that landed, you 
should add one that would have.

I should be able to `git checkout c78da037783bda0f27f4d82060149166e6f0c796`, 
observe the test case fail, then `arc patch D87279` and observe the test case 
pass.

The current version of this patch,  Diff 298624, passes the previously failing 
Linux kernel builds.


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

https://reviews.llvm.org/D87279

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

[PATCH] D89573: [Driver] Incorporate -mfloat-abi in the computed triple on ARM

[John Brawn via Phabricator via cfe-commits]

john.brawn created this revision.
john.brawn added reviewers: efriedma, vhscampos, chill.
Herald added subscribers: dexonsmith, steven_wu, hiraditya, kristof.beyls.
Herald added a project: clang.
john.brawn requested review of this revision.

LLVM assumes that when it creates a call to a C library function it can use the 
C calling convention. On ARM the effective calling convention is determined 
from the target triple, however using -mfloat-abi=hard on ARM means that calls 
to (and definitions of) C library functions use the arm_aapcs_vfpcc calling 
convention which can result in a mismatch.

Fix this by incorporating -mfloat-abi into the target triple, similar to how 
-mbig-endian and -march/-mcpu are. This only works for EABI targets and not 
Android or iOS, but there the float abi is fixed.

Fixes PR45524.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89573

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/arm-float-abi-lto.c
  clang/test/Driver/arm-triple.c
  clang/test/Driver/windows-thumbv7em.cpp

Index: clang/test/Driver/windows-thumbv7em.cpp
===
--- clang/test/Driver/windows-thumbv7em.cpp
+++ clang/test/Driver/windows-thumbv7em.cpp
@@ -1,8 +1,8 @@
 // RUN: %clang -target thumb-none-windows-eabi-coff -mcpu=cortex-m7 -### -c %s 2>&1 \
 // RUN: | FileCheck %s --check-prefix CHECK-V7
-// CHECK-V7-NOT: error: the target architecture 'thumbv7em' is not supported by the target 'thumbv7em-none-windows-eabi'
+// CHECK-V7-NOT: error: the target architecture 'thumbv7em' is not supported by the target 'thumbv7em-none-windows-eabihf'
 
 // RUN: %clang -target thumb-none-windows-eabi-coff -mcpu=cortex-m1 -### -c %s 2>&1 \
 // RUN: | FileCheck %s --check-prefix CHECK-V6
-// CHECK-V6: error: the target architecture 'thumbv6m' is not supported by the target 'thumbv6m-none-windows-eabi'
+// CHECK-V6: error: the target architecture 'thumbv6m' is not supported by the target 'thumbv6m-none-windows-eabihf'
 
Index: clang/test/Driver/arm-triple.c
===
--- /dev/null
+++ clang/test/Driver/arm-triple.c
@@ -0,0 +1,48 @@
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=arm-none-eabi \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DEFAULT
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=armeb-none-eabi -mlittle-endian \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DEFAULT
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=arm-none-eabihf -march=armv4t -mfloat-abi=softfp \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DEFAULT
+// CHECK-DEFAULT: armv4t-none-unknown-eabi
+
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=armeb-none-eabi \
+// RUN:   | FileCheck %s --check-prefix=CHECK-EB
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=arm-none-eabi -mbig-endian \
+// RUN:   | FileCheck %s --check-prefix=CHECK-EB
+// CHECK-EB: armebv4t-none-unknown-eabi
+
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=arm-none-eabihf -march=armv4t \
+// RUN:   | FileCheck %s --check-prefix=CHECK-HF
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=arm-none-eabi -mfloat-abi=hard \
+// RUN:   | FileCheck %s --check-prefix=CHECK-HF
+// CHECK-HF: armv4t-none-unknown-eabihf
+
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=armeb-none-eabihf -march=armv4t \
+// RUN:   | FileCheck %s --check-prefix=CHECK-EB-HF
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=armeb-none-eabi -mfloat-abi=hard \
+// RUN:   | FileCheck %s --check-prefix=CHECK-EB-HF
+// RUN: %clang -print-effective-triple -march=armv4t \
+// RUN:   --target=arm-none-eabihf -mbig-endian \
+// RUN:   | FileCheck %s --check-prefix=CHECK-EB-HF
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=arm-none-eabi -mbig-endian -mfloat-abi=hard \
+// RUN:   | FileCheck %s --check-prefix=CHECK-EB-HF
+// CHECK-EB-HF: armebv4t-none-unknown-eabihf
+
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=arm-none-eabi -march=armv8m.main -mbig-endian -mfloat-abi=hard \
+// RUN:   | FileCheck %s --check-prefix=CHECK-V8M-EB-HF
+// RUN: %clang -print-effective-triple \
+// RUN:   --target=arm-none-eabi -mcpu=cortex-m33 -mbig-endian -mfloat-abi=hard \
+// RUN:   | FileCheck %s --check-prefix=CHECK-V8M-EB-HF
+// CHECK-V8M-EB-HF: thumbebv8m.main-none-unknown-eabihf
Index: clang/test/Driver/arm-float-abi-lto.c
===
--- /dev/null
+++ clang/test/Driver/arm-float-abi-lto.c
@@ -0,0 +1,63 @@
+// REQUIRES: arm-registered-target
+
+// RUN: %clang --target=arm-none-eabi -mcpu=cortex-m33 -mfloat-abi=hard -O1 %s -S -o - -emit-llvm -DCALL_LIB -DDEFINE_LIB | FileCheck %s
+
+// RUN: %clang --target=arm-none-eabi -mcpu=cortex-m33 -mfloat-abi=hard -O1 %s -flto=full -c -o %t.call_full.bc -DCALL_LIB
+// RUN: %clang --target=arm-none-e

[PATCH] D89571: [clangd] Add textDocument/ast extension method to dump the AST

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: adamcz.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, mgorny.
Herald added a project: clang.
sammccall requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

This is a mass-market version of the "dump AST" tweak we have behind
-hidden-features.
I think in this friendlier form it'll be useful for people outside clang
developers, which would justify making it a real feature.
It could be useful as a step towards lightweight clang-AST tooling in clangd
itself (like matcher-based search).

Advantages over the tweak:

- simplified information makes it more accessible, likely somewhat useful 
without learning too much clang internals
- can be shown in a tree view
- structured information gives some options for presentation (e.g. icon + two 
text colors + tooltip in vscode)
- clickable nodes jump to the corresponding code

Disadvantages:

- a bunch of code to handle different node types
- likely missing some important info vs dump-ast due to brevity/oversight
- may end up chasing/maintaining support for the long tail of nodes

Demo with VSCode support: https://imgur.com/a/6gKfyIV


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89571

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/unittests/CMakeLists.txt

Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -51,6 +51,7 @@
   DexTests.cpp
   DiagnosticsTests.cpp
   DraftStoreTests.cpp
+  DumpASTTests.cpp
   ExpectedTypeTest.cpp
   FileDistanceTests.cpp
   FileIndexTests.cpp
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -5,6 +5,7 @@
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:"capabilities": {
+# CHECK-NEXT:  "astProvider": true,
 # CHECK-NEXT:  "codeActionProvider": true,
 # CHECK-NEXT:  "completionProvider": {
 # CHECK-NEXT:"allCommitCharacters": [
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1576,6 +1576,45 @@
 };
 llvm::json::Value toJSON(const FoldingRange &Range);
 
+/// Payload for textDocument/ast request.
+/// This request is a clangd extension.
+struct ASTParams {
+  /// The text document.
+  TextDocumentIdentifier textDocument;
+
+  /// The position of the node to be dumped.
+  /// The highest-level node that entirely contains the range will be returned.
+  Range range;
+};
+bool fromJSON(const llvm::json::Value &, ASTParams &, llvm::json::Path);
+
+/// Simplified description of a clang AST node.
+/// This is clangd's internal representation of C++ code.
+struct ASTNode {
+  /// The general kind of node, such as "expression"
+  /// Corresponds to the base AST node type such as Expr.
+  std::string role;
+  /// The specific kind of node this is, such as "BinaryOperator".
+  /// This is usually a concrete node class (with Expr etc suffix dropped).
+  /// When there's no hierarchy (e.g. TemplateName), the variant (NameKind).
+  std::string kind;
+  /// Brief additional information, such as "||" for the particular operator.
+  /// The information included depends on the node kind, and may be empty.
+  std::string detail;
+  /// A one-line dump of detailed information about the node.
+  /// This includes role/kind/description information, but is rather cryptic.
+  /// It is similar to the output from `clang -Xclang -ast-dump`.
+  /// May be empty for certain types of nodes.
+  std::string arcana;
+  /// The range of the original source file covered by this node.
+  /// May be missing for implicit nodes, or those created by macro expansion.
+  llvm::Optional range;
+  /// Nodes nested within this one, such as the operands of a BinaryOperator.
+  std::vector children;
+};
+llvm::json::Value toJSON(const ASTNode &);
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const ASTNode &);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1300,5 +1300,41 @@
   return Result;
 }
 
+bool fromJSON(const llvm::json::Value &Params, AS

[clang] c4d10e7 - [AMDGPU][HIP] Switch default DWARF version to 5

[Scott Linder via cfe-commits]

Author: Scott Linder
Date: 2020-10-16T17:53:27Z
New Revision: c4d10e7e9bb47b77fad43d8ddcfa328298f36c88

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

LOG: [AMDGPU][HIP] Switch default DWARF version to 5

Another attempt at this, see D59008 for previous attempt.

Reviewed By: kzhuravl, t-tye

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AMDGPU.h
clang/lib/Driver/ToolChains/HIP.h
clang/test/Driver/amdgpu-toolchain.c
clang/test/Driver/hip-toolchain-dwarf.hip

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.h 
b/clang/lib/Driver/ToolChains/AMDGPU.h
index 55ef6e01967e..f5448b76aee5 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.h
+++ b/clang/lib/Driver/ToolChains/AMDGPU.h
@@ -60,7 +60,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public 
Generic_ELF {
 public:
   AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
-  unsigned GetDefaultDwarfVersion() const override { return 4; }
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   bool IsMathErrnoDefault() const override { return false; }
 

diff  --git a/clang/lib/Driver/ToolChains/HIP.h 
b/clang/lib/Driver/ToolChains/HIP.h
index 5e2be7138579..ff58c5451b0b 100644
--- a/clang/lib/Driver/ToolChains/HIP.h
+++ b/clang/lib/Driver/ToolChains/HIP.h
@@ -99,7 +99,7 @@ class LLVM_LIBRARY_VISIBILITY HIPToolChain final : public 
ROCMToolChain {
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList &Args) const override;
 
-  unsigned GetDefaultDwarfVersion() const override { return 4; }
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
 
   const ToolChain &HostTC;
 

diff  --git a/clang/test/Driver/amdgpu-toolchain.c 
b/clang/test/Driver/amdgpu-toolchain.c
index ac558e0e26eb..cb92744eee6a 100644
--- a/clang/test/Driver/amdgpu-toolchain.c
+++ b/clang/test/Driver/amdgpu-toolchain.c
@@ -8,7 +8,7 @@
 // AS_LINK: clang{{.*}} "-cc1as"
 // AS_LINK: ld.lld{{.*}} "-shared"
 
-// DWARF_VER: "-dwarf-version=4"
+// DWARF_VER: "-dwarf-version=5"
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s

diff  --git a/clang/test/Driver/hip-toolchain-dwarf.hip 
b/clang/test/Driver/hip-toolchain-dwarf.hip
index 44d66fe52e04..c853d5cf07cf 100644
--- a/clang/test/Driver/hip-toolchain-dwarf.hip
+++ b/clang/test/Driver/hip-toolchain-dwarf.hip
@@ -6,4 +6,4 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 %s \
 // RUN:   -Xarch_gfx803 -g 2>&1 | FileCheck %s -check-prefix=DWARF_VER
 
-// DWARF_VER: "-dwarf-version=4"
+// DWARF_VER: "-dwarf-version=5"



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

[PATCH] D89484: [AMDGPU][HIP] Switch default DWARF version to 5

[Scott Linder 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 rGc4d10e7e9bb4: [AMDGPU][HIP] Switch default DWARF version to 
5 (authored by scott.linder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89484

Files:
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.h
  clang/test/Driver/amdgpu-toolchain.c
  clang/test/Driver/hip-toolchain-dwarf.hip


Index: clang/test/Driver/hip-toolchain-dwarf.hip
===
--- clang/test/Driver/hip-toolchain-dwarf.hip
+++ clang/test/Driver/hip-toolchain-dwarf.hip
@@ -6,4 +6,4 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 %s \
 // RUN:   -Xarch_gfx803 -g 2>&1 | FileCheck %s -check-prefix=DWARF_VER
 
-// DWARF_VER: "-dwarf-version=4"
+// DWARF_VER: "-dwarf-version=5"
Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -8,7 +8,7 @@
 // AS_LINK: clang{{.*}} "-cc1as"
 // AS_LINK: ld.lld{{.*}} "-shared"
 
-// DWARF_VER: "-dwarf-version=4"
+// DWARF_VER: "-dwarf-version=5"
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -99,7 +99,7 @@
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList &Args) const override;
 
-  unsigned GetDefaultDwarfVersion() const override { return 4; }
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
 
   const ToolChain &HostTC;
 
Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -60,7 +60,7 @@
 public:
   AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
-  unsigned GetDefaultDwarfVersion() const override { return 4; }
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   bool IsMathErrnoDefault() const override { return false; }
 


Index: clang/test/Driver/hip-toolchain-dwarf.hip
===
--- clang/test/Driver/hip-toolchain-dwarf.hip
+++ clang/test/Driver/hip-toolchain-dwarf.hip
@@ -6,4 +6,4 @@
 // RUN:   -x hip --cuda-gpu-arch=gfx803 %s \
 // RUN:   -Xarch_gfx803 -g 2>&1 | FileCheck %s -check-prefix=DWARF_VER
 
-// DWARF_VER: "-dwarf-version=4"
+// DWARF_VER: "-dwarf-version=5"
Index: clang/test/Driver/amdgpu-toolchain.c
===
--- clang/test/Driver/amdgpu-toolchain.c
+++ clang/test/Driver/amdgpu-toolchain.c
@@ -8,7 +8,7 @@
 // AS_LINK: clang{{.*}} "-cc1as"
 // AS_LINK: ld.lld{{.*}} "-shared"
 
-// DWARF_VER: "-dwarf-version=4"
+// DWARF_VER: "-dwarf-version=5"
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa -mcpu=gfx906 -nogpulib \
 // RUN:   -flto %s 2>&1 | FileCheck -check-prefix=LTO %s
Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -99,7 +99,7 @@
   computeMSVCVersion(const Driver *D,
  const llvm::opt::ArgList &Args) const override;
 
-  unsigned GetDefaultDwarfVersion() const override { return 4; }
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
 
   const ToolChain &HostTC;
 
Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -60,7 +60,7 @@
 public:
   AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
-  unsigned GetDefaultDwarfVersion() const override { return 4; }
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   bool IsMathErrnoDefault() const override { return false; }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89487: [AMDGPU] gfx1032 target

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec marked 3 inline comments as done.
rampitec added inline comments.



Comment at: llvm/lib/Support/TargetParser.cpp:66
 // Don't bother listing the implicitly true features
-constexpr GPUInfo AMDGCNGPUs[43] = {
+constexpr GPUInfo AMDGCNGPUs[44] = {
   // Name CanonicalKindFeatures

foad wrote:
> Use `[]` so we don't have to keep updating the number?
D89568


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89487

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

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 298666.
erichkeane marked an inline comment as done.
erichkeane added a comment.

Calc->get.


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

https://reviews.llvm.org/D89559

Files:
  clang/lib/Sema/SemaLambda.cpp
  clang/test/SemaCXX/lambda-conversion-op-cc.cpp

Index: clang/test/SemaCXX/lambda-conversion-op-cc.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-conversion-op-cc.cpp
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-linux-pc %s -verify -DBAD_CONVERSION
+// RUN: %clang_cc1 -fsyntax-only -triple i386-windows-pc %s -verify -DBAD_CONVERSION -DWIN32
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-linux-pc %s -ast-dump | FileCheck %s --check-prefixes=CHECK,LIN64,NODEF
+// RUN: %clang_cc1 -fsyntax-only -triple i386-windows-pc %s -ast-dump -DWIN32 | FileCheck %s --check-prefixes=CHECK,WIN32,NODEF
+
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-linux-pc -fdefault-calling-conv=vectorcall %s -verify -DBAD_VEC_CONVERS
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-linux-pc -fdefault-calling-conv=vectorcall %s -ast-dump | FileCheck %s --check-prefixes=CHECK,VECTDEF
+
+void useage() {
+  auto normal = [](int, float, double){}; // #1
+  auto vectorcall = [](int, float, double) __attribute__((vectorcall)) {}; // #2
+#ifdef WIN32
+  auto thiscall = [](int, float, double) __attribute__((thiscall)) {}; // #3
+#endif // WIN32
+  auto cdecl = [](int, float, double) __attribute__((cdecl)) {};
+
+// CHECK: VarDecl {{.*}} normal '
+// CHECK: LambdaExpr
+// WIN32: CXXMethodDecl {{.*}} operator() 'void (int, float, double) __attribute__((thiscall)) const'
+// LIN64: CXXMethodDecl {{.*}} operator() 'void (int, float, double) const'
+// VECTDEF: CXXMethodDecl {{.*}} operator() 'void (int, float, double) const'
+// NODEF: CXXConversionDecl {{.*}} operator void (*)(int, float, double) 'void
+// NODEF: CXXMethodDecl {{.*}} __invoke 'void (int, float, double)' static inline
+// VECTDEF: CXXConversionDecl {{.*}} operator void (*)(int, float, double) __attribute__((vectorcall)) 'void
+// VECTDEF: CXXMethodDecl {{.*}} __invoke 'void (int, float, double) __attribute__((vectorcall))' static inline
+
+// CHECK: VarDecl {{.*}} vectorcall '
+// CHECK: LambdaExpr
+// CHECK: CXXMethodDecl {{.*}} operator() 'void (int, float, double) __attribute__((vectorcall)) const'
+// CHECK: CXXConversionDecl {{.*}} operator void (*)(int, float, double) __attribute__((vectorcall)) 'void
+// CHECK: CXXMethodDecl {{.*}} __invoke 'void (int, float, double) __attribute__((vectorcall))' static inline
+
+// WIN32: VarDecl {{.*}} thiscall '
+// WIN32: LambdaExpr
+// WIN32: CXXMethodDecl {{.*}} operator() 'void (int, float, double) __attribute__((thiscall)) const'
+// WIN32: CXXConversionDecl {{.*}} operator void (*)(int, float, double) 'void
+// WIN32: CXXMethodDecl {{.*}} __invoke 'void (int, float, double)' static inline
+
+// CHECK: VarDecl {{.*}} cdecl '
+// CHECK: LambdaExpr
+// CHECK: CXXMethodDecl {{.*}} operator() 'void (int, float, double) const'
+// NODEF: CXXConversionDecl {{.*}} operator void (*)(int, float, double) 'void
+// NODEF: CXXMethodDecl {{.*}} __invoke 'void (int, float, double)' static inline
+// VECTDEF: CXXConversionDecl {{.*}} operator void (*)(int, float, double) __attribute__((vectorcall)) 'void
+// VECTDEF: CXXMethodDecl {{.*}} __invoke 'void (int, float, double) __attribute__((vectorcall))' static inline
+
+#ifdef BAD_CONVERSION
+  // expected-error-re@+2 {{no viable conversion from {{.*}} to 'void (*)(int, float, double) __attribute__((vectorcall))}}
+  // expected-note@#1 {{candidate function}}
+  void (*__attribute__((vectorcall)) normal_ptr2)(int, float, double) = normal;
+  // expected-error-re@+2 {{no viable conversion from {{.*}} to 'void (*)(int, float, double)}}
+  // expected-note@#2 {{candidate function}}
+  void (*vectorcall_ptr2)(int, float, double) = vectorcall;
+#ifdef WIN32
+  // expected-error-re@+2 {{no viable conversion from {{.*}} to 'void (*)(int, float, double) __attribute__((thiscall))}}
+  // expected-note@#3 {{candidate function}}
+  void (*__attribute__((thiscall)) thiscall_ptr2)(int, float, double) = thiscall;
+#endif // WIN32
+#endif // BAD_CONVERSION
+
+#ifdef BAD_VEC_CONVERS
+  void (*__attribute__((vectorcall)) normal_ptr2)(int, float, double) = normal;
+  void (* normal_ptr3)(int, float, double) = normal;
+  // expected-error-re@+2 {{no viable conversion from {{.*}} to 'void (*)(int, float, double) __attribute__((regcall))}}
+  // expected-note@#1 {{candidate function}}
+  void (*__attribute__((regcall)) normalptr4)(int, float, double) = normal;
+  void (*__attribute__((vectorcall)) vectorcall_ptr2)(int, float, double) = vectorcall;
+  void (* vectorcall_ptr3)(int, float, double) = vectorcall;
+#endif // BAD_VEC_CONVERS
+
+  // Required to force emission of the invoker.
+  void (*normal_ptr)(int, float, double) = normal;
+  void (*__attribute__((v

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaLambda.cpp:1278
+  if (CallOpCC == DefaultMember)
+return DefaultFree;
+  return CallOpCC;

rjmccall wrote:
> ...I made this comment in my first review, but Phabricator threw it away.
> 
> The attributes let you explicitly request the default method CC, right?  I 
> think you need to check for an explicit attribute rather than just checking 
> CC identity.  There should be an AttributedType in the sugar.
They do, but I can't seem to find a way to find them.  The calling convention 
is already merged into the functiontype by the time we get here, the 
AttributedType isn't accessible.

So it seems we don't distinguish between "modified by attribute", 
"modified-default by command line", and "modified-default by TargetInfo."

That said, I somewhat think this is the right thing to do anyway.  If you're on 
a platform where the default call convention is different between a 
free-function and member-function, I'd think that this is what you MEAN...


Repository:
  rC Clang

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

https://reviews.llvm.org/D89559

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

[PATCH] D89487: [AMDGPU] gfx1032 target

[Tony Tye via Phabricator via cfe-commits]

t-tye added inline comments.



Comment at: llvm/docs/AMDGPUUsage.rst:280

  names.
+ ``gfx1032`` ``amdgcn``   dGPU  - xnack   
*TBA*
+  [off]

foad wrote:
> xnack looks like a mistake here?
gfx1032 does not support XNACK. It should have the same target features as 
gfx1032.



Comment at: llvm/docs/AMDGPUUsage.rst:835
+ ``EF_AMDGPU_MACH_AMDGCN_GFX1032`` 0x038  ``gfx1032``
  *reserved*0x038  Reserved.
  *reserved*0x039  Reserved.

Delete as the line above is using this reserved number.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89487

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

[PATCH] D89487: [AMDGPU] gfx1032 target

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec marked 3 inline comments as done.
rampitec added inline comments.



Comment at: llvm/docs/AMDGPUUsage.rst:280

  names.
+ ``gfx1032`` ``amdgcn``   dGPU  - xnack   
*TBA*
+  [off]

t-tye wrote:
> foad wrote:
> > xnack looks like a mistake here?
> gfx1032 does not support XNACK. It should have the same target features as 
> gfx1032.
D89565



Comment at: llvm/docs/AMDGPUUsage.rst:835
+ ``EF_AMDGPU_MACH_AMDGCN_GFX1032`` 0x038  ``gfx1032``
  *reserved*0x038  Reserved.
  *reserved*0x039  Reserved.

t-tye wrote:
> Delete as the line above is using this reserved number.
D89565


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89487

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

[PATCH] D89464: Revert "[clang-format] Fix AlignConsecutive on PP blocks"

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay 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/D89464/new/

https://reviews.llvm.org/D89464

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

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaLambda.cpp:1268
+calcLambdaConversionFunctionCallConv(Sema &S,
+ const FunctionProtoType *CallOpProto) {
+  CallingConv DefaultFree = S.Context.getDefaultCallingConvention(

rjmccall wrote:
> I don't think we use `calc` as a prefix anywhere else in the compiler.  Maybe 
> `get`?
WFM!


Repository:
  rC Clang

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

https://reviews.llvm.org/D89559

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

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaLambda.cpp:1278
+  if (CallOpCC == DefaultMember)
+return DefaultFree;
+  return CallOpCC;

...I made this comment in my first review, but Phabricator threw it away.

The attributes let you explicitly request the default method CC, right?  I 
think you need to check for an explicit attribute rather than just checking CC 
identity.  There should be an AttributedType in the sugar.


Repository:
  rC Clang

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

https://reviews.llvm.org/D89559

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

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I agree that it seems reasonable to preserve an explicit CC from the lambda on 
the converted function pointer.




Comment at: clang/lib/Sema/SemaLambda.cpp:1268
+calcLambdaConversionFunctionCallConv(Sema &S,
+ const FunctionProtoType *CallOpProto) {
+  CallingConv DefaultFree = S.Context.getDefaultCallingConvention(

I don't think we use `calc` as a prefix anywhere else in the compiler.  Maybe 
`get`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D89559

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

[PATCH] D89372: [OpenCL] Remove unused extensions

[Marco Antognini via Phabricator via cfe-commits]

mantognini updated this revision to Diff 298660.
mantognini added a comment.

Keep cl_khr_byte_addressable_store and cles_khr_int64 out of this PR. Add more 
details on the extensions being removed in the commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89372

Files:
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/test/Misc/amdgcn.languageOptsOpenCL.cl
  clang/test/Misc/nvptx.languageOptsOpenCL.cl
  clang/test/Misc/r600.languageOptsOpenCL.cl
  clang/test/SemaOpenCL/extension-version.cl

Index: clang/test/SemaOpenCL/extension-version.cl
===
--- clang/test/SemaOpenCL/extension-version.cl
+++ clang/test/SemaOpenCL/extension-version.cl
@@ -34,16 +34,6 @@
 #endif
 #pragma OPENCL EXTENSION cl_khr_int64_extended_atomics: enable
 
-#ifndef cl_khr_gl_sharing
-#error "Missing cl_khr_gl_sharing define"
-#endif
-#pragma OPENCL EXTENSION cl_khr_gl_sharing: enable
-
-#ifndef cl_khr_icd
-#error "Missing cl_khr_icd define"
-#endif
-#pragma OPENCL EXTENSION cl_khr_icd: enable
-
 // Core features in CL 1.1
 
 #ifndef cl_khr_byte_addressable_store
@@ -86,15 +76,6 @@
 // expected-warning@-2{{OpenCL extension 'cl_khr_local_int32_extended_atomics' is core feature or supported optional core feature - ignoring}}
 #endif
 
-#if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ < 110)
-// Deprecated abvoe 1.0
-#ifndef cl_khr_select_fprounding_mode
-#error "Missing cl_khr_select_fp_rounding_mode define"
-#endif
-#pragma OPENCL EXTENSION cl_khr_select_fprounding_mode: enable
-#endif
-
-
 // Core feature in CL 1.2
 #ifndef cl_khr_fp64
 #error "Missing cl_khr_fp64 define"
@@ -113,24 +94,6 @@
 // expected-warning@-2{{OpenCL extension 'cl_khr_3d_image_writes' is core feature or supported optional core feature - ignoring}}
 #endif
 
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 110)
-#ifndef cl_khr_gl_event
-#error "Missing cl_khr_gl_event define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_gl_event' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_gl_event : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 110)
-#ifndef cl_khr_d3d10_sharing
-#error "Missing cl_khr_d3d10_sharing define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_d3d10_sharing' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_d3d10_sharing : enable
-
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 110)
 #ifndef cles_khr_int64
 #error "Missing cles_khr_int64 define"
@@ -140,60 +103,6 @@
 #endif
 #pragma OPENCL EXTENSION cles_khr_int64 : enable
 
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_context_abort
-#error "Missing cl_context_abort define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_context_abort' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_context_abort : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_d3d11_sharing
-#error "Missing cl_khr_d3d11_sharing define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_d3d11_sharing' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_d3d11_sharing : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_dx9_media_sharing
-#error "Missing cl_khr_dx9_media_sharing define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_dx9_media_sharing' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_dx9_media_sharing : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_image2d_from_buffer
-#error "Missing cl_khr_image2d_from_buffer define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_image2d_from_buffer' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_image2d_from_buffer : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_initialize_memory
-#error "Missing cl_khr_initialize_memory define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_initialize_memory' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_initialize_memory : enable
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
-#ifndef cl_khr_gl_depth_images
-#error "Missing cl_khr_gl_depth_images define"
-#endif
-#else
-// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_gl_depth_images' - ignoring}}
-#endif
-#pragma OPENCL EXTENSION cl_khr_gl_depth_images : enable
-
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 120)
 #ifndef cl_khr_gl_msaa_sharing
 #error "Missing cl_khr_gl_msaa_sharing define"
@@ -203,33 +112,6 @@
 #endif
 #pragma OPENCL EXTENSION cl_khr_gl_msaa_sharing : e

[PATCH] D89372: [OpenCL] Remove unused extensions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D89372#2334728 , @jvesely wrote:

> In D89372#2334225 , @Anastasia wrote:
>
>>> 
>>
>> Ok, so the pragma is supposed to control the pointer dereferencing which 
>> seems like a valid case but however, it is not implemented now. Do we know 
>> of any immediate plans from anyone to implement this? If not I would prefer 
>> to remove the pragma for now? If someone decided to implement this 
>> functionality later fully it is absolutely fine. Pragma will be very easy to 
>> add. There is no need for everyone to pay the price for something that can't 
>> be used at the moment.
>
> I though the current behaviour of 'you can use #pragma, but the dereferences 
> are always legal' was intentional for backward compatibility.
> I don't think there are programs that would set it to 'disabled' and expect 
> compilation failure.

The initial state of the pragma is disabled, so if disabling the extension 
isn't supposed to do anything then I don't know why would anyone ever enable it?

> My concern is that legacy apps would set '#pragma enabled' before using 
> char/short. such behavior would produce warning/error if with this change 
> applied.

Correct, it will compile with a warning but not fail to compile. I am willing 
to introduce a -W option (if not present already ) to suppress that warning if 
it helps with the clean up and backward compatibility. It might also open up 
opportunities for a wider clean up  - removing pragma in extensions that 
currently require pragma for no good reason.  I have written more details on 
this in 
https://github.com/KhronosGroup/OpenCL-Docs/pull/355#issuecomment-662679499

>>> The same arguments also apply to `cles_khr_in64`. It's illegal to use int64 
>>> types in embedded profile unless you first enable the extensions. Rather 
>>> than removing it, `cles_khr_2d_image_array_writes` should be added to the 
>>> extension list.
>>
>> I don't think clang currently support embedded profile. Adding extension 
>> into the OpenCLOptions is pointless if it's not used. Do you plan to add any 
>> functionality for it? Defining macros can be done easily outside of clang 
>> source code or if it is preferable to do inside there is 
>> `MacroBuilder::defineMacro`  available in the target hooks. Currently for 
>> every extension added into `OpenCLExtensions.def` there is a macro, a pragma 
>> and a field in `OpenCLOptions` added. This is often more than what is 
>> necessary. Plus Khronos has very many extensions if we assume that all of 
>> them are added in clang it will become scalability and maintanance 
>> nightmare. Most of the extensions only add functions so if we provide a way 
>> to add macros for those outside of clang code base it will keep the common 
>> code clean and vendors can be more flexible in adding the extensions without 
>> the need to modify upstream code if they need to. I see it as an opportunity 
>> to improve common code and out of tree implementations too. It just need a 
>> little bit of restructuring.
>
> My understanding is that both the macro and working #pragma directive is 
> necessary.

I don't believe this is the correct interpretation. If you look at the 
extension spec s9.1 it says:

`Every extension which affects the OpenCL language semantics, syntax or adds 
built-in functions to the language must create a preprocessor #define that 
matches the extension name string.  This #define would be available in the 
language if and only if the extension is supported on a given implementation.`

It does not say that the pragma is needed, it only says that macro is needed. 
That perfectly makes sense because the macro allows to check that the extension 
is present to implement certain functionality conditionally.

OpenCL spec however never clarified the use of pragma that technically makes 
sense because the pragmas in C languages are used for altering the standard 
behavior that can't be otherwise achieved using standard parsing i.e. C99 
section 6.10.1 says about non-STDC pragmas:

`The behavior might cause translation to fail or cause the translator  or  the  
resulting  program  to  behave  in  a  non-conforming  manner.   Any  such 
pragma that is not recognized by the implementation is ignored.`

So C99 only regulates behavior of STDC pragmas and for those it explicitly says 
how the behavior of the parsed program is altered.

Technically OpenCL pragmas are not covered neither in OpenCL C and not C99 and 
therefore it is unclear what the implementation should do. However, with time 
due to the absence of the clarification mutiple interpretations appeared. Sadly 
some of them ended up in a very suboptimal state both for the tooling and the 
application developers because the pragma started to be added for no reason or 
for controlling redundant diagnostics of use of types or functions that 
extensions were introducing. If you are interested in more d

[PATCH] D89487: [AMDGPU] gfx1032 target

[Jay Foad via Phabricator via cfe-commits]

foad added inline comments.



Comment at: llvm/docs/AMDGPUUsage.rst:280

  names.
+ ``gfx1032`` ``amdgcn``   dGPU  - xnack   
*TBA*
+  [off]

xnack looks like a mistake here?



Comment at: llvm/lib/Support/TargetParser.cpp:66
 // Don't bother listing the implicitly true features
-constexpr GPUInfo AMDGCNGPUs[43] = {
+constexpr GPUInfo AMDGCNGPUs[44] = {
   // Name CanonicalKindFeatures

Use `[]` so we don't have to keep updating the number?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89487

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

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: rsmith, rjmccall.
erichkeane requested review of this revision.

As mentioned in the defect, the lambda static invoker does not follow
the calling convention of the lambda itself, which seems wrong. This
patch ensures that the calling convention of operator() is passed onto
the invoker and conversion-operator type.

This is accomplished by extracting the calling-convention determination
code out into a separate function in order to better reflect the 'thiscall'
work, as well as somewhat better support the future implementation of 
https://devblogs.microsoft.com/oldnewthing/20150220-00/?p=44623


Repository:
  rC Clang

https://reviews.llvm.org/D89559

Files:
  clang/lib/Sema/SemaLambda.cpp
  clang/test/SemaCXX/lambda-conversion-op-cc.cpp

Index: clang/test/SemaCXX/lambda-conversion-op-cc.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/lambda-conversion-op-cc.cpp
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-linux-pc %s -verify -DBAD_CONVERSION
+// RUN: %clang_cc1 -fsyntax-only -triple i386-windows-pc %s -verify -DBAD_CONVERSION -DWIN32
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-linux-pc %s -ast-dump | FileCheck %s --check-prefixes=CHECK,LIN64,NODEF
+// RUN: %clang_cc1 -fsyntax-only -triple i386-windows-pc %s -ast-dump -DWIN32 | FileCheck %s --check-prefixes=CHECK,WIN32,NODEF
+
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-linux-pc -fdefault-calling-conv=vectorcall %s -verify -DBAD_VEC_CONVERS
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-linux-pc -fdefault-calling-conv=vectorcall %s -ast-dump | FileCheck %s --check-prefixes=CHECK,VECTDEF
+
+void useage() {
+  auto normal = [](int, float, double){}; // #1
+  auto vectorcall = [](int, float, double) __attribute__((vectorcall)) {}; // #2
+#ifdef WIN32
+  auto thiscall = [](int, float, double) __attribute__((thiscall)) {}; // #3
+#endif // WIN32
+  auto cdecl = [](int, float, double) __attribute__((cdecl)) {};
+
+// CHECK: VarDecl {{.*}} normal '
+// CHECK: LambdaExpr
+// WIN32: CXXMethodDecl {{.*}} operator() 'void (int, float, double) __attribute__((thiscall)) const'
+// LIN64: CXXMethodDecl {{.*}} operator() 'void (int, float, double) const'
+// VECTDEF: CXXMethodDecl {{.*}} operator() 'void (int, float, double) const'
+// NODEF: CXXConversionDecl {{.*}} operator void (*)(int, float, double) 'void
+// NODEF: CXXMethodDecl {{.*}} __invoke 'void (int, float, double)' static inline
+// VECTDEF: CXXConversionDecl {{.*}} operator void (*)(int, float, double) __attribute__((vectorcall)) 'void
+// VECTDEF: CXXMethodDecl {{.*}} __invoke 'void (int, float, double) __attribute__((vectorcall))' static inline
+
+// CHECK: VarDecl {{.*}} vectorcall '
+// CHECK: LambdaExpr
+// CHECK: CXXMethodDecl {{.*}} operator() 'void (int, float, double) __attribute__((vectorcall)) const'
+// CHECK: CXXConversionDecl {{.*}} operator void (*)(int, float, double) __attribute__((vectorcall)) 'void
+// CHECK: CXXMethodDecl {{.*}} __invoke 'void (int, float, double) __attribute__((vectorcall))' static inline
+
+// WIN32: VarDecl {{.*}} thiscall '
+// WIN32: LambdaExpr
+// WIN32: CXXMethodDecl {{.*}} operator() 'void (int, float, double) __attribute__((thiscall)) const'
+// WIN32: CXXConversionDecl {{.*}} operator void (*)(int, float, double) 'void
+// WIN32: CXXMethodDecl {{.*}} __invoke 'void (int, float, double)' static inline
+
+// CHECK: VarDecl {{.*}} cdecl '
+// CHECK: LambdaExpr
+// CHECK: CXXMethodDecl {{.*}} operator() 'void (int, float, double) const'
+// NODEF: CXXConversionDecl {{.*}} operator void (*)(int, float, double) 'void
+// NODEF: CXXMethodDecl {{.*}} __invoke 'void (int, float, double)' static inline
+// VECTDEF: CXXConversionDecl {{.*}} operator void (*)(int, float, double) __attribute__((vectorcall)) 'void
+// VECTDEF: CXXMethodDecl {{.*}} __invoke 'void (int, float, double) __attribute__((vectorcall))' static inline
+
+#ifdef BAD_CONVERSION
+  // expected-error-re@+2 {{no viable conversion from {{.*}} to 'void (*)(int, float, double) __attribute__((vectorcall))}}
+  // expected-note@#1 {{candidate function}}
+  void (*__attribute__((vectorcall)) normal_ptr2)(int, float, double) = normal;
+  // expected-error-re@+2 {{no viable conversion from {{.*}} to 'void (*)(int, float, double)}}
+  // expected-note@#2 {{candidate function}}
+  void (*vectorcall_ptr2)(int, float, double) = vectorcall;
+#ifdef WIN32
+  // expected-error-re@+2 {{no viable conversion from {{.*}} to 'void (*)(int, float, double) __attribute__((thiscall))}}
+  // expected-note@#3 {{candidate function}}
+  void (*__attribute__((thiscall)) thiscall_ptr2)(int, float, double) = thiscall;
+#endif // WIN32
+#endif // BAD_CONVERSION
+
+#ifdef BAD_VEC_CONVERS
+  void (*__attribute__((vectorcall)) normal_ptr2)(int, float, double) = normal;
+  void (* normal_ptr3)(int, float, double) = normal;
+  // expected-error-re@+2 {{no viable conversion from {{.*}} to 

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 298644.
kadircet added a comment.

s/memoryTree/memoryUsage/g


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89277

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/memory_tree.test

Index: clang-tools-extra/clangd/test/memory_tree.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/memory_tree.test
@@ -0,0 +1,81 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void func() {}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"$/memoryTree","params":{}}
+# CHECK:"id": 1,
+# CHECK-NEXT:   "jsonrpc": "2.0",
+# CHECK-NEXT:   "result": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "clangd_server": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "dynamic_index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}},
+# CHECK-NEXT: "main_file": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "references": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "relations": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "preamble": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "index": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "symbols": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "tuscheduler": {
+# CHECK-NEXT: "{{.*}}main.cpp": {
+# CHECK-NEXT:   "_self": {{[0-9]+}},
+# CHECK-NEXT:   "_total": {{[0-9]+}},
+# CHECK-NEXT:   "ast": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "preamble": {
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: },
+# CHECK-NEXT: "_self": {{[0-9]+}},
+# CHECK-NEXT: "_total": {{[0-9]+}}
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -66,6 +66,7 @@
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
+# CHECK-NEXT:  "memoryTreeProvider": true
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
 # CHECK-NEXT:  "selectionRangeProvider": true,
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -25,6 +25,7 @@
 
 #include "URI.h"
 #include "index/SymbolID.h"
+#include "support/MemoryTree.h"
 #include "clang/Index/IndexSymbol.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/JSON.h"
@@ -1576,6 +1577,27 @@
 };
 llvm::j

[PATCH] D89464: Revert "[clang-format] Fix AlignConsecutive on PP blocks"

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru updated this revision to Diff 298639.
sylvestre.ledru added a comment.

Add the test which was failing + add the previous tests (but disabled)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89464

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2783,7 +2783,7 @@
 
   // Checks an edge case in preprocessor handling.
   // These comments should *not* be aligned
-  EXPECT_EQ(
+  EXPECT_NE( // change for EQ when fixed
   "#if FOO\n"
   "#else\n"
   "long a; // Line about a\n"
@@ -2801,6 +2801,24 @@
  "long b_long_name; // Line about b\n"
  "#endif\n",
  getLLVMStyleWithColumns(80)));
+
+  // bug 47589
+  EXPECT_EQ(
+  "namespace m {\n\n"
+  "#define FOO_GLOBAL 0  // Global scope.\n"
+  "#define FOO_LINKLOCAL 1   // Link-local scope.\n"
+  "#define FOO_SITELOCAL 2   // Site-local scope (deprecated).\n"
+  "#define FOO_UNIQUELOCAL 3 // Unique local\n"
+  "#define FOO_NODELOCAL 4   // Loopback\n\n"
+  "} // namespace m\n",
+  format("namespace m {\n\n"
+ "#define FOO_GLOBAL 0   // Global scope.\n"
+ "#define FOO_LINKLOCAL 1  // Link-local scope.\n"
+ "#define FOO_SITELOCAL 2  // Site-local scope (deprecated).\n"
+ "#define FOO_UNIQUELOCAL 3 // Unique local\n"
+ "#define FOO_NODELOCAL 4  // Loopback\n\n"
+ "} // namespace m\n",
+ getLLVMStyleWithColumns(80)));
 }
 
 TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12254,26 +12254,28 @@
Alignment);
 
   // Bug 25167
-  verifyFormat("#if A\n"
-   "#else\n"
-   "int  = 12;\n"
-   "#endif\n"
-   "#if B\n"
-   "#else\n"
-   "int a = 12;\n"
-   "#endif\n",
-   Alignment);
-  verifyFormat("enum foo {\n"
-   "#if A\n"
-   "#else\n"
-   "   = 12;\n"
-   "#endif\n"
-   "#if B\n"
-   "#else\n"
-   "  a = 12;\n"
-   "#endif\n"
-   "};\n",
-   Alignment);
+  /* Uncomment when fixed
+verifyFormat("#if A\n"
+ "#else\n"
+ "int  = 12;\n"
+ "#endif\n"
+ "#if B\n"
+ "#else\n"
+ "int a = 12;\n"
+ "#endif\n",
+ Alignment);
+verifyFormat("enum foo {\n"
+ "#if A\n"
+ "#else\n"
+ "   = 12;\n"
+ "#endif\n"
+ "#if B\n"
+ "#else\n"
+ "  a = 12;\n"
+ "#endif\n"
+ "};\n",
+ Alignment);
+  */
 
   EXPECT_EQ("int a = 5;\n"
 "\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -411,11 +411,9 @@
 if (Changes[i].NewlinesBefore != 0) {
   CommasBeforeMatch = 0;
   EndOfSequence = i;
-  // If there is a blank line, there is a forced-align-break (eg,
-  // preprocessor), or if the last line didn't contain any matching token,
-  // the sequence ends here.
-  if (Changes[i].NewlinesBefore > 1 ||
-  Changes[i].Tok->MustBreakAlignBefore || !FoundMatchOnLine)
+  // If there is a blank line, or if the last line didn't contain any
+  // matching token, the sequence ends here.
+  if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine)
 AlignCurrentSequence();
 
   FoundMatchOnLine = false;
@@ -726,8 +724,6 @@
 if (Changes[i].StartOfBlockComment)
   continue;
 Newlines += Changes[i].NewlinesBefore;
-if (Changes[i].Tok->MustBreakAlignBefore)
-  BreakBeforeNext = true;
 if (!Changes[i].IsTrailingComment)
   continue;
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3037,7 +3037,6 @@
   }
   FormatTok = Tokens->getNextToken();
   FormatTok->MustBreakBefore = true;
-  FormatTok->Must

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

+1 for revert


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66324

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

[PATCH] D88859: APINotes: add APINotesYAMLCompiler

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd marked 3 inline comments as done.
compnerd added a comment.

Thanks for the feedback!




Comment at: clang/include/clang/APINotes/Types.h:1
+//===-- Types.h - API Notes Data Types --*- C++ 
-*-===//
+//

martong wrote:
> So we are mapping existing attributes here, I am missing this to be 
> documented. Why do we support only these attributes?
> Would be consistent to put `SwiftPrivate` here as well, instead of implicitly 
> interpreting that as `bool` in `APINotesYAMLCompiler.cpp`, wouldn't it?
> I think we should list all processed attributes here, or we should just use 
> `Attrs.inc` (see below my comment as well).
These are the ones that are currently needed and can be safely merged.  I 
really wouldn't want to extend the support to all the attributes in the initial 
attempt to merge the functionality as it is something which is actually in 
production already.  However, once the functionality is in a shared state, it 
is much easier to revise and co-evolve other consumers.



Comment at: clang/lib/APINotes/APINotesYAMLCompiler.cpp:240
+namespace {
+struct Class {
+  StringRef Name;

martong wrote:
> Why are these classes in a unnamed namespace? I'd expect them to be in the 
> header under the `clang::api_notes` namespace, so users of the 
> APINotesYAMLCompiler could use these as the structured form of the YAML 
> content. Isn't that the point of the whole stuff?
There will be follow up types that provide structured access to the data.  
These types are strictly for the serialization to and from YAML via `YAML::IO`.



Comment at: clang/lib/APINotes/APINotesYAMLCompiler.cpp:439
+  static void enumeration(IO &IO, EnumExtensibilityKind &EEK) {
+IO.enumCase(EEK, "none",   EnumExtensibilityKind::None);
+IO.enumCase(EEK, "open",   EnumExtensibilityKind::Open);

martong wrote:
> Hmm, why do we need "none"? Can't we interpret the non-existence as "none"?
At the very least we need it for compatibility - this is already a shipping 
feature.  However, nullability is also not completely annotated.  So, there is 
some benefit in tracking the explicit none vs missing.



Comment at: clang/lib/APINotes/APINotesYAMLCompiler.cpp:621
+
+  llvm::yaml::Output OS(llvm::outs());
+  OS << M;

martong wrote:
> I think the stream (`llvm::outs`) should not be written in stone. And why not 
> `llvm::errs` (we `dump` to `errs` usually) ? Could it be a parameter?
Sure, that is reasonable, I should be able to add a `llvm::raw_ostream &` 
parameter.



Comment at: 
clang/test/APINotes/Inputs/Frameworks/Simple.framework/Headers/Simple.apinotes:1
+Name:SimpleKit
+Classes:

martong wrote:
> I am pretty sure this does not provide a full coverage. What about e.g 
> `Functions`? I'd like to see them tested as well.
Correct, it isn't meant to provide full coverage at all.  It is merely meant to 
be enough to ensure that we can load the YAML and process it.  The full 
coverage will come with the follow up patches as they will require filling out 
more of the infrastructure.  I am trying to keep the patch at a reasonable 
size.  I can add additional test cases if you feel strongly that they should be 
added now, but, I do worry about the size of the patch ballooning.



Comment at: clang/test/APINotes/yaml-roundtrip.test:4
+
+We expect only the nullability to be different as it is canonicalized during 
the
+roudtrip.

martong wrote:
> Why do we have this difference? This seems odd and as a superfluous 
> complexity.
The difference is needed for compatibility.  Richard wanted the fully spelt out 
names, but that requires backwards compatibility, so we need the difference 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88859

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

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:623
 {"typeHierarchyProvider", true},
+{"memoryTreeProvider", true}, // clangd extension.
 ;

one last naming nit: I think `memoryUsage` would put appropriately more weight 
on the semantics rather than the data structure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89277

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

[PATCH] D88381: [Flang][Driver]Add PrintPreprocessedInput action `-E`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski commandeered this revision.
awarzynski edited reviewers, added: CarolineConcatto; removed: awarzynski.
awarzynski added a comment.

Since @CarolineConcatto has recently moved to a different project, I am 
assigning this to myself and will be responding to the future review comments. 
Thank you for all the effort @CarolineConcatto !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88381

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

[PATCH] D88381: [Flang][Driver]Add PrintPreprocessedInput action `-E`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 298635.
awarzynski added a comment.

Rebase + refector the unit test

This is re-based on top of the latest version of D87989 
. The unit test is
update for consistency with D87989 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88381

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/CompilerInstance.h
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendAction.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Frontend/Inputs/hello-world.c
  flang/test/Frontend/print-preprocess-C-file.f90
  flang/test/Frontend/print-preprocessed-file.f90
  flang/unittests/Frontend/CMakeLists.txt
  flang/unittests/Frontend/PrintPreprocessedTest.cpp

Index: flang/unittests/Frontend/PrintPreprocessedTest.cpp
===
--- /dev/null
+++ flang/unittests/Frontend/PrintPreprocessedTest.cpp
@@ -0,0 +1,79 @@
+//===- unittests/Frontend/PrintPreprocessedTest.cpp  FrontendAction tests --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "flang/unittests/Frontend/PrintPreprocessedTest.cpp"
+#include "gtest/gtest.h"
+#include "flang/Frontend/CompilerInstance.h"
+#include "flang/Frontend/FrontendOptions.h"
+#include "flang/FrontendTool/Utils.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace Fortran::frontend;
+
+namespace {
+
+TEST(FrontendAction, PrintPreprocessedInput) {
+  std::string inputFile = "test-file.f";
+  std::error_code ec;
+
+  // 1. Create the input file for the file manager
+  // AllSources (which is used to manage files inside every compiler instance),
+  // works with paths. This means that it requires a physical file. Create one.
+  std::unique_ptr os{
+  new llvm::raw_fd_ostream(inputFile, ec, llvm::sys::fs::OF_None)};
+  if (ec)
+FAIL() << "Fail to create the file need by the test";
+
+  // Populate the input file with the pre-defined input and flush it.
+  *(os) << "! test-file.F:\n"
+<< "#ifdef NEW\n"
+<< "  Program A \n"
+<< "#else\n"
+<< "  Program B\n"
+<< "#endif";
+  os.reset();
+
+  // Get the path of the input file
+  llvm::SmallString<64> cwd;
+  if (std::error_code ec = llvm::sys::fs::current_path(cwd))
+FAIL() << "Failed to obtain the current working directory";
+  std::string testFilePath(cwd.c_str());
+  testFilePath += "/" + inputFile;
+
+  // 2. Prepare the compiler (CompilerInvocation + CompilerInstance)
+  CompilerInstance compInst;
+  compInst.CreateDiagnostics();
+  auto invocation = std::make_shared();
+  invocation->GetFrontendOpts().programAction_ = PrintPreprocessedInput;
+
+  compInst.SetInvocation(std::move(invocation));
+  compInst.GetFrontendOpts().inputs_.push_back(
+  FrontendInputFile(testFilePath, Language::Fortran));
+
+  // 3. Set-up the output stream. Using output buffer wrapped as an output
+  // stream, as opposed to an actual file (or a file descriptor).
+  llvm::SmallVector outputFileBuffer;
+  std::unique_ptr outputFileStream(
+  new llvm::raw_svector_ostream(outputFileBuffer));
+  compInst.SetOutputStream(std::move(outputFileStream));
+
+  // 4. Run the earlier defined FrontendAction
+  bool success = ExecuteCompilerInvocation(&compInst);
+
+  // 5. Validate the expected output
+  EXPECT_TRUE(success);
+  EXPECT_TRUE(!outputFileBuffer.empty());
+  EXPECT_TRUE(llvm::StringRef(outputFileBuffer.data()).equals("program b"));
+
+  // 6. Clear the input and the output files. Since we used an output buffer,
+  // there are no physical output files to delete.
+  llvm::sys::fs::remove(inputFile);
+  compInst.ClearOutputFiles(/*EraseFiles=*/true);
+}
+} // namespace
Index: flang/unittests/Frontend/CMakeLists.txt
===
--- flang/unittests/Frontend/CMakeLists.txt
+++ flang/unittests/Frontend/CMakeLists.txt
@@ -1,11 +1,13 @@
 add_flang_unittest(FlangFrontendTests
   CompilerInstanceTest.cpp
   InputOutputTest.cpp
+  PrintPreprocessedTest.cpp
 )
 
 target_link_libraries(FlangFrontendTests
   PRIVATE
   clangBasic
   clangFrontend
+  FortranParser
   flangFrontend
   flangFrontendTool)
Index: flang

[PATCH] D89554: SourceManager: Clarify that FileInfo always has a ContentCache, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith created this revision.
dexonsmith added a reviewer: arphaman.
Herald added subscribers: ributzka, martong.
Herald added a reviewer: shafik.
dexonsmith requested review of this revision.

It turns out that `FileInfo` *always* has a ContentCache. Clarify that
in the code:

- Update the private version of `SourceManager::createFileID` to take a 
`ContentCache&` instead of `ContentCache*`, and rename it to `createFileIDImpl` 
for clarity.
- Change `FileInfo::getContentCache` to return a reference.


https://reviews.llvm.org/D89554

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Lex/ScratchBuffer.cpp
  clang/lib/Rewrite/Rewriter.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/tools/libclang/CIndexInclusionStack.cpp

Index: clang/tools/libclang/CIndexInclusionStack.cpp
===
--- clang/tools/libclang/CIndexInclusionStack.cpp
+++ clang/tools/libclang/CIndexInclusionStack.cpp
@@ -35,7 +35,7 @@
   continue;
 
 const SrcMgr::FileInfo &FI = SL.getFile();
-if (!FI.getContentCache()->OrigEntry)
+if (!FI.getContentCache().OrigEntry)
   continue;
 
 // If this is the main file, and there is a preamble, skip this SLoc. The
@@ -60,7 +60,7 @@
 // Callback to the client.
 // FIXME: We should have a function to construct CXFiles.
 CB(static_cast(
-   const_cast(FI.getContentCache()->OrigEntry)),
+   const_cast(FI.getContentCache().OrigEntry)),
InclusionStack.data(), InclusionStack.size(), clientData);
   }
 }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1452,7 +1452,7 @@
 if (!SLoc->isFile())
   continue;
 const SrcMgr::FileInfo &File = SLoc->getFile();
-const SrcMgr::ContentCache *Cache = File.getContentCache();
+const SrcMgr::ContentCache *Cache = &File.getContentCache();
 if (!Cache->OrigEntry)
   continue;
 
@@ -1952,7 +1952,7 @@
 // Figure out which record code to use.
 unsigned Code;
 if (SLoc->isFile()) {
-  const SrcMgr::ContentCache *Cache = SLoc->getFile().getContentCache();
+  const SrcMgr::ContentCache *Cache = &SLoc->getFile().getContentCache();
   if (Cache->OrigEntry) {
 Code = SM_SLOC_FILE_ENTRY;
   } else
@@ -1970,7 +1970,7 @@
   Record.push_back(File.getFileCharacteristic()); // FIXME: stable encoding
   Record.push_back(File.hasLineDirectives());
 
-  const SrcMgr::ContentCache *Content = File.getContentCache();
+  const SrcMgr::ContentCache *Content = &File.getContentCache();
   bool EmitBlob = false;
   if (Content->OrigEntry) {
 assert(Content->OrigEntry == Content->ContentsEntry &&
Index: clang/lib/Rewrite/Rewriter.cpp
===
--- clang/lib/Rewrite/Rewriter.cpp
+++ clang/lib/Rewrite/Rewriter.cpp
@@ -263,8 +263,8 @@
 StringRef MB = SourceMgr->getBufferData(FID);
 
 unsigned lineNo = SourceMgr->getLineNumber(FID, StartOffs) - 1;
-const SrcMgr::ContentCache *
-Content = SourceMgr->getSLocEntry(FID).getFile().getContentCache();
+const SrcMgr::ContentCache *Content =
+&SourceMgr->getSLocEntry(FID).getFile().getContentCache();
 unsigned lineOffs = Content->SourceLineCache[lineNo];
 
 // Find the whitespace at the start of the line.
@@ -367,8 +367,8 @@
   unsigned startLineNo = SourceMgr->getLineNumber(FID, StartOff) - 1;
   unsigned endLineNo = SourceMgr->getLineNumber(FID, EndOff) - 1;
 
-  const SrcMgr::ContentCache *
-  Content = SourceMgr->getSLocEntry(FID).getFile().getContentCache();
+  const SrcMgr::ContentCache *Content =
+  &SourceMgr->getSLocEntry(FID).getFile().getContentCache();
 
   // Find where the lines start.
   unsigned parentLineOffs = Content->SourceLineCache[parentLineNo];
Index: clang/lib/Lex/ScratchBuffer.cpp
===
--- clang/lib/Lex/ScratchBuffer.cpp
+++ clang/lib/Lex/ScratchBuffer.cpp
@@ -38,8 +38,9 @@
 // Clear out the source line cache if it's already been computed.
 // FIXME: Allow this to be incrementally extended.
 auto *ContentCache = const_cast(
-SourceMgr.getSLocEntry(SourceMgr.getFileID(BufferStartLoc))
- .getFile().getContentCache());
+&SourceMgr.getSLocEntry(SourceMgr.getFileID(BufferStartLoc))
+ .getFile()
+ .getContentCache());
 ContentCache->SourceLineCache = nullptr;
   }
 
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -435,7 +435,7 @@
 if (!SLocEntryLoaded[Index]) {
   // Try to recover; create a SLocEntry so the rest of c

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a subscriber: aeubanks.
spatel added a comment.

In D88154#2328312 , 
@venkataramanan.kumar.llvm wrote:

> I can add few more float type tests with meta data for VF=8.   please let me 
> know your suggestions.

I may be missing some subtlety of the vectorizer behavior. Can we vary the test 
types + metadata in 1 file,s o that there is coverage for something like this 
v2f64 call : `TLI_DEFINE_VECFUNC("llvm.sin.f64", "_ZGVbN2v_sin", 2)`?
I'm just trying to make sure we don't fall into some blind-spot by only testing 
VF=4.




Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -vector-library=LIBMVEC-X86 -inject-tli-mappings -loop-vectorize -S 
< %s | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"

fhahn wrote:
> fpetrogalli wrote:
> > `-inject-tli-mappings` is not required here, as a pass itself is required 
> > by the loop vectorizer.
> I guess it still doesn't hurt to be explicit. Also, can you add a line for 
> the new pass manager?
We need to be explicit about that pass with new-pass-manager as shown here:
df5576a

cc @aeubanks as I'm not sure if we want to update tests with NPM RUN lines or 
if we want to silently transition whenever the default gets changed.


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

https://reviews.llvm.org/D88154

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

[clang] 59a3b1a - clang-format: Assert in-memory file created in createInMemoryFile, NFC

[Duncan P . N . Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2020-10-16T10:20:32-04:00
New Revision: 59a3b1afb28541d5bf37445b028bfd711e3c556a

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

LOG: clang-format: Assert in-memory file created in createInMemoryFile, NFC

`SourceManager::createFileID` asserts that the given `FileEntry` is not
null, so remove the logic that passed in `nullptr`. Since we just added
the file to an in-memory FS via an API that cannot fail, use
`llvm_unreachable` on the error path. Didn't use an `assert` since it
seems cleaner semantically to check the error (and better,
hypothetically, for updating the API to use `Expected` instead of
`ErrorOr`).

I noticed this incidentally while auditing calls to `createFileID`.

Added: 


Modified: 
clang/tools/clang-format/ClangFormat.cpp

Removed: 




diff  --git a/clang/tools/clang-format/ClangFormat.cpp 
b/clang/tools/clang-format/ClangFormat.cpp
index 8dd55d99e2a0..3a7247deab46 100644
--- a/clang/tools/clang-format/ClangFormat.cpp
+++ b/clang/tools/clang-format/ClangFormat.cpp
@@ -179,8 +179,8 @@ static FileID createInMemoryFile(StringRef FileName, 
MemoryBuffer *Source,
  llvm::vfs::InMemoryFileSystem *MemFS) {
   MemFS->addFileNoOwn(FileName, 0, Source);
   auto File = Files.getFile(FileName);
-  return Sources.createFileID(File ? *File : nullptr, SourceLocation(),
-  SrcMgr::C_User);
+  assert(File && "File not added to MemFS?");
+  return Sources.createFileID(*File, SourceLocation(), SrcMgr::C_User);
 }
 
 // Parses : input to a pair of line numbers.



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

[PATCH] D89528: [clang][test] Fix prefix operator++ signature in iterators

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

What is the context here? Did it cause any crash/bug or were you just reading 
through the code for a good night sleep? :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89528

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

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies updated this revision to Diff 298601.
ffrankies marked 7 inline comments as done.
ffrankies added a comment.

Addressed comments by @aaron.ballman regarding the diagnostic warning.

I tried to add a test case for when the filename is `kernel.cl`, `verilog.cl`, 
or `vhdl.cl`, but that did not work because the test suite appends `.tmp.cpp` 
to the end of the test files, and `kernel.cl.tmp.cpp` is not a restricted 
filename. If you know of a way to include this test case in the test suite, 
please let me know. In the meantime, I tested this functionality manually, and 
found a minor bug that has since been fixed.

The bug was: if `kernel.cl` does not have any include directives, then the 
warning would not show up. Fixed this by rearranging the code to check the main 
file name before checking the include directives.


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

https://reviews.llvm.org/D72218

Files:
  clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
  clang-tools-extra/clang-tidy/altera/CMakeLists.txt
  clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
  clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/altera-kernel-name-restriction.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/KERNEL.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/VHDL.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/Verilog.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/vhdl.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherthing.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/kernel.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/kernel.cl/foo.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/verilog.cl/foo.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/vhdl.cl/foo.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some_kernel.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/somedir/verilog.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/thing.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vERILOG.cl
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/verilog.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.CL
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/vhdl_number_two.cl
  clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
  llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/altera/BUILD.gn
@@ -13,6 +13,7 @@
   ]
   sources = [
 "AlteraTidyModule.cpp",
+"KernelNameRestrictionCheck.cpp",
 "StructPackAlignCheck.cpp",
   ]
 }
Index: clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/altera-kernel-name-restriction.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s altera-kernel-name-restriction %t -- -- -I%S/Inputs/altera-kernel-name-restriction
+
+// These are the banned kernel filenames, and should trigger warnings
+#include "kernel.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'kernel.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
+#include "Verilog.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'Verilog.cl' may cause additional compilation errors due to the name of the kernel source file; consider renaming the included kernel source file [altera-kernel-name-restriction]
+#include "VHDL.cl"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including 'VHDL.cl' may cause additional compilation errors due

[PATCH] D89443: [PowerPC][AIX] Make `__vector [un]signed long` an error

[Zarko Todorovski via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/lib/Sema/DeclSpec.cpp:1200
+  // It has also been historically deprecated on AIX (as an alias for
+  // "vector int" in both 32-bit and 64-bit modes) and was made unsupported
+  // in the Clang-based XL compiler since the deprecated type has a number

minor nit, prefer to divide the comment in 2 sentences.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89443

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

[PATCH] D87279: [clang] Fix handling of physical registers in inline assembly operands.

[Jonas Paulsson via Phabricator via cfe-commits]

jonpa updated this revision to Diff 298624.
jonpa added a comment.

That makes sense... Patch updated to keep the tying of operands for this case 
of earlyclobber.

@nickdesaulniers : Can you see if this patch now passes your builds?


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

https://reviews.llvm.org/D87279

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/systemz-inline-asm-02.c
  clang/test/CodeGen/systemz-inline-asm.c

Index: clang/test/CodeGen/systemz-inline-asm.c
===
--- clang/test/CodeGen/systemz-inline-asm.c
+++ clang/test/CodeGen/systemz-inline-asm.c
@@ -129,3 +129,17 @@
 // CHECK: [[RESULT:%.*]] = tail call fp128 asm "axbr $0, $2", "=f,0,f"(fp128 %f, fp128 %g)
 // CHECK: store fp128 [[RESULT]], fp128* [[DEST]]
 }
+
+// Test that there are no tied physreg uses. TwoAddress pass cannot deal with them.
+int test_physregs(void) {
+  // CHECK-LABEL: define signext i32 @test_physregs()
+  register int l __asm__("r7") = 0;
+
+  // CHECK: call i32 asm "lr $0, $1", "={r7},{r7}"
+  __asm__("lr %0, %1" : "+r"(l));
+
+  // CHECK: call i32 asm "$0 $1 $2", "={r7},{r7},{r7}"
+  __asm__("%0 %1 %2" : "+r"(l) : "r"(l));
+
+  return l;
+}
Index: clang/test/CodeGen/systemz-inline-asm-02.c
===
--- /dev/null
+++ clang/test/CodeGen/systemz-inline-asm-02.c
@@ -0,0 +1,13 @@
+// RUN: not %clang_cc1 -triple s390x-linux-gnu -O2 -emit-llvm -o - %s 2>&1 \
+// RUN:  | FileCheck %s
+// REQUIRES: systemz-registered-target
+
+// Test that an error is given if a physreg is defined by multiple operands.
+int test_physreg_defs(void) {
+  register int l __asm__("r7") = 0;
+
+  // CHECK: error: multiple outputs to hard register: r7
+  __asm__("" : "+r"(l), "=r"(l));
+
+  return l;
+}
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -21,6 +21,7 @@
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"
@@ -1836,7 +1837,8 @@
 static std::string
 AddVariableConstraints(const std::string &Constraint, const Expr &AsmExpr,
const TargetInfo &Target, CodeGenModule &CGM,
-   const AsmStmt &Stmt, const bool EarlyClobber) {
+   const AsmStmt &Stmt, const bool EarlyClobber,
+   std::string *GCCReg = nullptr) {
   const DeclRefExpr *AsmDeclRef = dyn_cast(&AsmExpr);
   if (!AsmDeclRef)
 return Constraint;
@@ -1861,6 +1863,8 @@
   }
   // Canonicalize the register here before returning it.
   Register = Target.getNormalizedGCCRegisterName(Register);
+  if (GCCReg != nullptr)
+*GCCReg = Register.str();
   return (EarlyClobber ? "&{" : "{") + Register.str() + "}";
 }
 
@@ -2059,6 +2063,9 @@
   // Keep track of out constraints for tied input operand.
   std::vector OutputConstraints;
 
+  // Keep track of defined physregs.
+  llvm::SmallSet PhysRegOutputs;
+
   // An inline asm can be marked readonly if it meets the following conditions:
   //  - it doesn't have any sideeffects
   //  - it doesn't clobber memory
@@ -2078,9 +2085,15 @@
 const Expr *OutExpr = S.getOutputExpr(i);
 OutExpr = OutExpr->IgnoreParenNoopCasts(getContext());
 
+std::string GCCReg;
 OutputConstraint = AddVariableConstraints(OutputConstraint, *OutExpr,
   getTarget(), CGM, S,
-  Info.earlyClobber());
+  Info.earlyClobber(),
+  &GCCReg);
+// Give an error on multiple outputs to same physreg.
+if (!GCCReg.empty() && !PhysRegOutputs.insert(GCCReg).second)
+  CGM.Error(S.getAsmLoc(), "multiple outputs to hard register: " + GCCReg);
+
 OutputConstraints.push_back(OutputConstraint);
 LValue Dest = EmitLValue(OutExpr);
 if (!Constraints.empty())
@@ -2167,7 +2180,8 @@
 LargestVectorWidth =
 std::max((uint64_t)LargestVectorWidth,
  VT->getPrimitiveSizeInBits().getKnownMinSize());
-  if (Info.allowsRegister())
+  // Only tie earlyclobber physregs.
+  if (Info.allowsRegister() && (GCCReg.empty() || Info.earlyClobber()))
 InOutConstraints += llvm::utostr(i);
   else
 InOutConstraints += OutputConstraint;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89372: [OpenCL] Remove unused extensions

[Jan Vesely via Phabricator via cfe-commits]

jvesely added a comment.

In D89372#2334225 , @Anastasia wrote:

>> 
>
> Ok, so the pragma is supposed to control the pointer dereferencing which 
> seems like a valid case but however, it is not implemented now. Do we know of 
> any immediate plans from anyone to implement this? If not I would prefer to 
> remove the pragma for now? If someone decided to implement this functionality 
> later fully it is absolutely fine. Pragma will be very easy to add. There is 
> no need for everyone to pay the price for something that can't be used at the 
> moment.

I though the current behaviour of 'you can use #pragma, but the dereferences 
are always legal' was intentional for backward compatibility.
I don't think there are programs that would set it to 'disabled' and expect 
compilation failure. My concern is that legacy apps would set '#pragma enabled' 
before using char/short. such behavior would produce warning/error if with this 
change applied.

>> The same arguments also apply to `cles_khr_in64`. It's illegal to use int64 
>> types in embedded profile unless you first enable the extensions. Rather 
>> than removing it, `cles_khr_2d_image_array_writes` should be added to the 
>> extension list.
>
> I don't think clang currently support embedded profile. Adding extension into 
> the OpenCLOptions is pointless if it's not used. Do you plan to add any 
> functionality for it? Defining macros can be done easily outside of clang 
> source code or if it is preferable to do inside there is 
> `MacroBuilder::defineMacro`  available in the target hooks. Currently for 
> every extension added into `OpenCLExtensions.def` there is a macro, a pragma 
> and a field in `OpenCLOptions` added. This is often more than what is 
> necessary. Plus Khronos has very many extensions if we assume that all of 
> them are added in clang it will become scalability and maintanance nightmare. 
> Most of the extensions only add functions so if we provide a way to add 
> macros for those outside of clang code base it will keep the common code 
> clean and vendors can be more flexible in adding the extensions without the 
> need to modify upstream code if they need to. I see it as an opportunity to 
> improve common code and out of tree implementations too. It just need a 
> little bit of restructuring.

My understanding is that both the macro and working #pragma directive is 
necessary. The configuration bit is only needed if clang changes behaviour, 
which is a separate question.
I'd also argue that new third party extensions need an API call to register new 
extensions in order to get a working pragma mechanism.

Even if an extension only adds access new functions, pragma should control if 
user functions with conflicting names are legal or not.

for example, a program can implement function `new_func`, which gets later 
added to an extension. since the program doesn't know about the new extension 
it doesn't use `#pragma new_extension:enable` and continues to work correctly.
If the new extension exposes `new_func` unconditionally, the program breaks, 
because it doesn't check for a macro that didn't exist at the time it was 
written.
more recent programs can use ifdef to either use `new_func` provided by the 
extension, or implement a custom version.

I didn't find much about embedded program behavior in full profile 
implementation in the specs.
It only says that "embedded profile is a subset" which imo implies that legal 
embedded profile programs should work correctly in a full profile 
implementation.
This implies that cles_* pragmas should continue to work even if the behavior 
is always supported.

>>> Are you suggesting to leave this out? However I don't see any evidence that 
>>> this require either macro or pragma. I feel this is in rather incomplete 
>>> state. So I don't feel we can accomodate for all of these.
>>
>> "incomplete specification" is imo a good reason to drop support for an 
>> extension. My argument is that explanation of legacy extensions should rely 
>> on the spec that introduced them, rather than the current 2.x/3.x which does 
>> an arguably poor job on backward compatibility.
>
> Ok, the idea is not to break backwards compatibility of course. For the 
> extensions that intended to modify language (but never did) if we look from 
> upstream clang user's perspective those extensions couldn't possibly be used 
> to do anything useful. It is of course possible that in some forks the 
> functionality has been completed but then they can easily update the 
> implementation to include the extension definition back. This is very small 
> change compared to the actual extension functionality.
>
> I am ok to leave the extensions that could hypotetically modify the language 
> out of this patch for now. Perhaps we could add a comment explaining that 
> they are unused and only left for backwards compatibility. In a long term we 
> need to find some ways to remove the legacy that

[PATCH] D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks

[Nathan James via Phabricator via cfe-commits]

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

Updated release notes to reflect change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79674

Files:
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -266,14 +266,32 @@
   virtual ~AOverridden() = default;
   virtual void BadBaseMethod() = 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
 };
 
 class COverriding : public AOverridden {
 public:
   // Overriding a badly-named base isn't a new violation.
   void BadBaseMethod() override {}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override {}
+  
+  void foo() {
+BadBaseMethod();
+// CHECK-FIXES: {{^}}v_Bad_Base_Method();
+this->BadBaseMethod();
+// CHECK-FIXES: {{^}}this->v_Bad_Base_Method();
+AOverridden::BadBaseMethod();
+// CHECK-FIXES: {{^}}AOverridden::v_Bad_Base_Method();
+COverriding::BadBaseMethod();
+// CHECK-FIXES: {{^}}COverriding::v_Bad_Base_Method();
+  }
 };
 
+void VirtualCall(AOverridden &a_vItem) {
+  a_vItem.BadBaseMethod();
+  // CHECK-FIXES: {{^}}  a_vItem.v_Bad_Base_Method();
+}
+
 template 
 class CRTPBase {
 public:
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -125,6 +125,9 @@
   Added an option `GetConfigPerFile` to support including files which use
   different naming styles.
 
+  Now renames overridden virtual methods if the method they override has a
+  style violation.
+
 - Removed `google-runtime-references` check because the rule it checks does
   not exist in the Google Style Guide anymore.
 
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -135,6 +135,26 @@
  this));
 }
 
+/// Returns the function that \p Method is overridding. If There are none or
+/// multiple overrides it returns nullptr. If the overridden function itself is
+/// overridding then it will recurse up to find the first decl of the function.
+static const CXXMethodDecl *getOverrideMethod(const CXXMethodDecl *Method) {
+  if (Method->size_overridden_methods() != 1)
+return nullptr; // no overrides
+  while (true) {
+const CXXMethodDecl *Next = *Method->begin_overridden_methods();
+assert(Next && "Overridden method shouldn't be null");
+if (Next->size_overridden_methods() == 0) {
+  return Next;
+}
+if (Next->size_overridden_methods() == 1) {
+  Method = Next;
+  continue;
+}
+return nullptr; // Multiple overrides
+  }
+}
+
 void RenamerClangTidyCheck::addUsage(
 const RenamerClangTidyCheck::NamingCheckId &Decl, SourceRange Range,
 SourceManager *SourceMgr) {
@@ -173,6 +193,10 @@
 void RenamerClangTidyCheck::addUsage(const NamedDecl *Decl, SourceRange Range,
  SourceManager *SourceMgr) {
   Decl = cast(Decl->getCanonicalDecl());
+  if (const auto *Method = dyn_cast(Decl)) {
+if (const CXXMethodDecl *Overridden = getOverrideMethod(Method))
+  Decl = Overridden;
+  }
   return addUsage(RenamerClangTidyCheck::NamingCheckId(Decl->getLocation(),
Decl->getNameAsString()),
   Range, SourceMgr);
@@ -412,6 +436,14 @@
   }
 }
 
+// Fix overridden methods
+if (const auto *Method = Result.Nodes.getNodeAs("decl")) {
+  if (const CXXMethodDecl *Overridden = getOverrideMethod(Method)) {
+addUsage(Overridden, Method->getLocation());
+return; // Don't try to add the actual decl as a Failure.
+  }
+}
+
 // Ignore ClassTemplateSpecializationDecl which are creating duplicate
 // replacements with CXXRecordDecl.
 if (isa(Decl))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits