[PATCH] D135558: [Clang][NFC]Use isa instead of dyn_cast in shouldAddReversedEqEq

[Liao Chunyu via Phabricator via cfe-commits]

liaolucy abandoned this revision.
liaolucy added a comment.

In D135558#3848649 , @tbaeder wrote:

> This was fixed as 
> https://github.com/llvm/llvm-project/commit/6c49d5db30227d21e929bb12dc046c36ede67fad.
>  Note that for such patches, you don't need to wait for review, like Erich 
> didn't.

thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135558

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

[PATCH] D129156: Add -fpass-plugin option to Flang

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D129156#3847396 , @tarunprabhu 
wrote:

> Added `examples` to `REQUIRES` in `test/Driver/pass-plugin.f90.

Thanks for the update!

> I still cannot reproduce the build failure on my end. @MatsPetersson tested 
> this patch and the tests passed.

@MatsPetersson & @clementval , could you share you build command so that the 
failure can be reproduced before this re-lands?

> Could someone else test this on their build and let me know if it works - 
> especially if the previous version failed for you.

I can try as soon as folks share their build commands. In general, I'm 
concerned that this fix is not enough. It merely makes sure that 
"pass-plugin.f90" is only run when `LLVM_BUILD_EXAMPLES` is set (or whatever 
other flag that sets `examples` in LIT). However, that's not sufficient to make 
sure that `libBye.so` is built when running `ninja check-flang`. I might wrong 
though - have you checked this?


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

https://reviews.llvm.org/D129156

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

[PATCH] D129156: Add -fpass-plugin option to Flang

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D129156#3848692 , @awarzynski 
wrote:

> In D129156#3847396 , @tarunprabhu 
> wrote:
>
>> Added `examples` to `REQUIRES` in `test/Driver/pass-plugin.f90.
>
> Thanks for the update!
>
>> I still cannot reproduce the build failure on my end. @MatsPetersson tested 
>> this patch and the tests passed.
>
> @MatsPetersson & @clementval , could you share you build command so that the 
> failure can be reproduced before this re-lands?

I shared it with @tarunprabhu and I think the only major difference is the use 
of Ninja vs. Unix Makefiles.

  cmake \
-G "Unix Makefiles" \
-DCMAKE_BUILD_TYPE=Release \
-DLLVM_ENABLE_ASSERTIONS=ON \
-DLLVM_TARGETS_TO_BUILD=host \
-DLLVM_ENABLE_PROJECTS="clang;mlir;flang" \
-DLLVM_ENABLE_RUNTIMES="compiler-rt"


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

https://reviews.llvm.org/D129156

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

[PATCH] D129156: Add -fpass-plugin option to Flang

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D129156#3848704 , @clementval 
wrote:

>>> I still cannot reproduce the build failure on my end. @MatsPetersson tested 
>>> this patch and the tests passed.
>>
>> @MatsPetersson & @clementval , could you share you build command so that the 
>> failure can be reproduced before this re-lands?
>
> I shared it with @tarunprabhu and I think the only major difference is the 
> use of Ninja vs. Unix Makefiles.
>
>   cmake \
> -G "Unix Makefiles" \
> -DCMAKE_BUILD_TYPE=Release \
> -DLLVM_ENABLE_ASSERTIONS=ON \
> -DLLVM_TARGETS_TO_BUILD=host \
> -DLLVM_ENABLE_PROJECTS="clang;mlir;flang" \
> -DLLVM_ENABLE_RUNTIMES="compiler-rt"

Thanks Valentin! Switching between generators will definitely change the order 
in which libraries will built (and, AFAIK, the order is non-deterministic to 
begin with). I will try to experiment later today.

If anyone manages to test this in the meantime, please share here - that's part 
of the public review process :)


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

https://reviews.llvm.org/D129156

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

> FAOD: I think this should not be about "who's patches" - but about the 
> compilation model and command lines that we want to end up with.

BTW, in the current context, when I say "my" and "your", I just want to refer 
the corresponding things. There is no means to offend.

In D134267#3848672 , @iains wrote:

>> (2) Your scheme saves the time for deserialization. However, clang implement 
>> many ODR checks in the deserialization part. Skipping the deserialization 
>> will omit these ODR checks, which will break the semantics. And this is the 
>> reason why that scheme may not be good.
>
> What you seem to be saying here is that:
>
> `clang++ -std=c++20 foo.cxxm -c -o foo,o`
>
> edit;  maybe I should have written -xc++ foo.cxxm
>
> Does different ODR analysis from:
>
> `clang++ -std=c++20 foo.cxxm --precompile -o foo.pcm`
> `clang++ -std=c++20 foo.pcm -o foo.o`
>
> If so, then that is of some concern since those are both valid compilations 
> in the current compiler.
>
> In the first case, the back end is connected as an AST consumer to the front 
> - there is no serialise/deserialise.

No. **Currently**, in clang, the following two is the same:

`clang++ -std=c++20 foo.cxxm -c -o foo.o`

with

`clang++ -std=c++20 foo.cxxm --precompile -o foo.pcm`
`clang++ -std=c++20 foo.pcm -o foo.o`

The first compilation will generate `.pcm` files in the `/tmp` directories and 
compile the `.pcm` files in the `/tmp` directories.

We could verify it by inserting debugging informations. Or writing a 
ODR-mismatch example, then when the compiler emits an error, we'll find the 
compiler emits the error in the deserialization part. I met these 2 situations 
for many times : )

> The model I implemented simply streams the AST instead of throwing it away ...

So that model misses the ODR checks, which is not good. Since it matters with 
the semantics.

> (repeating here, it is not the deserialisation that I think is key [although 
> that is also useful] but minimising process launches).

So from my point of view, it is much  more important to preserve the semantics 
than minimizing the process launches.

> That is a possibility, at the expense of more process launches - and I do not 
> think that the corresponding model for the server-interactive approach has 
> yet been fully explored.
>
> We go to some trouble (with the driver invoking the compiler in-process) to 
> avoid unnecessary process launches .. with the current state of hardware, it 
> could well be more efficient to have many processes waiting rather than many 
> processes launched.
>
> (I am not making a claim here, just observing that the data are incomplete, 
> since we do not have a corresponding model offered for the client-server 
> case, and we do not have any kind of measurements - these are all "thought 
> experiments").

Yeah, I agree that there more space to explore in the client-server model. But 
for the current building model, the 2 phase compilation is obviously better 
than the 1 phase compilation. And my point is, if one day we proved that there 
are many good points in the client/server model, it is not too late to add it 
in clang. And **currently** it is not a good reason to block the current patch, 
at least it is pretty simple and innocent.

> There seem to be two schools of thought. (1) It is good to use extensions to 
> determine the file contents (2) we should not impose a requirement for magic 
> extension names if the implementation can determine the content without.
>
> For my part, I do prefer the second .. but of course will be happy to go with 
> consensus,

I'm confused the second. I mean what if the other static analyzing tools 
(including build system but not limited to) want to analyze the modular 
project, if they can only invoke the compiler to query the result. Then things 
look complex enough in the state. But assume we can implement great compiler 
interfaces. There are still problems. The tools need to pass the potential 
module unit files to the compiler. With the first choice, the tools can pass 
the module unit files precisely while with the second one, the input number 
will be much larger. So it looks really worse to me...



Since we discussed many things. Let me try to summarize to avoid missing the 
topic.

(1) For the client-server model, I think it is too far now. I am not against 
it. Also I don't feel the current simple patch would is not good for it. 
(2) Your implementation skips the deserialization part, which misses many ODR 
checks. I think this is not good.
(3) I suddenly realize that your **design** and my **design** are not 
overlapped in some perspective. I mean the patch is working for `*.cppm` files, 
and your design is intended for `*.cpp` files. Then a question is: "what would 
the patch do for `*.cppm` files"? If we don't handle it specially, in your 
implementation, when we compile a `.cppm` file, the `.pcm` file would be 

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D134267#3848745 , @ChuanqiXu wrote:

>> FAOD: I think this should not be about "who's patches" - but about the 
>> compilation model and command lines that we want to end up with.
>
> BTW, in the current context, when I say "my" and "your", I just want to refer 
> the corresponding things. There is no means to offend.

sure - no problem.

I guess we ought to be a bit more clear:
There is implementation divergence between GCC and clang and from the point to 
view of users and build systems having two different command line sets is not 
helpful.

The patches I drafted were aimed at removing that divergence.
If that is not possible (because the ODR analysis requires steps that make it 
very difficult or inefficient to do so) then some more thought and/or 
restructuring is needed.

> In D134267#3848672 , @iains wrote:
>
>>> (2) Your scheme saves the time for deserialization. However, clang 
>>> implement many ODR checks in the deserialization part. Skipping the 
>>> deserialization will omit these ODR checks, which will break the semantics. 
>>> And this is the reason why that scheme may not be good.
>>
>> What you seem to be saying here is that:
>>
>> `clang++ -std=c++20 foo.cxxm -c -o foo,o`
>>
>> edit;  maybe I should have written -xc++ foo.cxxm
>>
>> Does different ODR analysis from:
>>
>> `clang++ -std=c++20 foo.cxxm --precompile -o foo.pcm`
>> `clang++ -std=c++20 foo.pcm -o foo.o`
>>
>> If so, then that is of some concern since those are both valid compilations 
>> in the current compiler.
>>
>> In the first case, the back end is connected as an AST consumer to the front 
>> - there is no serialise/deserialise.
>
> No. **Currently**, in clang, the following two is the same:
>
> `clang++ -std=c++20 foo.cxxm -c -o foo.o`

That's why I added the edit ...

`clang++ -std=c++20 -xc++ foo.cxxm -c -o foo.o`

AFAIU that will not produce any error for the user?

suppose I have foo.cxx which includes modules and has local declarations?
`clang++ -std=c++20  foo.cxx -c -o foo.o`

> with
>
> `clang++ -std=c++20 foo.cxxm --precompile -o foo.pcm`
> `clang++ -std=c++20 foo.pcm -o foo.o`
>
> The first compilation will generate `.pcm` files in the `/tmp` directories 
> and compile the `.pcm` files in the `/tmp` directories.

yes, I should have added the -xc++ in the first case ;)

> We could verify it by inserting debugging informations. Or writing a 
> ODR-mismatch example, then when the compiler emits an error, we'll find the 
> compiler emits the error in the deserialization part. I met these 2 
> situations for many times : )
>
>> The model I implemented simply streams the AST instead of throwing it away 
>> ...
>
> So that model misses the ODR checks, which is not good. Since it matters with 
> the semantics.

Understood - I wonder if we have layering right here - surely Sema should be 
responsible for ODR checks?
Is there any reason that relevant hashes cannot be generated on the AST 
//**without**// streaming it?
Was this model intended from the start ?
(the fact that one can connect a direct AST consumer to the back end, suggests 
that enforcing serialisation/deserialisation was not expected to be a 
requirement at some stage)

>> (repeating here, it is not the deserialisation that I think is key [although 
>> that is also useful] but minimising process launches).
>
> So from my point of view, it is much  more important to preserve the 
> semantics than minimizing the process launches.
>
>> That is a possibility, at the expense of more process launches - and I do 
>> not think that the corresponding model for the server-interactive approach 
>> has yet been fully explored.
>>
>> We go to some trouble (with the driver invoking the compiler in-process) to 
>> avoid unnecessary process launches .. with the current state of hardware, it 
>> could well be more efficient to have many processes waiting rather than many 
>> processes launched.
>>
>> (I am not making a claim here, just observing that the data are incomplete, 
>> since we do not have a corresponding model offered for the client-server 
>> case, and we do not have any kind of measurements - these are all "thought 
>> experiments").
>
> Yeah, I agree that there more space to explore in the client-server model. 
> But for the current building model, the 2 phase compilation is obviously 
> better than the 1 phase compilation. And my point is, if one day we proved 
> that there are many good points in the client/server model, it is not too 
> late to add it in clang. And **currently** it is not a good reason to block 
> the current patch, at least it is pretty simple and innocent.
>
>> There seem to be two schools of thought. (1) It is good to use extensions to 
>> determine the file contents (2) we should not impose a requirement for magic 
>> extension names if the implementation can determine the content without.
>>
>> For my part, I do prefer the sec

[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

[David Spickett via Phabricator via cfe-commits]

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

> although those changes don't actually test the code path changed here

Yeah I'm just cargo culting on that one so it doesn't look strange that a few 
are missing.

If we're going to change the suffix that can be done elsewhere and reviewed by 
someone who understands them better than me :)

Not crashing is an improvement in itself, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135461

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Clang header modules started with implicit builds and module caches. Apple is 
moving too explicit builds. There are all kinds of problems with implicit 
module builds. I believe that C++20 modules shouldn't make the same mistake.


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

https://reviews.llvm.org/D134267

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

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 466729.
hokein marked an inline comment as done.
hokein added a comment.

add assert


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135440

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
@@ -336,6 +336,7 @@
 void SourceManager::clearIDTables() {
   MainFileID = FileID();
   LocalSLocEntryTable.clear();
+  LocalLocOffsetTable.clear();
   LoadedSLocEntryTable.clear();
   SLocEntryLoaded.clear();
   LastLineNoFileIDQuery = FileID();
@@ -615,9 +616,11 @@
 Diag.Report(IncludePos, diag::err_include_too_large);
 return FileID();
   }
+  assert(LoadedSLocEntryTable.size() == LocalLocOffsetTable.size());
   LocalSLocEntryTable.push_back(
   SLocEntry::get(NextLocalOffset,
  FileInfo::get(IncludePos, File, FileCharacter, Filename)));
+  LocalLocOffsetTable.push_back(NextLocalOffset);
   // We do a +1 here because we want a SourceLocation that means "the end of the
   // file", e.g. for the "no newline at the end of the file" diagnostic.
   NextLocalOffset += FileSize + 1;
@@ -668,7 +671,9 @@
 SLocEntryLoaded[Index] = true;
 return SourceLocation::getMacroLoc(LoadedOffset);
   }
+  assert(LocalSLocEntryTable.size() == LocalLocOffsetTable.size());
   LocalSLocEntryTable.push_back(SLocEntry::get(NextLocalOffset, Info));
+  LocalLocOffsetTable.push_back(NextLocalOffset);
   assert(NextLocalOffset + Length + 1 > NextLocalOffset &&
  NextLocalOffset + Length + 1 <= CurrentLoadedOffset &&
  "Ran out of source locations!");
@@ -799,10 +804,10 @@
   // SLocOffset.
   unsigned LessIndex = 0;
   // upper bound of the search range.
-  unsigned GreaterIndex = LocalSLocEntryTable.size();
+  unsigned GreaterIndex = LocalLocOffsetTable.size();
   if (LastFileIDLookup.ID >= 0) {
 // Use the LastFileIDLookup to prune the search space.
-if (LocalSLocEntryTable[LastFileIDLookup.ID].getOffset() < SLocOffset)
+if (LocalLocOffsetTable[LastFileIDLookup.ID] < SLocOffset)
   LessIndex = LastFileIDLookup.ID;
 else
   GreaterIndex = LastFileIDLookup.ID;
@@ -812,8 +817,8 @@
   unsigned NumProbes = 0;
   while (true) {
 --GreaterIndex;
-assert(GreaterIndex < LocalSLocEntryTable.size());
-if (LocalSLocEntryTable[GreaterIndex].getOffset() <= SLocOffset) {
+assert(GreaterIndex < LocalLocOffsetTable.size());
+if (LocalLocOffsetTable[GreaterIndex] <= SLocOffset) {
   FileID Res = FileID::get(int(GreaterIndex));
   // Remember it.  We have good locality across FileID lookups.
   LastFileIDLookup = Res;
@@ -827,8 +832,7 @@
   NumProbes = 0;
   while (true) {
 unsigned MiddleIndex = (GreaterIndex-LessIndex)/2+LessIndex;
-SourceLocation::UIntTy MidOffset =
-getLocalSLocEntry(MiddleIndex).getOffset();
+SourceLocation::UIntTy MidOffset = LocalLocOffsetTable[MiddleIndex];
 
 ++NumProbes;
 
@@ -840,8 +844,8 @@
 }
 
 // If the middle index contains the value, succeed and return.
-if (MiddleIndex + 1 == LocalSLocEntryTable.size() ||
-SLocOffset < getLocalSLocEntry(MiddleIndex + 1).getOffset()) {
+if (MiddleIndex + 1 == LocalLocOffsetTable.size() ||
+SLocOffset < LocalLocOffsetTable[MiddleIndex + 1]) {
   FileID Res = FileID::get(MiddleIndex);
 
   // Remember it.  We have good locality across FileID lookups.
@@ -2169,6 +2173,9 @@
<< NumMacroArgsComputed << " files with macro args computed.\n";
   llvm::errs() << "FileID scans: " << NumLinearScans << " linear, "
<< NumBinaryProbes << " binary.\n";
+  llvm::errs() << "Total bytes: "
+   << getDataStructureSizes() + getContentCacheSize() +
+  getMemoryBufferSizes().malloc_bytes;
 }
 
 LLVM_DUMP_METHOD void SourceManager::dump() const {
@@ -2252,6 +2259,7 @@
 size_t SourceManager::getDataStructureSizes() const {
   size_t size = llvm::capacity_in_bytes(MemBufferInfos)
 + llvm::capacity_in_bytes(LocalSLocEntryTable)
++ llvm::capacity_in_bytes(LocalLocOffsetTable)
 + llvm::capacity_in_bytes(LoadedSLocEntryTable)
 + llvm::capacity_in_bytes(SLocEntryLoaded)
 + llvm::capacity_in_bytes(FileInfos);
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -692,6 +692,9 @@
   /// Positive FileIDs are indexes into this table. Entry 0 indicates an invalid
   /// expansion.
   SmallVector LocalSLocEntryTable;
+  /// An in-parallel SlocEntry offset table, merely used for speeding up the
+  /// FileID lookup.
+  SmallVector LocalLocOffsetTable;
 
   /// The table of SLocEntries

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D134267#3848822 , @tschuett wrote:

> Clang header modules started with implicit builds and module caches. Apple is 
> moving too explicit builds. There are all kinds of problems with implicit 
> module builds. I believe that C++20 modules shouldn't make the same mistake.

I do not believe that the client-sever build system model (a.k.a. P1184 
 / module mapper) is quite the same thing as 
implicit modules - there is a partial similarity in that the dependencies are 
discovered from the sources during compilation (rather than from a pre-scan).

However it does not try to turn the compiler into a build system; that work is 
done by the module-mapper (which can be some trivial in-process scheme or a 
full-blown build system, e.g. build2 - which I believe has an implementation 
that works with GCC).

(clearly we [the whole C++ community] need to figure out how we want these 
things to look to the end user and try to harmonise the implementations)


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

https://reviews.llvm.org/D134267

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

In D134267#3848838 , @iains wrote:

> In D134267#3848822 , @tschuett 
> wrote:
>
>> Clang header modules started with implicit builds and module caches. Apple 
>> is moving too explicit builds. There are all kinds of problems with implicit 
>> module builds. I believe that C++20 modules shouldn't make the same mistake.
>
> I do not believe that the client-sever build system model (a.k.a. P1184 
>  / module mapper) is quite the same thing as 
> implicit modules - there is a partial similarity in that the dependencies are 
> discovered from the sources during compilation (rather than from a pre-scan).
>
> However it does not try to turn the compiler into a build system; that work 
> is done by the module-mapper (which can be some trivial in-process scheme or 
> a full-blown build system, e.g. build2 - which I believe has an 
> implementation that works with GCC).

But this diff is doing something else. It puts the build-system again into 
clang.

> (clearly we [the whole C++ community] need to figure out how we want these 
> things to look to the end user and try to harmonise the implementations)




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

https://reviews.llvm.org/D134267

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

[PATCH] D135440: [SourceManager] Speedup getFileIDLocal with a separate Offset Table.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D135440#3847720 , @nickdesaulniers 
wrote:

> Is it worth it and possible to fully decompose `LocalSLocEntryTable` into 
> arrays of its constituent parts, and only construct a `SLocEntry` when 
> necessary?

Fully decomposing `LocalSLocEntryTable` into arrays seems a good idea (which 
will get us more memory back, since we don't need to pay the 4 padding bytes 
for `SLocEntry`).
But I think constructing a `SLocEntry` every time we call `getSLocEntry` would 
be expensive since this is a hot function. Instead, we should probably 
deprecate the `SLocEntry` structure at all -- adding `getOffset(FileID)`, 
`getFileInfo(FileID)`, `getExpansionInfo(FileID)` to the `SourceManager`, this 
would take some more work (update all `SLocEntry` usage, tackle 
loadedSLocEntry` etc).

> I'm ok with the cost increase;  please also adding asserts to keep the size 
> of the vectors equivalent.

added. I'd wait for a few days before landing it for @aaron.ballman's opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135440

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

[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 466731.
ckandeler added a comment.

Template parameters are definitions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
@@ -18,18 +18,18 @@
 TEST_F(AnnotateHighlightingsTest, Test) {
   EXPECT_AVAILABLE("^vo^id^ ^f(^) {^}^"); // available everywhere.
   EXPECT_AVAILABLE("[[int a; int b;]]");
-  EXPECT_EQ("void /* Function [decl] [globalScope] */f() {}",
+  EXPECT_EQ("void /* Function [decl] [def] [globalScope] */f() {}",
 apply("void ^f() {}"));
 
   EXPECT_EQ(apply("[[int f1(); const int x = f1();]]"),
 "int /* Function [decl] [globalScope] */f1(); "
-"const int /* Variable [decl] [readonly] [fileScope] */x = "
+"const int /* Variable [decl] [def] [readonly] [fileScope] */x = "
 "/* Function [globalScope] */f1();");
 
   // Only the targeted range is annotated.
   EXPECT_EQ(apply("void f1(); void f2() {^}"),
 "void f1(); "
-"void /* Function [decl] [globalScope] */f2() {}");
+"void /* Function [decl] [def] [globalScope] */f2() {}");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -48,12 +48,21 @@
 assert(StartOffset <= EndOffset);
 assert(NextChar <= StartOffset);
 
+bool hasDef =
+T.Modifiers & (1 << uint32_t(HighlightingModifier::Definition));
+bool hasDecl =
+T.Modifiers & (1 << uint32_t(HighlightingModifier::Declaration));
+EXPECT_TRUE(!hasDef || hasDecl);
+
 OS << Input.substr(NextChar, StartOffset - NextChar);
 OS << '$' << T.Kind;
 for (unsigned I = 0;
  I <= static_cast(HighlightingModifier::LastModifier); ++I) {
-  if (T.Modifiers & (1 << I))
-OS << '_' << static_cast(I);
+  if (T.Modifiers & (1 << I)) {
+// _decl_def is common and redundant, just print _def instead.
+if (I != uint32_t(HighlightingModifier::Declaration) || !hasDef)
+  OS << '_' << static_cast(I);
+  }
 }
 OS << "[[" << Input.substr(StartOffset, EndOffset - StartOffset) << "]]";
 NextChar = EndOffset;
@@ -96,52 +105,52 @@
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
   R"cpp(
-  struct $Class_decl[[AS]] {
+  struct $Class_def[[AS]] {
 double $Field_decl[[SomeMember]];
   };
   struct {
-  } $Variable_decl[[S]];
-  void $Function_decl[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
-$Class[[AS]] $LocalVariable_decl[[AA]];
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
-auto $LocalVariable_decl[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
+  } $Variable_def[[S]];
+  void $Function_def[[foo]](int $Parameter_def[[A]], $Class[[AS]] $Parameter_def[[As]]) {
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[VeryLongVariableName]] = 12312;
+$Class[[AS]] $LocalVariable_def[[AA]];
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+auto $LocalVariable_def[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_def[[A]]) -> void {};
 $LocalVariable[[FN]](12312);
   }
 )cpp",
   R"cpp(
   void $Function_decl[[foo]](int);
   void $Function_decl[[Gah]]();
-  void $Function_decl[[foo]]() {
-auto $LocalVariable_decl[[Bou]] = $Function[[Gah]];
+  void $Function_def[[foo]]() {
+auto $LocalVariable_def[[Bou]] = $Function[[Gah]];
   }
-  struct $Class_decl[[A]] {
+  struct $Class_def[[A]] {
 void $Method_decl[[abc]]();
   };
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
-template
-struct $Class_decl[[A]] {
+template
+struct $Class_def[[A

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D134267#3848793 , @iains wrote:

> In D134267#3848745 , @ChuanqiXu 
> wrote:
>
>>> FAOD: I think this should not be about "who's patches" - but about the 
>>> compilation model and command lines that we want to end up with.
>>
>> BTW, in the current context, when I say "my" and "your", I just want to 
>> refer the corresponding things. There is no means to offend.
>
> sure - no problem.
>
> I guess we ought to be a bit more clear:
> There is implementation divergence between GCC and clang and from the point 
> to view of users and build systems having two different command line sets is 
> not helpful.
>
> The patches I drafted were aimed at removing that divergence.
> If that is not possible (because the ODR analysis requires steps that make it 
> very difficult or inefficient to do so) then some more thought and/or 
> restructuring is needed.

Yeah, now this is more clear.

>> In D134267#3848672 , @iains wrote:
>>
 (2) Your scheme saves the time for deserialization. However, clang 
 implement many ODR checks in the deserialization part. Skipping the 
 deserialization will omit these ODR checks, which will break the 
 semantics. And this is the reason why that scheme may not be good.
>>>
>>> What you seem to be saying here is that:
>>>
>>> `clang++ -std=c++20 foo.cxxm -c -o foo,o`
>>>
>>> edit;  maybe I should have written -xc++ foo.cxxm
>>>
>>> Does different ODR analysis from:
>>>
>>> `clang++ -std=c++20 foo.cxxm --precompile -o foo.pcm`
>>> `clang++ -std=c++20 foo.pcm -o foo.o`
>>>
>>> If so, then that is of some concern since those are both valid compilations 
>>> in the current compiler.
>>>
>>> In the first case, the back end is connected as an AST consumer to the 
>>> front - there is no serialise/deserialise.
>>
>> No. **Currently**, in clang, the following two is the same:
>>
>> `clang++ -std=c++20 foo.cxxm -c -o foo.o`
>
> That's why I added the edit ...
>
> `clang++ -std=c++20 -xc++ foo.cxxm -c -o foo.o`
>
> AFAIU that will not produce any error for the user?
>
> suppose I have foo.cxx which includes modules and has local declarations?
> `clang++ -std=c++20  foo.cxx -c -o foo.o`
>
>> with
>>
>> `clang++ -std=c++20 foo.cxxm --precompile -o foo.pcm`
>> `clang++ -std=c++20 foo.pcm -o foo.o`
>>
>> The first compilation will generate `.pcm` files in the `/tmp` directories 
>> and compile the `.pcm` files in the `/tmp` directories.
>
> yes, I should have added the -xc++ in the first case ;)

Oh, I got it. Now it makes sense. I misses the edit : (.

The answer is:
(1) Now, the behavior is different. And I once sent a bug report for it.
(2) **Now** there won't be direct error message. And I **was** planned to add 
it. This is the reason why I closed the above bug report : )
(3) If we're going to make it an error, this is not decided yet.

>> We could verify it by inserting debugging informations. Or writing a 
>> ODR-mismatch example, then when the compiler emits an error, we'll find the 
>> compiler emits the error in the deserialization part. I met these 2 
>> situations for many times : )
>>
>>> The model I implemented simply streams the AST instead of throwing it away 
>>> ...
>>
>> So that model misses the ODR checks, which is not good. Since it matters 
>> with the semantics.
>
> Understood - I wonder if we have layering right here - surely Sema should be 
> responsible for ODR checks?
> Is there any reason that relevant hashes cannot be generated on the AST 
> //**without**// streaming it?
> Was this model intended from the start ?
> (the fact that one can connect a direct AST consumer to the back end, 
> suggests that enforcing serialisation/deserialisation was not expected to be 
> a requirement at some stage)

In fact, we do the ODR checks in Sema too. (Would you feel it amazing? I felt 
amazing when I realize this at the first time)

For the following example, the ODR check is made in Deserializer:

  module;
  #include "something"
  export module any_module_name;
  import m;

or

  // decls
  import m;

or

  import m1;
  import m2;

In the above cases, the ODR check is made in deserialization part. And in the 
following examples, the ODR check is made in Sema part:

  import m;
  // decls

What is the principle? I think it is caused by the on-the-fly compilation 
principle (The name is constructed by myself. I think you know what I am 
saying.)

I mean, when we (the compiler) parse a declaration and we want to know if it 
violates the ODR, we made it in the Sema part. And when we import something 
(precisely, read something from a module, since clang did lazy loading), we did 
the ODR check in deserialization.

Although I felt odd at first, but now I feel it reasonable in some levels. How 
do you feel now?


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

https://reviews.llvm.org/D134267

_

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D134267#3848822 , @tschuett wrote:

> Clang header modules started with implicit builds and module caches. Apple is 
> moving too explicit builds. There are all kinds of problems with implicit 
> module builds. I believe that C++20 modules shouldn't make the same mistake.

Could you provide more details? Since we can't understand you if you only said 
it is bad without reasons then we can't be in the same page.

From my natural imagination, some kind of module cache is necessary. Otherwise, 
we may not be able to compile a hello world example in one command line like:

  clang++ -std=c++20 Hello.cppm main.cpp

And for a larger project, you can see the demo at: 
https://github.com/ChuanqiXu9/llvm-project/tree/MyP1689/P1689Examples. The 
cache is controlled by the build system instead of the compiler. So we are not 
moving the build system into the compiler.

My rough feeling is that you're telling us that something is bad in 100 steps 
aways. But we're in fact to move 1 step further. But let's be back in the same 
page first.


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

https://reviews.llvm.org/D134267

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D134267#3848862 , @ChuanqiXu wrote:

> In D134267#3848793 , @iains wrote:
>
>> In D134267#3848745 , @ChuanqiXu 
>> wrote:
>>
 FAOD: I think this should not be about "who's patches" - but about the 
 compilation model and command lines that we want to end up with.
>>>
>>> BTW, in the current context, when I say "my" and "your", I just want to 
>>> refer the corresponding things. There is no means to offend.
>>
>> sure - no problem.
>>
>> I guess we ought to be a bit more clear:
>> There is implementation divergence between GCC and clang and from the point 
>> to view of users and build systems having two different command line sets is 
>> not helpful.
>>
>> The patches I drafted were aimed at removing that divergence.
>> If that is not possible (because the ODR analysis requires steps that make 
>> it very difficult or inefficient to do so) then some more thought and/or 
>> restructuring is needed.
>
> Yeah, now this is more clear.
>
>>> In D134267#3848672 , @iains wrote:
>>>
> (2) Your scheme saves the time for deserialization. However, clang 
> implement many ODR checks in the deserialization part. Skipping the 
> deserialization will omit these ODR checks, which will break the 
> semantics. And this is the reason why that scheme may not be good.

 What you seem to be saying here is that:

 `clang++ -std=c++20 foo.cxxm -c -o foo,o`

 edit;  maybe I should have written -xc++ foo.cxxm

 Does different ODR analysis from:

 `clang++ -std=c++20 foo.cxxm --precompile -o foo.pcm`
 `clang++ -std=c++20 foo.pcm -o foo.o`

 If so, then that is of some concern since those are both valid 
 compilations in the current compiler.

 In the first case, the back end is connected as an AST consumer to the 
 front - there is no serialise/deserialise.
>>>
>>> No. **Currently**, in clang, the following two is the same:
>>>
>>> `clang++ -std=c++20 foo.cxxm -c -o foo.o`
>>
>> That's why I added the edit ...
>>
>> `clang++ -std=c++20 -xc++ foo.cxxm -c -o foo.o`
>>
>> AFAIU that will not produce any error for the user?
>>
>> suppose I have foo.cxx which includes modules and has local declarations?
>> `clang++ -std=c++20  foo.cxx -c -o foo.o`
>>
>>> with
>>>
>>> `clang++ -std=c++20 foo.cxxm --precompile -o foo.pcm`
>>> `clang++ -std=c++20 foo.pcm -o foo.o`
>>>
>>> The first compilation will generate `.pcm` files in the `/tmp` directories 
>>> and compile the `.pcm` files in the `/tmp` directories.
>>
>> yes, I should have added the -xc++ in the first case ;)
>
> Oh, I got it. Now it makes sense. I misses the edit : (.
>
> The answer is:
> (1) Now, the behavior is different. And I once sent a bug report for it.
> (2) **Now** there won't be direct error message. And I **was** planned to add 
> it. This is the reason why I closed the above bug report : )
> (3) If we're going to make it an error, this is not decided yet.

If we are going to **//require//** the serialisation/deserialisation for 
correctness then IMO we should have an error for an incorrect compilation.

>>> We could verify it by inserting debugging informations. Or writing a 
>>> ODR-mismatch example, then when the compiler emits an error, we'll find the 
>>> compiler emits the error in the deserialization part. I met these 2 
>>> situations for many times : )
>>>
 The model I implemented simply streams the AST instead of throwing it away 
 ...
>>>
>>> So that model misses the ODR checks, which is not good. Since it matters 
>>> with the semantics.
>>
>> Understood - I wonder if we have layering right here - surely Sema should be 
>> responsible for ODR checks?
>> Is there any reason that relevant hashes cannot be generated on the AST 
>> //**without**// streaming it?
>> Was this model intended from the start ?
>> (the fact that one can connect a direct AST consumer to the back end, 
>> suggests that enforcing serialisation/deserialisation was not expected to be 
>> a requirement at some stage)
>
> In fact, we do the ODR checks in Sema too. (Would you feel it amazing? I felt 
> amazing when I realize this at the first time)
>
> For the following example, the ODR check is made in Deserializer:
>
>   module;
>   #include "something"
>   export module any_module_name;
>   import m;
>
> or
>
>   // decls
>   import m;
>
> or
>
>   import m1;
>   import m2;
>
> In the above cases, the ODR check is made in deserialization part. And in the 
> following examples, the ODR check is made in Sema part:
>
>   import m;
>   // decls
>
> What is the principle? I think it is caused by the on-the-fly compilation 
> principle (The name is constructed by myself. I think you know what I am 
> saying.)
>
> I mean, when we (the compiler) parse a declaration and we want to know if it 
> violates the ODR,

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D134267#3848876 , @iains wrote:

> If we are going to **//require//** the serialisation/deserialisation for 
> correctness then IMO we should have an error for an incorrect compilation.

Of course.

> I still feel that deserialisation should do deserialisation, and Sema should 
> be doing the diagnoses when it tries to use a decl (in fact, I would expect 
> that to be required for correctness in the C++20 scheme, since the point of 
> use is significant).  I will defer to folks who know the history better - but 
> this seems like a layering bug to me still.

Another reason I forgot to mention is that the deserialization need to do 
merging. It is common in practice that a similar declarations lives in multiple 
module files (e.g., by GMF). Then if the deserialization needs to merge things, 
it is naturally that the deserilizer need to do ODR checks. But if you feel 
like the merging job belongs to Sema too, I think it might not be bad/wrong.


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

https://reviews.llvm.org/D134267

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D134267#3848883 , @ChuanqiXu wrote:

> In D134267#3848876 , @iains wrote:
>
>> If we are going to **//require//** the serialisation/deserialisation for 
>> correctness then IMO we should have an error for an incorrect compilation.
>
> Of course.
>
>> I still feel that deserialisation should do deserialisation, and Sema should 
>> be doing the diagnoses when it tries to use a decl (in fact, I would expect 
>> that to be required for correctness in the C++20 scheme, since the point of 
>> use is significant).  I will defer to folks who know the history better - 
>> but this seems like a layering bug to me still.
>
> Another reason I forgot to mention is that the deserialization need to do 
> merging. It is common in practice that a similar declarations lives in 
> multiple module files (e.g., by GMF). Then if the deserialization needs to 
> merge things, it is naturally that the deserilizer need to do ODR checks. But 
> if you feel like the merging job belongs to Sema too, I think it might not be 
> bad/wrong.

I would be concerned that doing this work in the deserialisation could:

- produce false positives (diagnostics) where there should be no error until an 
object is actually used.
- make it more difficult to track cases where what is merged at the point of 
use is semantically significant.

It would seem to better that the consumer of the AST should not have to know 
whether it had been round-tripped through the serialisation/deserialization 
process.

From the point of view of this diff - it is relevant to consider other 
implementations that achieve the same objective - we seem to have uncovered 
some additional questions to answer.


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

https://reviews.llvm.org/D134267

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D134267#3848958 , @iains wrote:

> In D134267#3848883 , @ChuanqiXu 
> wrote:
>
>> In D134267#3848876 , @iains wrote:
>>
>>> If we are going to **//require//** the serialisation/deserialisation for 
>>> correctness then IMO we should have an error for an incorrect compilation.
>>
>> Of course.
>>
>>> I still feel that deserialisation should do deserialisation, and Sema 
>>> should be doing the diagnoses when it tries to use a decl (in fact, I would 
>>> expect that to be required for correctness in the C++20 scheme, since the 
>>> point of use is significant).  I will defer to folks who know the history 
>>> better - but this seems like a layering bug to me still.
>>
>> Another reason I forgot to mention is that the deserialization need to do 
>> merging. It is common in practice that a similar declarations lives in 
>> multiple module files (e.g., by GMF). Then if the deserialization needs to 
>> merge things, it is naturally that the deserilizer need to do ODR checks. 
>> But if you feel like the merging job belongs to Sema too, I think it might 
>> not be bad/wrong.
>
> I would be concerned that doing this work in the deserialisation could:
>
> - produce false positives (diagnostics) where there should be no error until 
> an object is actually used.
> - make it more difficult to track cases where what is merged at the point of 
> use is semantically significant.
>
> It would seem to better that the consumer of the AST should not have to know 
> whether it had been round-tripped through the serialisation/deserialization 
> process.

The reading process of modules are lazy. So the problem is relatively 
mitigated. But I understand your concern in some level.

> From the point of view of this diff - it is relevant to consider other 
> implementations that achieve the same objective - we seem to have uncovered 
> some additional questions to answer.

I feel the main blocking issue for this diff is the concern about modules cache 
throw from @dblaikie and @tschuett that we (at least me) don't know the reason. 
I'm still waiting to more detailed reason to understand them.

And I feel like your opinion is mainly in the higher level. And I don't feel it 
is blocking this patch. Because if we're going to refactor it actually someday, 
this diff won't never be a blocker. And under the current frame, I think this 
implementation is the simplest way to achieve the same goal. So what we talked 
about is mainly about some higher level things.


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

https://reviews.llvm.org/D134267

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

[PATCH] D129156: Add -fpass-plugin option to Flang

[Mats Petersson via Phabricator via cfe-commits]

Leporacanthicus added a comment.

> @MatsPetersson & @clementval , could you share you build command so that the 
> failure can be reproduced before this re-lands?

I can't share the same command that I used, because it's some old CMAKE command 
that I no longer have in my history. But I tested this patch, and it works in 
the same tree that I was using it last week.

I just double-checked again in my release-build directory, and it also works. 
So I think the latest version works...


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

https://reviews.llvm.org/D129156

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D134267#3848974 , @ChuanqiXu wrote:

> In D134267#3848958 , @iains wrote:
>
>> In D134267#3848883 , @ChuanqiXu 
>> wrote:
>>
>>> In D134267#3848876 , @iains wrote:
>>>
 If we are going to **//require//** the serialisation/deserialisation for 
 correctness then IMO we should have an error for an incorrect compilation.
>>>
>>> Of course.
>>>
 I still feel that deserialisation should do deserialisation, and Sema 
 should be doing the diagnoses when it tries to use a decl (in fact, I 
 would expect that to be required for correctness in the C++20 scheme, 
 since the point of use is significant).  I will defer to folks who know 
 the history better - but this seems like a layering bug to me still.
>>>
>>> Another reason I forgot to mention is that the deserialization need to do 
>>> merging. It is common in practice that a similar declarations lives in 
>>> multiple module files (e.g., by GMF). Then if the deserialization needs to 
>>> merge things, it is naturally that the deserilizer need to do ODR checks. 
>>> But if you feel like the merging job belongs to Sema too, I think it might 
>>> not be bad/wrong.
>>
>> I would be concerned that doing this work in the deserialisation could:
>>
>> - produce false positives (diagnostics) where there should be no error until 
>> an object is actually used.
>> - make it more difficult to track cases where what is merged at the point of 
>> use is semantically significant.
>>
>> It would seem to better that the consumer of the AST should not have to know 
>> whether it had been round-tripped through the serialisation/deserialization 
>> process.
>
> The reading process of modules are lazy. So the problem is relatively 
> mitigated. But I understand your concern in some level.

(IMO) We ought to be able to move the ODR work to Sema (but I did not yet look 
at how hard this would be) - it seems that deserialisation should supply 
(lazily) decls in response to requests and the query side should determine if 
they are legally mergeable.  However, let's move this discussion elsewhere now 
- I think we've covered the main points.

>> From the point of view of this diff - it is relevant to consider other 
>> implementations that achieve the same objective - we seem to have uncovered 
>> some additional questions to answer.
>
> I feel the main blocking issue for this diff is the concern about modules 
> cache throw from @dblaikie and @tschuett that we (at least me) don't know the 
> reason. I'm still waiting for more detailed reason to understand them.
>
> And I feel like your opinion is mainly in the higher level. And I don't feel 
> it is blocking this patch. Because if we're going to refactor it actually 
> someday, this diff won't never be a blocker.

I think that is correct - the implementations are independent (my, so far 
unpublished, driver patches probably would require modification, but that's not 
a big deal).

> And under the current frame, I think this implementation is the simplest way 
> to achieve the same goal. So what we talked about is mainly about some higher 
> level things.

Sure, but having a high-level plan is important too :)

One specific comment about this patch; one of the key things that GCC does is 
to remove the need for the user to write a specific extension to use modular 
C++.  So from that point of view, at least, this patch does not provide a 
solution to match the command line interfaces of GCC.


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

https://reviews.llvm.org/D134267

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

[PATCH] D135657: add time traces for loading PCH files

[Trass3r via Phabricator via cfe-commits]

Trass3r created this revision.
Herald added a project: All.
Trass3r requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fills gaps in the time trace when precompiled headers are involved.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135657

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -100,6 +100,7 @@
 #include "llvm/Support/OnDiskHashTable.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SHA1.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -4484,6 +4485,7 @@
  Module *WritingModule, StringRef isysroot,
  bool hasErrors,
  bool ShouldCacheASTInMemory) {
+  llvm::TimeTraceScope scope("WriteAST", OutputFile);
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -122,6 +122,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/raw_ostream.h"
@@ -4209,6 +4210,8 @@
 SourceLocation ImportLoc,
 unsigned ClientLoadCapabilities,
 SmallVectorImpl 
*Imported) {
+  llvm::TimeTraceScope scope("ReadAST", FileName);
+
   llvm::SaveAndRestore
 SetCurImportLocRAII(CurrentImportLoc, ImportLoc);
   llvm::SaveAndRestore> SetCurModuleKindRAII(


Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -100,6 +100,7 @@
 #include "llvm/Support/OnDiskHashTable.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SHA1.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -4484,6 +4485,7 @@
  Module *WritingModule, StringRef isysroot,
  bool hasErrors,
  bool ShouldCacheASTInMemory) {
+  llvm::TimeTraceScope scope("WriteAST", OutputFile);
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -122,6 +122,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/VersionTuple.h"
 #include "llvm/Support/raw_ostream.h"
@@ -4209,6 +4210,8 @@
 SourceLocation ImportLoc,
 unsigned ClientLoadCapabilities,
 SmallVectorImpl *Imported) {
+  llvm::TimeTraceScope scope("ReadAST", FileName);
+
   llvm::SaveAndRestore
 SetCurImportLocRAII(CurrentImportLoc, ImportLoc);
   llvm::SaveAndRestore> SetCurModuleKindRAII(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 6547565 - [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[Weining Lu via cfe-commits]

Author: wanglei
Date: 2022-10-11T18:12:37+08:00
New Revision: 6547565e7bdcd9c3f683ad196b62d08c7061fdf1

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

LOG: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for 
LoongArch

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

Added: 
clang/test/CodeGen/LoongArch/atomics.c

Modified: 
clang/lib/Basic/Targets/LoongArch.h

Removed: 




diff  --git a/clang/lib/Basic/Targets/LoongArch.h 
b/clang/lib/Basic/Targets/LoongArch.h
index 5d711c6b1db4c..6a735a7d8f833 100644
--- a/clang/lib/Basic/Targets/LoongArch.h
+++ b/clang/lib/Basic/Targets/LoongArch.h
@@ -79,6 +79,9 @@ class LLVM_LIBRARY_VISIBILITY LoongArch32TargetInfo
 }
 return false;
   }
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32;
+  }
 };
 
 class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
@@ -100,6 +103,9 @@ class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
 }
 return false;
   }
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+  }
 };
 } // end namespace targets
 } // end namespace clang

diff  --git a/clang/test/CodeGen/LoongArch/atomics.c 
b/clang/test/CodeGen/LoongArch/atomics.c
new file mode 100644
index 0..edc58d30db186
--- /dev/null
+++ b/clang/test/CodeGen/LoongArch/atomics.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple loongarch32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA32
+// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA64
+
+/// This test demonstrates that MaxAtomicInlineWidth is set appropriately.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // LA32: load atomic i8, ptr %a seq_cst, align 1
+  // LA32: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA32: atomicrmw add ptr %a, i8 %b seq_cst
+  // LA64: load atomic i8, ptr %a seq_cst, align 1
+  // LA64: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA64: atomicrmw add ptr %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // LA32: load atomic i32, ptr %a seq_cst, align 4
+  // LA32: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA32: atomicrmw add ptr %a, i32 %b seq_cst
+  // LA64: load atomic i32, ptr %a seq_cst, align 4
+  // LA64: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA64: atomicrmw add ptr %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // LA32: call i64 @__atomic_load_8
+  // LA32: call void @__atomic_store_8
+  // LA32: call i64 @__atomic_fetch_add_8
+  // LA64: load atomic i64, ptr %a seq_cst, align 8
+  // LA64: store atomic i64 %b, ptr %a seq_cst, align 8
+  // LA64: atomicrmw add ptr %a, i64 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}



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

[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[Lu Weining 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 rG6547565e7bdc: [clang][LoongArch] Set MaxAtomicInlineWidth 
and MaxAtomicPromoteWidth for… (authored by wangleiat, committed by SixWeining).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135526

Files:
  clang/lib/Basic/Targets/LoongArch.h
  clang/test/CodeGen/LoongArch/atomics.c


Index: clang/test/CodeGen/LoongArch/atomics.c
===
--- /dev/null
+++ clang/test/CodeGen/LoongArch/atomics.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple loongarch32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA32
+// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA64
+
+/// This test demonstrates that MaxAtomicInlineWidth is set appropriately.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // LA32: load atomic i8, ptr %a seq_cst, align 1
+  // LA32: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA32: atomicrmw add ptr %a, i8 %b seq_cst
+  // LA64: load atomic i8, ptr %a seq_cst, align 1
+  // LA64: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA64: atomicrmw add ptr %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // LA32: load atomic i32, ptr %a seq_cst, align 4
+  // LA32: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA32: atomicrmw add ptr %a, i32 %b seq_cst
+  // LA64: load atomic i32, ptr %a seq_cst, align 4
+  // LA64: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA64: atomicrmw add ptr %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // LA32: call i64 @__atomic_load_8
+  // LA32: call void @__atomic_store_8
+  // LA32: call i64 @__atomic_fetch_add_8
+  // LA64: load atomic i64, ptr %a seq_cst, align 8
+  // LA64: store atomic i64 %b, ptr %a seq_cst, align 8
+  // LA64: atomicrmw add ptr %a, i64 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
Index: clang/lib/Basic/Targets/LoongArch.h
===
--- clang/lib/Basic/Targets/LoongArch.h
+++ clang/lib/Basic/Targets/LoongArch.h
@@ -79,6 +79,9 @@
 }
 return false;
   }
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32;
+  }
 };
 
 class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
@@ -100,6 +103,9 @@
 }
 return false;
   }
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+  }
 };
 } // end namespace targets
 } // end namespace clang


Index: clang/test/CodeGen/LoongArch/atomics.c
===
--- /dev/null
+++ clang/test/CodeGen/LoongArch/atomics.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple loongarch32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA32
+// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA64
+
+/// This test demonstrates that MaxAtomicInlineWidth is set appropriately.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // LA32: load atomic i8, ptr %a seq_cst, align 1
+  // LA32: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA32: atomicrmw add ptr %a, i8 %b seq_cst
+  // LA64: load atomic i8, ptr %a seq_cst, align 1
+  // LA64: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA64: atomicrmw add ptr %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // LA32: load atomic i32, ptr %a seq_cst, align 4
+  // LA32: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA32: atomicrmw add ptr %a, i32 %b seq_cst
+  // LA64: load atomic i32, ptr %a seq_cst, align 4
+  // LA64: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA64: atomicrmw add ptr %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // LA32: call i64 @__atomic_load_8
+  // LA32: call void @__atomic_store_8
+  // LA32: call i64 @__atomic_fetch_add_8
+  // LA64: load atomic i64, ptr %a seq_cst, align 8

[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This seems to break check-clamg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135526

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

[PATCH] D128861: [clang-tidy] add cppcoreguidelines-symmetric-binary-operator

[Julian Schmidt via Phabricator via cfe-commits]

5chmidti added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128861

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

[PATCH] D135341: [clang] adds `__reference_constructs_from_temporary`

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:1476
   is false. Note this trait will also return false when the initialization of
   ``T`` from ``U`` is ill-formed.
+* ``__reference_constructs_from_temporary(T, U)`` (C++)

royjacobson wrote:
> We should deprecate this, as `__reference_constructs_from_temporary` is 
> implemented now. There's a list of deprecated builtins in 
> SemaExprCXX.cpp:DiagnoseBuiltinDeprecation so I think it should be pretty 
> easy to add a warning for users that this is a non-standard attribute.
Though for the CI's sake maybe it's better to wait until after libcxx stop 
using it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135341

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

[clang] b32a1bd - Revert "[clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch"

[Weining Lu via cfe-commits]

Author: Weining Lu
Date: 2022-10-11T19:21:28+08:00
New Revision: b32a1bdf42b0be59d02876874b6764b4700ee7d7

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

LOG: Revert "[clang][LoongArch] Set MaxAtomicInlineWidth and 
MaxAtomicPromoteWidth for LoongArch"

This reverts commit 6547565e7bdcd9c3f683ad196b62d08c7061fdf1.

This breaks test: Preprocessor/init-loongarch.c

Added: 


Modified: 
clang/lib/Basic/Targets/LoongArch.h

Removed: 
clang/test/CodeGen/LoongArch/atomics.c



diff  --git a/clang/lib/Basic/Targets/LoongArch.h 
b/clang/lib/Basic/Targets/LoongArch.h
index 6a735a7d8f833..5d711c6b1db4c 100644
--- a/clang/lib/Basic/Targets/LoongArch.h
+++ b/clang/lib/Basic/Targets/LoongArch.h
@@ -79,9 +79,6 @@ class LLVM_LIBRARY_VISIBILITY LoongArch32TargetInfo
 }
 return false;
   }
-  void setMaxAtomicWidth() override {
-MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32;
-  }
 };
 
 class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
@@ -103,9 +100,6 @@ class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
 }
 return false;
   }
-  void setMaxAtomicWidth() override {
-MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
-  }
 };
 } // end namespace targets
 } // end namespace clang

diff  --git a/clang/test/CodeGen/LoongArch/atomics.c 
b/clang/test/CodeGen/LoongArch/atomics.c
deleted file mode 100644
index edc58d30db186..0
--- a/clang/test/CodeGen/LoongArch/atomics.c
+++ /dev/null
@@ -1,45 +0,0 @@
-// RUN: %clang_cc1 -triple loongarch32 -O1 -emit-llvm %s -o - \
-// RUN:   | FileCheck %s --check-prefix=LA32
-// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \
-// RUN:   | FileCheck %s --check-prefix=LA64
-
-/// This test demonstrates that MaxAtomicInlineWidth is set appropriately.
-
-#include 
-#include 
-
-void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
-  // LA32: load atomic i8, ptr %a seq_cst, align 1
-  // LA32: store atomic i8 %b, ptr %a seq_cst, align 1
-  // LA32: atomicrmw add ptr %a, i8 %b seq_cst
-  // LA64: load atomic i8, ptr %a seq_cst, align 1
-  // LA64: store atomic i8 %b, ptr %a seq_cst, align 1
-  // LA64: atomicrmw add ptr %a, i8 %b seq_cst
-  __c11_atomic_load(a, memory_order_seq_cst);
-  __c11_atomic_store(a, b, memory_order_seq_cst);
-  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
-}
-
-void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
-  // LA32: load atomic i32, ptr %a seq_cst, align 4
-  // LA32: store atomic i32 %b, ptr %a seq_cst, align 4
-  // LA32: atomicrmw add ptr %a, i32 %b seq_cst
-  // LA64: load atomic i32, ptr %a seq_cst, align 4
-  // LA64: store atomic i32 %b, ptr %a seq_cst, align 4
-  // LA64: atomicrmw add ptr %a, i32 %b seq_cst
-  __c11_atomic_load(a, memory_order_seq_cst);
-  __c11_atomic_store(a, b, memory_order_seq_cst);
-  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
-}
-
-void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
-  // LA32: call i64 @__atomic_load_8
-  // LA32: call void @__atomic_store_8
-  // LA32: call i64 @__atomic_fetch_add_8
-  // LA64: load atomic i64, ptr %a seq_cst, align 8
-  // LA64: store atomic i64 %b, ptr %a seq_cst, align 8
-  // LA64: atomicrmw add ptr %a, i64 %b seq_cst
-  __c11_atomic_load(a, memory_order_seq_cst);
-  __c11_atomic_store(a, b, memory_order_seq_cst);
-  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
-}



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

[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[Lu Weining via Phabricator via cfe-commits]

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

This breaks test: Preprocessor/init-loongarch.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135526

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

[PATCH] D135367: [clang-tidy] Dump effective diagnostics level in YAML output

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 466767.
DmitryPolukhin added a comment.

Added testcase for clang-tidy diagnostics as with --warnings-as-errors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135367

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


Index: clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
@@ -1,5 +1,5 @@
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t-input.cpp
-// RUN: not clang-tidy %t-input.cpp 
-checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes,clang-diagnostic-zero-length-array'
 -export-fixes=%t.yaml -- -Wmissing-prototypes -Wzero-length-array > %t.msg 2>&1
+// RUN: not clang-tidy %t-input.cpp 
-checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes,clang-diagnostic-zero-length-array'
 
--warnings-as-errors='clang-diagnostic-missing-prototypes,google-explicit-constructor'
 -export-fixes=%t.yaml -- -Wmissing-prototypes -Wzero-length-array > %t.msg 2>&1
 // RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s 
-implicit-check-not='{{warning|error|note}}:'
 // RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s
 #define X(n) void n ## n() {}
@@ -10,9 +10,10 @@
 void test(x);
 struct Foo {
   member;
+  Foo(int) {}
 };
 
-// CHECK-MESSAGES: -input.cpp:2:1: warning: no previous prototype for function 
'ff' [clang-diagnostic-missing-prototypes]
+// CHECK-MESSAGES: -input.cpp:2:1: error: no previous prototype for function 
'ff' [clang-diagnostic-missing-prototypes,-warnings-as-errors]
 // CHECK-MESSAGES: -input.cpp:1:19: note: expanded from macro 'X'
 // CHECK-MESSAGES: {{^}}note: expanded from here{{$}}
 // CHECK-MESSAGES: -input.cpp:2:1: note: declare 'static' if the function is 
not intended to be used outside of this translation unit
@@ -21,6 +22,7 @@
 // CHECK-MESSAGES: -input.cpp:4:7: warning: zero size arrays are an extension 
[clang-diagnostic-zero-length-array]
 // CHECK-MESSAGES: -input.cpp:6:11: error: unknown type name 'x' 
[clang-diagnostic-error]
 // CHECK-MESSAGES: -input.cpp:8:3: error: a type specifier is required for all 
declarations [clang-diagnostic-error]
+// CHECK-MESSAGES: -input.cpp:9:3: error: single-argument constructors must be 
marked explicit to avoid unintentional implicit conversions 
[google-explicit-constructor,-warnings-as-errors]
 
 // CHECK-YAML: ---
 // CHECK-YAML-NEXT: MainSourceFile:  '{{.*}}-input.cpp'
@@ -52,7 +54,7 @@
 // CHECK-YAML-NEXT: FilePath:'{{.*}}-input.cpp'
 // CHECK-YAML-NEXT: FileOffset:  13
 // CHECK-YAML-NEXT: Replacements:[]
-// CHECK-YAML-NEXT: Level:   Warning
+// CHECK-YAML-NEXT: Level:   Error
 // CHECK-YAML-NEXT: BuildDirectory:  '{{.*}}'
 // CHECK-YAML-NEXT:   - DiagnosticName:  clang-diagnostic-error
 // CHECK-YAML-NEXT: DiagnosticMessage:
@@ -94,4 +96,16 @@
 // CHECK-YAML-NEXT:   Replacements:[]
 // CHECK-YAML-NEXT: Level:   Error
 // CHECK-YAML-NEXT: BuildDirectory:  '{{.*}}'
+// CHECK-YAML-NEXT:   - DiagnosticName:  google-explicit-constructor
+// CHECK-YAML-NEXT: DiagnosticMessage:
+// CHECK-YAML-NEXT:   Message: single-argument constructors must 
be marked explicit to avoid unintentional implicit conversions
+// CHECK-YAML-NEXT:   FilePath:'{{.*}}-input.cpp'
+// CHECK-YAML-NEXT:   FileOffset:  96
+// CHECK-YAML-NEXT:   Replacements:
+// CHECK-YAML-NEXT: - FilePath:'{{.*}}-input.cpp'
+// CHECK-YAML-NEXT:   Offset:  96
+// CHECK-YAML-NEXT:   Length:  0
+// CHECK-YAML-NEXT:   ReplacementText: 'explicit '
+// CHECK-YAML-NEXT: Level:   Error
+// CHECK-YAML-NEXT: BuildDirectory:  '{{.*}}'
 // CHECK-YAML-NEXT: ...
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -403,6 +403,8 @@
 
 bool IsWarningAsError = DiagLevel == DiagnosticsEngine::Warning &&
 Context.treatAsError(CheckName);
+if (IsWarningAsError)
+  Level = ClangTidyError::Error;
 Errors.emplace_back(CheckName, Level, Context.getCurrentBuildDirectory(),
 IsWarningAsError);
   }


Index: clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cp

[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[wanglei via Phabricator via cfe-commits]

wangleiat added a comment.

In D135526#3849117 , @thakis wrote:

> This seems to break check-clang: http://45.33.8.238/linux/88687/step_7.txt
>
> Please take a look and revert for now if it takes a while to fix.

Thanks a lot, I'll fix it right away.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135526

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

[PATCH] D135367: [clang-tidy] Dump effective diagnostics level in YAML output

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added a comment.

In D135367#3842950 , @njames93 wrote:

> LGTM, just maybe include a test case where a warning from a clang-tidy check 
> is promoted to an error as well.

Thank you for the review!
I've added a test case for clang-tidy check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135367

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

[PATCH] D133578: [OpenMP][OMPIRBuilder] Add generation of SIMD align assumptions to OMPIRBuilder

[Dominik Adamski via Phabricator via cfe-commits]

domada updated this revision to Diff 466766.
domada edited the summary of this revision.
domada added a comment.

1. Modified generation of align assumptions. OMPIRBuilder generates now only 
assumptions calls. The arguments of the assumption calls are generated by Clang.
2. Added integration test to prove that Clang and OMPIRBuilder support aligned 
clause
3. Simplification of unit tests -> applying review remarks




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

https://reviews.llvm.org/D133578

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/irbuilder_simd_aligned.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

Index: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
===
--- mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -996,8 +996,9 @@
   if (llvm::Optional safelenVar = loop.getSafelen())
 safelen = builder.getInt64(safelenVar.value());
 
+  llvm::DenseMap alignedVars;
   ompBuilder->applySimd(
-  loopInfo,
+  loopInfo, alignedVars,
   loop.getIfExpr() ? moduleTranslation.lookupValue(loop.getIfExpr())
: nullptr,
   llvm::omp::OrderKind::OMP_ORDER_unknown, simdlen, safelen);
Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -1767,11 +1767,12 @@
 
 TEST_F(OpenMPIRBuilderTest, ApplySimd) {
   OpenMPIRBuilder OMPBuilder(*M);
-
+  DenseMap AlignedVars;
   CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
 
   // Simd-ize the loop.
-  OMPBuilder.applySimd(CLI, /* IfCond */ nullptr, OrderKind::OMP_ORDER_unknown,
+  OMPBuilder.applySimd(CLI, AlignedVars, /* IfCond */ nullptr,
+   OrderKind::OMP_ORDER_unknown,
/* Simdlen */ nullptr,
/* Safelen */ nullptr);
 
@@ -1798,13 +1799,76 @@
   }));
 }
 
-TEST_F(OpenMPIRBuilderTest, ApplySimdlen) {
+TEST_F(OpenMPIRBuilderTest, ApplySimdCustomAligned) {
   OpenMPIRBuilder OMPBuilder(*M);
+  IRBuilder<> Builder(BB);
+  const int AlignmentValue = 32;
+  AllocaInst *Alloc1 =
+  Builder.CreateAlloca(Builder.getInt8PtrTy(), Builder.getInt64(1));
+  LoadInst *Load1 = Builder.CreateLoad(Alloc1->getAllocatedType(), Alloc1);
+  DenseMap AlignedVars;
+  AlignedVars.insert({Load1, Builder.getInt64(AlignmentValue)});
+
+  CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
+
+  // Simd-ize the loop.
+  OMPBuilder.applySimd(CLI, AlignedVars, /* IfCond */ nullptr,
+   OrderKind::OMP_ORDER_unknown,
+   /* Simdlen */ nullptr,
+   /* Safelen */ nullptr);
+
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+
+  PassBuilder PB;
+  FunctionAnalysisManager FAM;
+  PB.registerFunctionAnalyses(FAM);
+  LoopInfo &LI = FAM.getResult(*F);
+
+  const std::vector &TopLvl = LI.getTopLevelLoops();
+  EXPECT_EQ(TopLvl.size(), 1u);
+
+  Loop *L = TopLvl.front();
+  EXPECT_TRUE(findStringMetadataForLoop(L, "llvm.loop.parallel_accesses"));
+  EXPECT_TRUE(getBooleanLoopAttribute(L, "llvm.loop.vectorize.enable"));
+
+  // Check for llvm.access.group metadata attached to the printf
+  // function in the loop body.
+  BasicBlock *LoopBody = CLI->getBody();
+  EXPECT_TRUE(any_of(*LoopBody, [](Instruction &I) {
+return I.getMetadata("llvm.access.group") != nullptr;
+  }));
 
+  // Check if number of assumption instructions is equal to number of aligned
+  // variables
+  BasicBlock *LoopPreheader = CLI->getPreheader();
+  size_t NumAssummptionCallsInPreheader = count_if(
+  *LoopPreheader, [](Instruction &I) { return isa(I); });
+  EXPECT_EQ(NumAssummptionCallsInPreheader, AlignedVars.size());
+
+  // Check if variables are correctly aligned
+  for (Instruction &Instr : *LoopPreheader) {
+if (!isa(Instr))
+  continue;
+AssumeInst *AssumeInstruction = cast(&Instr);
+if (AssumeInstruction->getNumTotalBundleOperands()) {
+  auto Bundle = AssumeInstruction->getOperandBundleAt(0);
+  if (Bundle.getTagName() == "align") {
+EXPECT_TRUE(isa(Bundle.Inputs[1]));
+auto ConstIntVal = dyn_cast(Bundle.Inputs[1]);
+EXPECT_EQ(ConstIntVal->getSExtValue(), AlignmentValue);
+  }
+}
+  }
+}
+TEST_F(OpenMPIRBuilderTest, ApplySimdlen) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  DenseMap AlignedVars;
   CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
 
   // Simd-ize the loop.
-  OMPBuilder.applySimd(CLI, /* IfCond */ nullptr, OrderKind::OMP_ORDER_unknown,
+  OMPBuil

[PATCH] D134589: [C++20][Modules] Elide unused guard variables in Itanium ABI module initializers.

[Nathan Sidwell via Phabricator via cfe-commits]

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

This is ok, but see the note I added about object-file compatibility.




Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:646-647
+   If there are no initalizers at all (and also no imported modules) we reduce
+   this to an empty function (since importers of the  module will call this
+   unconditionally for the current implementation).
 

It's not just 'current implementation' requirement, it's an ABI requirement.  
Remember, one could generate the interface object file from one compiler and 
then generate (and consume) the CMIs with a different compiler.  this achieves 
object-file interoperability, but does not require CMI compatibility. We have 
no expectation any particular compiler implements the optimization you refer to.

Feel free to note this at your discretion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134589

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

[PATCH] D133578: [OpenMP][OMPIRBuilder] Add generation of SIMD align assumptions to OMPIRBuilder

[Dominik Adamski via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2640
   if (UseOMPIRBuilder) {
+llvm::DenseMap AlignedVars;
 // Emit the associated statement and get its loop representation.

jdoerfert wrote:
> Where is this map populated?
I added function `GetAlignedMapping `which collects arguments for assumption 
calls. Now Clang collects all arguments required for assumption calls. 
OMPIRBuilder only inserts assumption calls.

When I was adding C integration tests I realized that it would be easier to 
implement in that way. Please let me know if this approach is ok for you.


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

https://reviews.llvm.org/D133578

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

[PATCH] D134654: [clang] Detect header loops

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:323
   "whitespace recommended after macro name">;
+def warn_include_cycle : Warning<"#include cycle">,
+   InGroup>, DefaultIgnore;

philnik wrote:
> aaron.ballman wrote:
> > urnathan wrote:
> > > aaron.ballman wrote:
> > > > This diagnostic doesn't really seem like it's going to help the user to 
> > > > know what's wrong with their code or how to fix it. (Also, the notes 
> > > > @erichkeane was hoping were emitted don't seem to be, at least 
> > > > according to the new test cases.) I think we need to give users a bit 
> > > > more guidance here.
> > > The test infrastructire ignores the 'included from ' chain, which is 
> > > emitted.  What do you suggest?
> > Even posting the "included from" doesn't help clarify what's wrong with the 
> > code though, if I'm understanding the tests properly. My concern is with 
> > code like:
> > ```
> > // self.h
> > #ifndef GUARD
> > #define GUARD
> > 
> > #include "self.h" // expected-warning {{#include cycle}}
> > #endif
> > ```
> > (This is effectively the same test as `warn-loop-macro-1.h`.) There is no 
> > include *cycle* ("a series of events that are regularly repeated in the 
> > same order.") in this code -- the header is included for the first time, 
> > `GUARD` is not defined, then `GUARD` is defined and the header is included 
> > for the second time. This time `GUARD` is defined and no further cycles 
> > into `self.h` occurs.
> > 
> > But I think I'm seeing now what you're trying to get at (correct me if I'm 
> > wrong): when processing an include stack, opening the same header file 
> > twice is a problem (regardless of the contents of the header or how you got 
> > into the same file twice)? If that's accurate, would it make more sense to 
> > diagnose this as:
> > 
> > `including the same header (%0) more than once in an include stack causes 
> > %select{incorrect behavior of Clang modules|the header to not be a C++ 
> > module header unit}1`
> > 
> > or something along those lines? Basically, I'd like to avoid saying "cycle" 
> > because that starts to sound like "you have potentially infinite recursion 
> > in the include stack" and I'd like to add information about what's actually 
> > wrong instead of pointing out what the code does and hoping the user knows 
> > why that's bad.
> FWIW in libc++ we have a script to check that we don't include the header 
> more than once in a stack and also call it a cycle, so there is some 
> precedent of calling it an include cycle, even thought it's technically not a 
> cycle.
Aaron, correct, the problem is with clang-modules/c++ header units.  The loop 
means that the header file does not expand to the same sequence of tokens at 
every inclusion.  This is particularly problematic with module maps, because 
the compiler will automatically start turning a header into a clang-module upon 
the first #include -- which might not be at the outermost level, (if it starts 
with a header at some other position in the loop).  Then when it starts with 
this header, it'll think it already has converted and loaded it.  One ends up 
with very obscure failure modes (such as link errors concerning missing 
template instantations).

I wonder if (in addition to allow detecting this in general), the warning 
should be enabled by default when building a header-unit/clang-module and one 
reincludes a header corresponding to a module currently being built.  That's 
the same check, because there can be a stack of modules being built, but I 
think we need some additional checking to avoid triggering on 'textual headers' 
from the module map.

IMHO it is still a cycle, it's a closed loop that executes exactly once :)  I 
can see the confusion though.  I mean, 'for (auto i = 0; i < 1; i++) {}' is 
still a loop, right?  just not an unbounded one. 

 Anyway, I think your diagagnostic text is better, thanks!



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:323
   "whitespace recommended after macro name">;
+def warn_include_cycle : Warning<"#include cycle">,
+   InGroup>, DefaultIgnore;

urnathan wrote:
> philnik wrote:
> > aaron.ballman wrote:
> > > urnathan wrote:
> > > > aaron.ballman wrote:
> > > > > This diagnostic doesn't really seem like it's going to help the user 
> > > > > to know what's wrong with their code or how to fix it. (Also, the 
> > > > > notes @erichkeane was hoping were emitted don't seem to be, at least 
> > > > > according to the new test cases.) I think we need to give users a bit 
> > > > > more guidance here.
> > > > The test infrastructire ignores the 'included from ' chain, which is 
> > > > emitted.  What do you suggest?
> > > Even posting the "included from" doesn't help clarify what's wrong with 
> > > the code though, if I'm understanding the tests properly. My concern is 
> > > with code like:
> > > ```
> > > // self.h
> > > #ifndef 

[PATCH] D133578: [OpenMP][OMPIRBuilder] Add generation of SIMD align assumptions to OMPIRBuilder

[Dominik Adamski via Phabricator via cfe-commits]

domada updated this revision to Diff 466774.
domada marked an inline comment as done.
domada added a comment.

1. Update description of AlignedVars argument


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

https://reviews.llvm.org/D133578

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/irbuilder_simd_aligned.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

Index: mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
===
--- mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -996,8 +996,9 @@
   if (llvm::Optional safelenVar = loop.getSafelen())
 safelen = builder.getInt64(safelenVar.value());
 
+  llvm::DenseMap alignedVars;
   ompBuilder->applySimd(
-  loopInfo,
+  loopInfo, alignedVars,
   loop.getIfExpr() ? moduleTranslation.lookupValue(loop.getIfExpr())
: nullptr,
   llvm::omp::OrderKind::OMP_ORDER_unknown, simdlen, safelen);
Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -1767,11 +1767,12 @@
 
 TEST_F(OpenMPIRBuilderTest, ApplySimd) {
   OpenMPIRBuilder OMPBuilder(*M);
-
+  DenseMap AlignedVars;
   CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
 
   // Simd-ize the loop.
-  OMPBuilder.applySimd(CLI, /* IfCond */ nullptr, OrderKind::OMP_ORDER_unknown,
+  OMPBuilder.applySimd(CLI, AlignedVars, /* IfCond */ nullptr,
+   OrderKind::OMP_ORDER_unknown,
/* Simdlen */ nullptr,
/* Safelen */ nullptr);
 
@@ -1798,13 +1799,76 @@
   }));
 }
 
-TEST_F(OpenMPIRBuilderTest, ApplySimdlen) {
+TEST_F(OpenMPIRBuilderTest, ApplySimdCustomAligned) {
   OpenMPIRBuilder OMPBuilder(*M);
+  IRBuilder<> Builder(BB);
+  const int AlignmentValue = 32;
+  AllocaInst *Alloc1 =
+  Builder.CreateAlloca(Builder.getInt8PtrTy(), Builder.getInt64(1));
+  LoadInst *Load1 = Builder.CreateLoad(Alloc1->getAllocatedType(), Alloc1);
+  DenseMap AlignedVars;
+  AlignedVars.insert({Load1, Builder.getInt64(AlignmentValue)});
+
+  CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
+
+  // Simd-ize the loop.
+  OMPBuilder.applySimd(CLI, AlignedVars, /* IfCond */ nullptr,
+   OrderKind::OMP_ORDER_unknown,
+   /* Simdlen */ nullptr,
+   /* Safelen */ nullptr);
+
+  OMPBuilder.finalize();
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+
+  PassBuilder PB;
+  FunctionAnalysisManager FAM;
+  PB.registerFunctionAnalyses(FAM);
+  LoopInfo &LI = FAM.getResult(*F);
+
+  const std::vector &TopLvl = LI.getTopLevelLoops();
+  EXPECT_EQ(TopLvl.size(), 1u);
+
+  Loop *L = TopLvl.front();
+  EXPECT_TRUE(findStringMetadataForLoop(L, "llvm.loop.parallel_accesses"));
+  EXPECT_TRUE(getBooleanLoopAttribute(L, "llvm.loop.vectorize.enable"));
+
+  // Check for llvm.access.group metadata attached to the printf
+  // function in the loop body.
+  BasicBlock *LoopBody = CLI->getBody();
+  EXPECT_TRUE(any_of(*LoopBody, [](Instruction &I) {
+return I.getMetadata("llvm.access.group") != nullptr;
+  }));
 
+  // Check if number of assumption instructions is equal to number of aligned
+  // variables
+  BasicBlock *LoopPreheader = CLI->getPreheader();
+  size_t NumAssummptionCallsInPreheader = count_if(
+  *LoopPreheader, [](Instruction &I) { return isa(I); });
+  EXPECT_EQ(NumAssummptionCallsInPreheader, AlignedVars.size());
+
+  // Check if variables are correctly aligned
+  for (Instruction &Instr : *LoopPreheader) {
+if (!isa(Instr))
+  continue;
+AssumeInst *AssumeInstruction = cast(&Instr);
+if (AssumeInstruction->getNumTotalBundleOperands()) {
+  auto Bundle = AssumeInstruction->getOperandBundleAt(0);
+  if (Bundle.getTagName() == "align") {
+EXPECT_TRUE(isa(Bundle.Inputs[1]));
+auto ConstIntVal = dyn_cast(Bundle.Inputs[1]);
+EXPECT_EQ(ConstIntVal->getSExtValue(), AlignmentValue);
+  }
+}
+  }
+}
+TEST_F(OpenMPIRBuilderTest, ApplySimdlen) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  DenseMap AlignedVars;
   CanonicalLoopInfo *CLI = buildSingleLoopFunction(DL, OMPBuilder, 32);
 
   // Simd-ize the loop.
-  OMPBuilder.applySimd(CLI, /* IfCond */ nullptr, OrderKind::OMP_ORDER_unknown,
+  OMPBuilder.applySimd(CLI, AlignedVars,
+   /* IfCond */ nullptr, OrderKind::OMP_ORDER_unknown,
ConstantInt::get(Type::getInt32Ty(Ctx), 3),
/* Safelen */ nullptr);
 
@@ -1834,12 +1898,13 @@
 
 TEST_F(Open

[PATCH] D133578: [OpenMP][OMPIRBuilder] Add generation of SIMD align assumptions to OMPIRBuilder

[Dominik Adamski via Phabricator via cfe-commits]

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



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:1871
+  }
+}
+TEST_F(OpenMPIRBuilderTest, ApplySimdlen) {

jdoerfert wrote:
> This doesn't actually test that 2 assumes with alignments have been found, 
> one for each alloca.
The latest change moved `alloca` handling to the Clang. That's why it is not 
necessary to check it here.


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

https://reviews.llvm.org/D133578

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

[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[wanglei via Phabricator via cfe-commits]

wangleiat updated this revision to Diff 466775.
wangleiat added a comment.

Fix `Preprocessor/init-loongarch.c` test failed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135526

Files:
  clang/lib/Basic/Targets/LoongArch.h
  clang/test/CodeGen/LoongArch/atomics.c
  clang/test/Preprocessor/init-loongarch.c

Index: clang/test/Preprocessor/init-loongarch.c
===
--- clang/test/Preprocessor/init-loongarch.c
+++ clang/test/Preprocessor/init-loongarch.c
@@ -30,16 +30,16 @@
 // LA32: #define __CHAR16_TYPE__ unsigned short
 // LA32: #define __CHAR32_TYPE__ unsigned int
 // LA32: #define __CHAR_BIT__ 8
-// LA32: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_INT_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_INT_LOCK_FREE 2
 // LA32: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_LONG_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_LONG_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 2
 // LA32: #define __DBL_DECIMAL_DIG__ 17
 // LA32: #define __DBL_DENORM_MIN__ 4.9406564584124654e-324
 // LA32: #define __DBL_DIG__ 15
@@ -70,17 +70,17 @@
 // LA32: #define __FLT_MIN_EXP__ (-125)
 // LA32: #define __FLT_MIN__ 1.17549435e-38F
 // LA32: #define __FLT_RADIX__ 2
-// LA32: #define __GCC_ATOMIC_BOOL_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_CHAR_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_INT_LOCK_FREE 1
+// LA32: #define __GCC_ATOMIC_BOOL_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_CHAR_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_INT_LOCK_FREE 2
 // LA32: #define __GCC_ATOMIC_LLONG_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_LONG_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_POINTER_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_SHORT_LOCK_FREE 1
+// LA32: #define __GCC_ATOMIC_LONG_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_POINTER_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_SHORT_LOCK_FREE 2
 // LA32: #define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1
-// LA32: #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 1
+// LA32: #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
 // LA32: #define __ILP32__ 1
 // LA32: #define __INT16_C_SUFFIX__
 // LA32: #define __INT16_FMTd__ "hd"
@@ -342,16 +342,16 @@
 // LA64: #define __CHAR16_TYPE__ unsigned short
 // LA64: #define __CHAR32_TYPE__ unsigned int
 // LA64: #define __CHAR_BIT__ 8
-// LA64: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_INT_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_LONG_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 1
+// LA64: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_INT_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_LONG_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 2
 // LA64: #define __DBL_DECIMAL_DIG__ 17
 // LA64: #define __DBL_DENORM_MIN__ 4.9406564584124654e-324
 // LA64: #define __DBL_DIG__ 15
@@ -382,17 +382,17 @@
 // LA64: #define __FLT_MIN_EXP__ (-125)
 // LA64: #define __FLT_MIN__ 1.17549435e-38F
 // LA64: #define __FLT_RADIX__ 2
-// LA64: #define __GCC_ATOMIC_BOOL_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_CHAR_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_INT_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_LLONG_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_LONG_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_POINTER_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_SHORT_L

[PATCH] D134475: Add C++11 attribute msvc::constexpr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: cpplearner.
aaron.ballman added a comment.

Pulling a comment out of https://reviews.llvm.org/D133853 that's relevant here:

In D133853#3844851 , @cpplearner 
wrote:

> In D133853#3808674 , @aaron.ballman 
> wrote:
>
>> Now I'm wondering why the attribute exists at all. If it's functionally 
>> equivalent to `constexpr` as a keyword, what are the use cases for the 
>> attribute?
>
> It appears that `[[msvc::constexpr]]` does not make a function `constexpr`, 
> but if `[[msvc::constexpr]]` is used in a function definition //and// in a 
> call to that function, then the annotated function call can be evaluated 
> during constant evaluation: https://godbolt.org/z/3MPTsz6Yn
>
> Apparently this is used to implement constexpr `std::construct_at`, which 
> needs to call placement `operator new`, but the latter is not `constexpr`.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

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

[clang] 42b7079 - Reland "[Clang][LoongArch] Add inline asm support for constraints k/m/ZB/ZC"

[Weining Lu via cfe-commits]

Author: Weining Lu
Date: 2022-10-11T19:51:48+08:00
New Revision: 42b70793a1df473be9c78b4141d3f3cedcbac988

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

LOG: Reland "[Clang][LoongArch] Add inline asm support for constraints 
k/m/ZB/ZC"

Reference: https://gcc.gnu.org/onlinedocs/gccint/Machine-Constraints.html

k: A memory operand whose address is formed by a base register and
(optionally scaled) index register.

m: A memory operand whose address is formed by a base register and
offset that is suitable for use in instructions with the same
addressing mode as st.w and ld.w.

ZB: An address that is held in a general-purpose register. The offset
is zero.

ZC: A memory operand whose address is formed by a base register and
offset that is suitable for use in instructions with the same
addressing mode as ll.w and sc.w.

Note:
The INLINEASM SDNode flags in below tests are updated because the new
introduced enum `Constraint_k` is added before `Constraint_m`.
  llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-inline-asm.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-inline-asm.ll
  llvm/test/CodeGen/X86/callbr-asm-kill.mir

This patch passes `ninja check-all` on a X86 machine with all official
targets and the LoongArch target enabled.

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

Added: 
llvm/test/CodeGen/LoongArch/inline-asm-constraint-ZB.ll
llvm/test/CodeGen/LoongArch/inline-asm-constraint-ZC.ll
llvm/test/CodeGen/LoongArch/inline-asm-constraint-k.ll
llvm/test/CodeGen/LoongArch/inline-asm-constraint-m.ll

Modified: 
clang/lib/Basic/Targets/LoongArch.cpp
clang/lib/Basic/Targets/LoongArch.h
clang/test/CodeGen/LoongArch/inline-asm-constraints.c
llvm/include/llvm/IR/InlineAsm.h
llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp
llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h
llvm/lib/Target/LoongArch/LoongArchISelDAGToDAG.cpp
llvm/lib/Target/LoongArch/LoongArchISelDAGToDAG.h
llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
llvm/lib/Target/LoongArch/LoongArchISelLowering.h
llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-inline-asm.ll
llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-inline-asm.ll
llvm/test/CodeGen/X86/callbr-asm-kill.mir

Removed: 




diff  --git a/clang/lib/Basic/Targets/LoongArch.cpp 
b/clang/lib/Basic/Targets/LoongArch.cpp
index cc93206dad686..1ad9c0fb8b495 100644
--- a/clang/lib/Basic/Targets/LoongArch.cpp
+++ b/clang/lib/Basic/Targets/LoongArch.cpp
@@ -67,14 +67,19 @@ bool LoongArchTargetInfo::validateAsmConstraint(
 const char *&Name, TargetInfo::ConstraintInfo &Info) const {
   // See the GCC definitions here:
   // https://gcc.gnu.org/onlinedocs/gccint/Machine-Constraints.html
+  // Note that the 'm' constraint is handled in TargetInfo.
   switch (*Name) {
-  // TODO: handle 'k', 'm', "ZB", "ZC".
   default:
 return false;
   case 'f':
 // A floating-point register (if available).
 Info.setAllowsRegister();
 return true;
+  case 'k':
+// A memory operand whose address is formed by a base register and
+// (optionally scaled) index register.
+Info.setAllowsMemory();
+return true;
   case 'l':
 // A signed 16-bit constant.
 Info.setRequiresImmediate(-32768, 32767);
@@ -87,7 +92,36 @@ bool LoongArchTargetInfo::validateAsmConstraint(
 // An unsigned 12-bit constant (for logic instructions).
 Info.setRequiresImmediate(0, 4095);
 return true;
+  case 'Z':
+// ZB: An address that is held in a general-purpose register. The offset is
+// zero.
+// ZC: A memory operand whose address is formed by a base register
+// and offset that is suitable for use in instructions with the same
+// addressing mode as ll.w and sc.w.
+if (Name[1] == 'C' || Name[1] == 'B') {
+  Info.setAllowsMemory();
+  ++Name; // Skip over 'Z'.
+  return true;
+}
+return false;
+  }
+}
+
+std::string
+LoongArchTargetInfo::convertConstraint(const char *&Constraint) const {
+  std::string R;
+  switch (*Constraint) {
+  case 'Z':
+// "ZC"/"ZB" are two-character constraints; add "^" hint for later
+// parsing.
+R = "^" + std::string(Constraint, 2);
+++Constraint;
+break;
+  default:
+R = TargetInfo::convertConstraint(Constraint);
+break;
   }
+  return R;
 }
 
 void LoongArchTargetInfo::getTargetDefines(const LangOptions &Opts,

diff  --git a/clang/lib/Basic/Targets/LoongArch.h 
b/clang/lib/Basic/Targets/LoongArch.h
index 5d711c6b1db4c..47ee2fc737752 100644
--- a/clang/lib/Basic/Targets/LoongArch.h
+++ b/clang/lib/Basic/Targets/LoongArch.h
@@ -55,6 +55,7 @@ class LLVM_LIBRARY_VISIBILITY LoongArchTargetInfo : public 
TargetInfo {
 
   bool validateAsmConstraint(const char *&Name,
   

[PATCH] D134638: [Clang][LoongArch] Add inline asm support for constraints k/m/ZB/ZC

[Lu Weining via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG42b70793a1df: Reland "[Clang][LoongArch] Add inline asm 
support for constraints k/m/ZB/ZC" (authored by SixWeining).

Changed prior to commit:
  https://reviews.llvm.org/D134638?vs=463847&id=466778#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134638

Files:
  clang/lib/Basic/Targets/LoongArch.cpp
  clang/lib/Basic/Targets/LoongArch.h
  clang/test/CodeGen/LoongArch/inline-asm-constraints.c
  llvm/include/llvm/IR/InlineAsm.h
  llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp
  llvm/lib/Target/LoongArch/LoongArchAsmPrinter.h
  llvm/lib/Target/LoongArch/LoongArchISelDAGToDAG.cpp
  llvm/lib/Target/LoongArch/LoongArchISelDAGToDAG.h
  llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
  llvm/lib/Target/LoongArch/LoongArchISelLowering.h
  llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-inline-asm.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-inline-asm.ll
  llvm/test/CodeGen/LoongArch/inline-asm-constraint-ZB.ll
  llvm/test/CodeGen/LoongArch/inline-asm-constraint-ZC.ll
  llvm/test/CodeGen/LoongArch/inline-asm-constraint-k.ll
  llvm/test/CodeGen/LoongArch/inline-asm-constraint-m.ll
  llvm/test/CodeGen/X86/callbr-asm-kill.mir

Index: llvm/test/CodeGen/X86/callbr-asm-kill.mir
===
--- llvm/test/CodeGen/X86/callbr-asm-kill.mir
+++ llvm/test/CodeGen/X86/callbr-asm-kill.mir
@@ -67,7 +67,7 @@
   ; CHECK-NEXT:   CALL64pcrel32 target-flags(x86-plt) @foo, csr_64, implicit $rsp, implicit $ssp, implicit killed $rdi, implicit-def $rsp, implicit-def $ssp
   ; CHECK-NEXT:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
   ; CHECK-NEXT:   [[COPY4:%[0-9]+]]:gr64 = COPY [[MOV64rm]]
-  ; CHECK-NEXT:   INLINEASM_BR &"", 9 /* sideeffect mayload attdialect */, 196654 /* mem:m */, killed [[MOV64rm]], 1, $noreg, 0, $noreg, 13 /* imm */, blockaddress(@test1, %ir-block.loop)
+  ; CHECK-NEXT:   INLINEASM_BR &"", 9 /* sideeffect mayload attdialect */, 262190 /* mem:m */, killed [[MOV64rm]], 1, $noreg, 0, $noreg, 13 /* imm */, blockaddress(@test1, %ir-block.loop)
   ; CHECK-NEXT:   JMP_1 %bb.2
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.2.end:
@@ -87,7 +87,7 @@
 $rdi = COPY killed %0
 CALL64pcrel32 target-flags(x86-plt) @foo, csr_64, implicit $rsp, implicit $ssp, implicit killed $rdi, implicit-def $rsp, implicit-def $ssp
 ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
-INLINEASM_BR &"", 9 /* sideeffect mayload attdialect */, 196654 /* mem:m */, %1, 1, $noreg, 0, $noreg, 13 /* imm */, blockaddress(@test1, %ir-block.loop)
+INLINEASM_BR &"", 9 /* sideeffect mayload attdialect */, 262190 /* mem:m */, %1, 1, $noreg, 0, $noreg, 13 /* imm */, blockaddress(@test1, %ir-block.loop)
 JMP_1 %bb.2
 
   bb.2.end:
Index: llvm/test/CodeGen/LoongArch/inline-asm-constraint-m.ll
===
--- /dev/null
+++ llvm/test/CodeGen/LoongArch/inline-asm-constraint-m.ll
@@ -0,0 +1,145 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc --mtriple=loongarch32 --verify-machineinstrs < %s | FileCheck %s --check-prefix=LA32
+; RUN: llc --mtriple=loongarch64 --verify-machineinstrs < %s | FileCheck %s --check-prefix=LA64
+
+define i32 @m_offset_neg_2049(ptr %p) nounwind {
+; LA32-LABEL: m_offset_neg_2049:
+; LA32:   # %bb.0:
+; LA32-NEXT:lu12i.w $a1, -1
+; LA32-NEXT:ori $a1, $a1, 2047
+; LA32-NEXT:add.w $a0, $a0, $a1
+; LA32-NEXT:#APP
+; LA32-NEXT:ld.w $a0, $a0, 0
+; LA32-NEXT:#NO_APP
+; LA32-NEXT:ret
+;
+; LA64-LABEL: m_offset_neg_2049:
+; LA64:   # %bb.0:
+; LA64-NEXT:lu12i.w $a1, -1
+; LA64-NEXT:ori $a1, $a1, 2047
+; LA64-NEXT:add.d $a0, $a0, $a1
+; LA64-NEXT:#APP
+; LA64-NEXT:ld.w $a0, $a0, 0
+; LA64-NEXT:#NO_APP
+; LA64-NEXT:ret
+  %1 = getelementptr inbounds i8, ptr %p, i32 -2049
+  %2 = call i32 asm "ld.w $0, $1", "=r,*m"(ptr elementtype(i32) %1)
+  ret i32 %2
+}
+
+define i32 @m_offset_neg_2048(ptr %p) nounwind {
+; LA32-LABEL: m_offset_neg_2048:
+; LA32:   # %bb.0:
+; LA32-NEXT:#APP
+; LA32-NEXT:ld.w $a0, $a0, -2048
+; LA32-NEXT:#NO_APP
+; LA32-NEXT:ret
+;
+; LA64-LABEL: m_offset_neg_2048:
+; LA64:   # %bb.0:
+; LA64-NEXT:#APP
+; LA64-NEXT:ld.w $a0, $a0, -2048
+; LA64-NEXT:#NO_APP
+; LA64-NEXT:ret
+  %1 = getelementptr inbounds i8, ptr %p, i32 -2048
+  %2 = call i32 asm "ld.w $0, $1", "=r,*m"(ptr elementtype(i32) %1)
+  ret i32 %2
+}
+
+define i32 @m_offset_neg_1(ptr %p) nounwind {
+; LA32-LABE

[PATCH] D134475: Add C++11 attribute msvc::constexpr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134475#3829583 , @RIscRIpt wrote:

>> TODO: I think, I'll need to read more about constexpr for functions in 
>> standard (and LLVM code), to add relevant restrictions here.
>
> In the standard I was able to find the following paragraphs about `constexpr`:
>
> 1. Neither constructor nor destructor can be constexpr if the class has some 
> virtual base class; this holds for `[[msvc::constexpr]]` in MSVC 14.33
> 2. `constexpr` is not a part of function signature, so same non-constexpr 
> function cannot be defined. The same holds for C++11 attributes, and 
> `[[msvc::constexpr]]` in MSVC 14.33
>
> I will add two relevant tests in `msvc-attrs-invalid.cpp`

Good, thank you!

> Regarding LLVM code, I made the following observations:
>
> - `FunctionDecl::isConstexprSpecified()` is mostly used for printing code, in 
> other words, to check that the function is `constexpr` (and not `consteval`).

Sort of. A templated function marked with `constexpr` remains a constexpr 
function even if it can't be executed at compile time, so 
`isConstexprSpecified()` is really telling you "did the user write `constexpr` 
on this declaration?" instead of "is this function really constexpr?" 
(http://eel.is/c++draft/dcl.constexpr#4)

> - `FunctionDecl::getConstexprKind()` is used by:
> - `ASTImporter.cpp` - seems that, it should take care of attributes 
> independently from the `getConstexprKind()`
> - `ASTWriterDecl.cpp` - seems that, it should take care of attributes 
> independently from the `getConstexprKind()`
> - `SemaDecl.cpp` / `SemaDeclCXX.cpp` - checks that re-declaration was not 
> performed with different constexpr specifier. MSVC and clang with current 
> implementation allows re-declaration with different attributes (there are 
> tests with `f2-f5` in `msvc-attrs.cpp`)
> - `SemaDeclCXX` - enables C++ constexpr constructor inheritance - problem - 
> my current implementation supports it, whereas MSVC doesn't:
>
>   // Check '[[msvc::constexpr]]' is not taken into account during constructor 
> inheritance
>   struct s2 {
>   [[msvc::constexpr]] s2() {}
>   constexpr operator bool() { return false; };
>   };
>   
>   struct s3 : s2 {
>   constexpr operator bool() { return true; };
>   };
>   static_assert(s3()); // MSVC: C2131: expression did not evaluate to a 
> constant
>   static_assert(s3()); // clang with my patch: OK

Hmmm, that's interesting! I wonder if that's intentional behavior for MSVC or a 
bug. I'm guessing there are no open issues on Microsoft's bug tracker because 
this isn't a documented feature? Might be worth filing one to see what they 
think though. (Might not be worth it either, see below.)

> - `SemaTemplate.cpp` - enables C++ template re-specialization with different 
> constexpr specifier, not a problem, because we are allowed to do the same 
> with attributes (don't we?)

Shake a magic 8-ball, unfortunately. There's a core issue open currently about 
template specializations and attributes: http://wg21.link/cwg2604

> - `SemaTemplateInstantiateDecl.cpp` (`TemplateDeclInstantiator`) - enables 
> C++ template instantiation with same constexpr specifier
>
>   template
>   struct s1 {
>   [[msvc::constexpr]] s1() {}
>   constexpr operator bool() { return true; };
>   };
>   
>   static_assert(s1()); // MSVC: C2131: expression did not evaluate to a 
> constant
>   static_assert(s1()); // clang with my patch: OK

Can you do: `constexpr s1 s;` in MSVC or does that also fail? (e.g., is 
the issue with the constructor or with the conversion operator?)

> - `TreeTransform.h` - code is related to lambdas - out-of-scope of the 
> current patch.
>
> Regarding `MSVC:C2131` I found one more problem:
>
>   [[msvc::constexpr]] int C() { return 0; }
>   constexpr int V = C(); // MSVC: C2131: expression did not evaluate to a 
> constant
>   constexpr int W = C(); // clang with my patch: OK

What the heck, that's a surprise! I would have expected that to work the same 
in Clang and MSVC (accepted by both).

> I am starting to question my patch - does LLVM really need it, if
>
> 1. There's no documentation for `[[msvc::constexpr]]`
> 2. The trivial implementation makes it an alias, but the semantic behavior is 
> different - we could fix that, but there are lots of things to check
>
> @aaron.ballman WDYT?
>
> P.S. I'd rather abandon it, than trying to match behavior of MSVC.

If you don't want to work on the feature, that's totally fine (you're under no 
obligation to finish this if it turns out to be more than you wanted to take 
on)! I think we're going to need to support this attribute at some point 
because `` has `#define _MSVC_CONSTEXPR [[msvc::constexpr]]` and 
that macro is used by the STL headers. But the usage I am seeing there is even 
more interesting than what we've discovered so far. From ``:

  constexpr _Ty* operator()(_Ty* _Location, _Types&&... _Args) const
  noexcept(noexcept(::new (const_cast(st

[PATCH] D135526: [clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch

[Lu Weining 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 rGdefe7c07f0db: Reland "[clang][LoongArch] Set 
MaxAtomicInlineWidth and MaxAtomicPromoteWidth… (authored by wangleiat, 
committed by SixWeining).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135526

Files:
  clang/lib/Basic/Targets/LoongArch.h
  clang/test/CodeGen/LoongArch/atomics.c
  clang/test/Preprocessor/init-loongarch.c

Index: clang/test/Preprocessor/init-loongarch.c
===
--- clang/test/Preprocessor/init-loongarch.c
+++ clang/test/Preprocessor/init-loongarch.c
@@ -30,16 +30,16 @@
 // LA32: #define __CHAR16_TYPE__ unsigned short
 // LA32: #define __CHAR32_TYPE__ unsigned int
 // LA32: #define __CHAR_BIT__ 8
-// LA32: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_INT_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_INT_LOCK_FREE 2
 // LA32: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_LONG_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_LONG_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 2
 // LA32: #define __DBL_DECIMAL_DIG__ 17
 // LA32: #define __DBL_DENORM_MIN__ 4.9406564584124654e-324
 // LA32: #define __DBL_DIG__ 15
@@ -70,17 +70,17 @@
 // LA32: #define __FLT_MIN_EXP__ (-125)
 // LA32: #define __FLT_MIN__ 1.17549435e-38F
 // LA32: #define __FLT_RADIX__ 2
-// LA32: #define __GCC_ATOMIC_BOOL_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_CHAR_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_INT_LOCK_FREE 1
+// LA32: #define __GCC_ATOMIC_BOOL_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_CHAR_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_INT_LOCK_FREE 2
 // LA32: #define __GCC_ATOMIC_LLONG_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_LONG_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_POINTER_LOCK_FREE 1
-// LA32: #define __GCC_ATOMIC_SHORT_LOCK_FREE 1
+// LA32: #define __GCC_ATOMIC_LONG_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_POINTER_LOCK_FREE 2
+// LA32: #define __GCC_ATOMIC_SHORT_LOCK_FREE 2
 // LA32: #define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1
-// LA32: #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 1
+// LA32: #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 2
 // LA32: #define __ILP32__ 1
 // LA32: #define __INT16_C_SUFFIX__
 // LA32: #define __INT16_FMTd__ "hd"
@@ -342,16 +342,16 @@
 // LA64: #define __CHAR16_TYPE__ unsigned short
 // LA64: #define __CHAR32_TYPE__ unsigned int
 // LA64: #define __CHAR_BIT__ 8
-// LA64: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_INT_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_LONG_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 1
-// LA64: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 1
+// LA64: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_INT_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_LONG_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 2
+// LA64: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 2
 // LA64: #define __DBL_DECIMAL_DIG__ 17
 // LA64: #define __DBL_DENORM_MIN__ 4.9406564584124654e-324
 // LA64: #define __DBL_DIG__ 15
@@ -382,17 +382,17 @@
 // LA64: #define __FLT_MIN_EXP__ (-125)
 // LA64: #define __FLT_MIN__ 1.17549435e-38F
 // LA64: #define __FLT_RADIX__ 2
-// LA64: #define __GCC_ATOMIC_BOOL_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_CHAR_LOCK_FREE 1
-// LA64: #define __GCC_ATOMIC_INT_LOCK_FREE 1
-// LA64: #define 

[clang] defe7c0 - Reland "[clang][LoongArch] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for LoongArch"

[Weining Lu via cfe-commits]

Author: wanglei
Date: 2022-10-11T20:36:09+08:00
New Revision: defe7c07f0db9d18a59087dd7aaf7becb38595b2

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

LOG: Reland "[clang][LoongArch] Set MaxAtomicInlineWidth and 
MaxAtomicPromoteWidth for LoongArch"

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

Added: 
clang/test/CodeGen/LoongArch/atomics.c

Modified: 
clang/lib/Basic/Targets/LoongArch.h
clang/test/Preprocessor/init-loongarch.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/LoongArch.h 
b/clang/lib/Basic/Targets/LoongArch.h
index 47ee2fc737752..09d3290362f65 100644
--- a/clang/lib/Basic/Targets/LoongArch.h
+++ b/clang/lib/Basic/Targets/LoongArch.h
@@ -80,6 +80,9 @@ class LLVM_LIBRARY_VISIBILITY LoongArch32TargetInfo
 }
 return false;
   }
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32;
+  }
 };
 
 class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
@@ -101,6 +104,9 @@ class LLVM_LIBRARY_VISIBILITY LoongArch64TargetInfo
 }
 return false;
   }
+  void setMaxAtomicWidth() override {
+MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
+  }
 };
 } // end namespace targets
 } // end namespace clang

diff  --git a/clang/test/CodeGen/LoongArch/atomics.c 
b/clang/test/CodeGen/LoongArch/atomics.c
new file mode 100644
index 0..edc58d30db186
--- /dev/null
+++ b/clang/test/CodeGen/LoongArch/atomics.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple loongarch32 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA32
+// RUN: %clang_cc1 -triple loongarch64 -O1 -emit-llvm %s -o - \
+// RUN:   | FileCheck %s --check-prefix=LA64
+
+/// This test demonstrates that MaxAtomicInlineWidth is set appropriately.
+
+#include 
+#include 
+
+void test_i8_atomics(_Atomic(int8_t) * a, int8_t b) {
+  // LA32: load atomic i8, ptr %a seq_cst, align 1
+  // LA32: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA32: atomicrmw add ptr %a, i8 %b seq_cst
+  // LA64: load atomic i8, ptr %a seq_cst, align 1
+  // LA64: store atomic i8 %b, ptr %a seq_cst, align 1
+  // LA64: atomicrmw add ptr %a, i8 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i32_atomics(_Atomic(int32_t) * a, int32_t b) {
+  // LA32: load atomic i32, ptr %a seq_cst, align 4
+  // LA32: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA32: atomicrmw add ptr %a, i32 %b seq_cst
+  // LA64: load atomic i32, ptr %a seq_cst, align 4
+  // LA64: store atomic i32 %b, ptr %a seq_cst, align 4
+  // LA64: atomicrmw add ptr %a, i32 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}
+
+void test_i64_atomics(_Atomic(int64_t) * a, int64_t b) {
+  // LA32: call i64 @__atomic_load_8
+  // LA32: call void @__atomic_store_8
+  // LA32: call i64 @__atomic_fetch_add_8
+  // LA64: load atomic i64, ptr %a seq_cst, align 8
+  // LA64: store atomic i64 %b, ptr %a seq_cst, align 8
+  // LA64: atomicrmw add ptr %a, i64 %b seq_cst
+  __c11_atomic_load(a, memory_order_seq_cst);
+  __c11_atomic_store(a, b, memory_order_seq_cst);
+  __c11_atomic_fetch_add(a, b, memory_order_seq_cst);
+}

diff  --git a/clang/test/Preprocessor/init-loongarch.c 
b/clang/test/Preprocessor/init-loongarch.c
index a6a0836e3fb1d..c26cc4f741582 100644
--- a/clang/test/Preprocessor/init-loongarch.c
+++ b/clang/test/Preprocessor/init-loongarch.c
@@ -30,16 +30,16 @@
 // LA32: #define __CHAR16_TYPE__ unsigned short
 // LA32: #define __CHAR32_TYPE__ unsigned int
 // LA32: #define __CHAR_BIT__ 8
-// LA32: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_INT_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_INT_LOCK_FREE 2
 // LA32: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_LONG_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 1
-// LA32: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_LONG_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 2
+// LA32: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 2
 // LA32: #define __DBL_DECIMAL_DIG__ 17
 // LA32: #define __DBL_DENORM_MIN__ 4.94065645841246

[PATCH] D135335: [HLSL] Add Resource kind for HLSLResourceAttr.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135335

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

[PATCH] D135375: [analyzer] Initialize regions returned by CXXNew to undefined

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Some early results:

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?review-status=Unreviewed&review-status=Confirmed%20bug&detection-status=New&detection-status=Reopened&detection-status=Unresolved&is-unique=on&run=%2acxxnew_baseline&newcheck=%2acxxnew_patched&diff-type=New

I'm still waiting on a few projects, but this is something. A few reports from 
`alpha.security.ArrayBound`, I still need to look into that.

This 

 report from `core.uninitialized.Assign` is interesting. First off, I'd 
appreciate if the "storing uninitialized value" was placed //inside// the notes 
about the call to `QScopedArrayPointer`'s constructor. Otherwise, the report 
looks correct.

Not too happy about this 

 report by `core.CallAndMessage`. Message 9 is placed on the line

  applyColorTransform(d->colorSpace.transformationToColorSpace(colorSpace));

Messag 10 talks about what happens in `applyColorTransform`, but the important 
thing happens at the evaluation of the argument, which is not described, so 
this isn't a very good bug report, can't tell whether its false.

Reports from `core.UndefinedBinaryOperatorResult` here 

 and `alpha.core.Conversion` here 

 are interesting, because there doesn't need to be caused by uninitialized 
dynamic memory, but rather the fact that the memory is being modeled at all. I 
suspect this is due us maybe discarding array size information or something 
similar when the value held in `UnknownVal`?

The rest of the reports seem nobrainder gains.

Interstingly, 3 reports are lost 

 from `alpha.security.ArrayBound`, but I'm not sure how seriously do we take 
this checker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135375

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

[PATCH] D135555: [libclang] CIndex: Visit UsingTypeLoc as TypeRef

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5d859a1cdee3: [libclang] CIndex: Visit UsingTypeLoc as 
TypeRef (authored by kiloalphaindia, committed by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D13

Files:
  clang/tools/libclang/CIndex.cpp
  clang/unittests/libclang/LibclangTest.cpp
  clang/unittests/libclang/TestUtils.h

Index: clang/unittests/libclang/TestUtils.h
===
--- clang/unittests/libclang/TestUtils.h
+++ clang/unittests/libclang/TestUtils.h
@@ -82,6 +82,12 @@
 &TraverseStateless>,
 &FunctorRef);
   }
+  static std::string fromCXString(CXString cx_string) {
+std::string string{clang_getCString(cx_string)};
+clang_disposeString(cx_string);
+return string;
+  };
+
 private:
   template
   static CXChildVisitResult TraverseStateless(CXCursor cx, CXCursor parent,
@@ -91,4 +97,4 @@
   }
 };
 
-#endif // LLVM_CLANG_TEST_TESTUTILS_H
\ No newline at end of file
+#endif // LLVM_CLANG_TEST_TESTUTILS_H
Index: clang/unittests/libclang/LibclangTest.cpp
===
--- clang/unittests/libclang/LibclangTest.cpp
+++ clang/unittests/libclang/LibclangTest.cpp
@@ -863,27 +863,19 @@
clang_isRestrictQualifiedType(type);
   };
 
-  auto from_CXString = [](CXString cx_string) -> std::string {
-std::string string{clang_getCString(cx_string)};
-
-clang_disposeString(cx_string);
-
-return string;
-  };
-
   ClangTU = clang_parseTranslationUnit(Index, Header.c_str(), nullptr, 0,
nullptr, 0, TUFlags);
 
-  Traverse([&is_qualified, &from_CXString](CXCursor cursor, CXCursor) {
+  Traverse([&is_qualified](CXCursor cursor, CXCursor) {
 if (clang_getCursorKind(cursor) == CXCursor_FunctionDecl) {
   CXType arg_type = clang_getArgType(clang_getCursorType(cursor), 0);
   EXPECT_TRUE(is_qualified(arg_type))
-  << "Input data '" << from_CXString(clang_getCursorSpelling(cursor))
+  << "Input data '" << fromCXString(clang_getCursorSpelling(cursor))
   << "' first argument does not have a qualified type.";
 
   CXType unqualified_arg_type = clang_getUnqualifiedType(arg_type);
   EXPECT_FALSE(is_qualified(unqualified_arg_type))
-  << "The type '" << from_CXString(clang_getTypeSpelling(arg_type))
+  << "The type '" << fromCXString(clang_getTypeSpelling(arg_type))
   << "' was not unqualified after a call to clang_getUnqualifiedType.";
 }
 
@@ -901,28 +893,20 @@
(type.kind == CXType_RValueReference);
   };
 
-  auto from_CXString = [](CXString cx_string) -> std::string {
-std::string string{clang_getCString(cx_string)};
-
-clang_disposeString(cx_string);
-
-return string;
-  };
-
   const char *Args[] = {"-xc++"};
   ClangTU = clang_parseTranslationUnit(Index, Header.c_str(), Args, 1, nullptr,
0, TUFlags);
 
-  Traverse([&is_ref_qualified, &from_CXString](CXCursor cursor, CXCursor) {
+  Traverse([&is_ref_qualified](CXCursor cursor, CXCursor) {
 if (clang_getCursorKind(cursor) == CXCursor_FunctionDecl) {
   CXType arg_type = clang_getArgType(clang_getCursorType(cursor), 0);
   EXPECT_TRUE(is_ref_qualified(arg_type))
-  << "Input data '" << from_CXString(clang_getCursorSpelling(cursor))
+  << "Input data '" << fromCXString(clang_getCursorSpelling(cursor))
   << "' first argument does not have a ref-qualified type.";
 
   CXType non_reference_arg_type = clang_getNonReferenceType(arg_type);
   EXPECT_FALSE(is_ref_qualified(non_reference_arg_type))
-  << "The type '" << from_CXString(clang_getTypeSpelling(arg_type))
+  << "The type '" << fromCXString(clang_getTypeSpelling(arg_type))
   << "' ref-qualifier was not removed after a call to "
  "clang_getNonReferenceType.";
 }
@@ -931,6 +915,37 @@
   });
 }
 
+TEST_F(LibclangParseTest, VisitUsingTypeLoc) {
+  const char testSource[] = R"cpp(
+namespace ns1 {
+class Class1
+{
+void fun();
+};
+}
+
+using ns1::Class1;
+
+void Class1::fun() {}
+)cpp";
+  std::string fileName = "main.cpp";
+  WriteFile(fileName, testSource);
+  const char *Args[] = {"-xc++"};
+  ClangTU = clang_parseTranslationUnit(Index, fileName.c_str(), Args, 1,
+   nullptr, 0, TUFlags);
+
+  llvm::Optional typeRefCsr;
+  Traverse([&](CXCursor cursor, CXCursor parent) -> CXChildVisitResult {
+if (cursor.kind == CXCursor_TypeRef) {
+  typeRefCsr.emplace(cursor);
+}
+return CXChildVisit_Recurse;
+  });
+  ASSERT_TRUE(typeRefCsr.has_value());
+  EXPECT_EQ(fromCXString(clang_getCursorSpelling(*typeRefCsr)),
+"class ns1::Class1");
+}
+
 class LibclangRewriteTest : public LibclangParseTest {
 public:

[clang] 5d859a1 - [libclang] CIndex: Visit UsingTypeLoc as TypeRef

[Sam McCall via cfe-commits]

Author: Kai Stierand
Date: 2022-10-11T15:01:31+02:00
New Revision: 5d859a1cdee3c15dce692767ee3e9ad03a8c4c1b

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

LOG: [libclang] CIndex: Visit UsingTypeLoc as TypeRef

Reviewed By: sammccall

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

Added: 


Modified: 
clang/tools/libclang/CIndex.cpp
clang/unittests/libclang/LibclangTest.cpp
clang/unittests/libclang/TestUtils.h

Removed: 




diff  --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp
index 4c7c018a231ce..69d34646ca937 100644
--- a/clang/tools/libclang/CIndex.cpp
+++ b/clang/tools/libclang/CIndex.cpp
@@ -1747,7 +1747,13 @@ bool 
CursorVisitor::VisitRValueReferenceTypeLoc(RValueReferenceTypeLoc TL) {
   return Visit(TL.getPointeeLoc());
 }
 
-bool CursorVisitor::VisitUsingTypeLoc(UsingTypeLoc TL) { return false; }
+bool CursorVisitor::VisitUsingTypeLoc(UsingTypeLoc TL) {
+  auto *underlyingDecl = TL.getUnderlyingType()->getAsTagDecl();
+  if (underlyingDecl) {
+return Visit(MakeCursorTypeRef(underlyingDecl, TL.getNameLoc(), TU));
+  }
+  return false;
+}
 
 bool CursorVisitor::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
   return Visit(TL.getModifiedLoc());

diff  --git a/clang/unittests/libclang/LibclangTest.cpp 
b/clang/unittests/libclang/LibclangTest.cpp
index d020fa9af61c5..a2a17646a871b 100644
--- a/clang/unittests/libclang/LibclangTest.cpp
+++ b/clang/unittests/libclang/LibclangTest.cpp
@@ -863,27 +863,19 @@ TEST_F(LibclangParseTest, 
clang_getUnqualifiedTypeRemovesQualifiers) {
clang_isRestrictQualifiedType(type);
   };
 
-  auto from_CXString = [](CXString cx_string) -> std::string {
-std::string string{clang_getCString(cx_string)};
-
-clang_disposeString(cx_string);
-
-return string;
-  };
-
   ClangTU = clang_parseTranslationUnit(Index, Header.c_str(), nullptr, 0,
nullptr, 0, TUFlags);
 
-  Traverse([&is_qualified, &from_CXString](CXCursor cursor, CXCursor) {
+  Traverse([&is_qualified](CXCursor cursor, CXCursor) {
 if (clang_getCursorKind(cursor) == CXCursor_FunctionDecl) {
   CXType arg_type = clang_getArgType(clang_getCursorType(cursor), 0);
   EXPECT_TRUE(is_qualified(arg_type))
-  << "Input data '" << from_CXString(clang_getCursorSpelling(cursor))
+  << "Input data '" << fromCXString(clang_getCursorSpelling(cursor))
   << "' first argument does not have a qualified type.";
 
   CXType unqualified_arg_type = clang_getUnqualifiedType(arg_type);
   EXPECT_FALSE(is_qualified(unqualified_arg_type))
-  << "The type '" << from_CXString(clang_getTypeSpelling(arg_type))
+  << "The type '" << fromCXString(clang_getTypeSpelling(arg_type))
   << "' was not unqualified after a call to clang_getUnqualifiedType.";
 }
 
@@ -901,28 +893,20 @@ TEST_F(LibclangParseTest, 
clang_getNonReferenceTypeRemovesRefQualifiers) {
(type.kind == CXType_RValueReference);
   };
 
-  auto from_CXString = [](CXString cx_string) -> std::string {
-std::string string{clang_getCString(cx_string)};
-
-clang_disposeString(cx_string);
-
-return string;
-  };
-
   const char *Args[] = {"-xc++"};
   ClangTU = clang_parseTranslationUnit(Index, Header.c_str(), Args, 1, nullptr,
0, TUFlags);
 
-  Traverse([&is_ref_qualified, &from_CXString](CXCursor cursor, CXCursor) {
+  Traverse([&is_ref_qualified](CXCursor cursor, CXCursor) {
 if (clang_getCursorKind(cursor) == CXCursor_FunctionDecl) {
   CXType arg_type = clang_getArgType(clang_getCursorType(cursor), 0);
   EXPECT_TRUE(is_ref_qualified(arg_type))
-  << "Input data '" << from_CXString(clang_getCursorSpelling(cursor))
+  << "Input data '" << fromCXString(clang_getCursorSpelling(cursor))
   << "' first argument does not have a ref-qualified type.";
 
   CXType non_reference_arg_type = clang_getNonReferenceType(arg_type);
   EXPECT_FALSE(is_ref_qualified(non_reference_arg_type))
-  << "The type '" << from_CXString(clang_getTypeSpelling(arg_type))
+  << "The type '" << fromCXString(clang_getTypeSpelling(arg_type))
   << "' ref-qualifier was not removed after a call to "
  "clang_getNonReferenceType.";
 }
@@ -931,6 +915,37 @@ TEST_F(LibclangParseTest, 
clang_getNonReferenceTypeRemovesRefQualifiers) {
   });
 }
 
+TEST_F(LibclangParseTest, VisitUsingTypeLoc) {
+  const char testSource[] = R"cpp(
+namespace ns1 {
+class Class1
+{
+void fun();
+};
+}
+
+using ns1::Class1;
+
+void Class1::fun() {}
+)cpp";
+  std::string fileName = "main.cpp";
+  WriteFile(fileName, testSource);
+  const char *Args[] = {"-xc++"};
+  ClangTU = clang_parseTra

[PATCH] D135513: [clang][Interp] Check instance pointers before calling functions on them

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Function.cpp:39
+  assert(It != SrcMap.end());
+  It--; // We want the offset *before* the given one.
   return It->second;

While I think the comment here is correct, the decrement itself certainly isn't.


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

https://reviews.llvm.org/D135513

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

[PATCH] D135384: [AIX] Enable the use of the -pg flag

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm added inline comments.



Comment at: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp:38
+const std::string &targetTriple = M.getTargetTriple();
+if (targetTriple.find("powerpc-ibm-aix") == std::string::npos ||
+targetTriple.find("powerpc64-ibm-aix") == std::string::npos) {

Please use isOSAIX


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135384

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

[PATCH] D129156: Add -fpass-plugin option to Flang

[Tarun Prabhu via Phabricator via cfe-commits]

tarunprabhu added a comment.

In D129156#3848728 , @awarzynski 
wrote:

> Thanks Valentin! Switching between generators will definitely change the 
> order in which libraries will built (and, AFAIK, the order is 
> non-deterministic to begin with). I will try to experiment later today.

I ran this with Unix Makefiles as the generator and it passes there too.

What I did notice when I tried the out-of-tree build of flang is that the test 
did not run. The message I got was that the test was "unsupported".

I assume that is because there is no dependence on LLVM examples in an 
out-of-tree build. Would adding an explicit dependence on LLVM's examples cause 
the test to run? I imagine that we want the feature to be tested even when 
building flang out-of-tree.


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

https://reviews.llvm.org/D129156

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

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

>> In D129755#3842744 , @hans wrote:
>>
>>> 
>>
>> I'll give you some time to reply before I reland, but I can't see a bug 
>> here. For the next time, as already pointed out, give the involved people 
>> some time to reply before you revert (unless it's a crash). We have to be 
>> able to continue working on the compiler without having Google revert 
>> changes all the time because we produce new warnings.
>
> We've seen reports here of various breakages from two different projects in 
> short time after your patch. The general policy is to revert to green, so I 
> think that was the right call.

I don't think it was the wrong call per se, but it was an aggressive revert 
IMO. Chromium tends to revert changes very quickly and without discussion, and 
I'm hoping we can change that slightly to include more up-front discussion 
before a revert. It's one thing to revert when the patch author isn't 
responsive, but reverting immediately and without discussion when the commit 
message explicitly states there is a change in behavior isn't how I would 
expect the revert policy to be interpreted.

> Typically, new warnings are introduced behind new flags so that users can 
> adopt them incrementally, though I realize that might be tricky in this 
> instance. In any case, perhaps we should give other projects some time to 
> assess the impact of this before relanding?

Other projects won't have the time to assess the impact of these changes 
because the changes were reverted (almost nobody applies one-off patches to try 
them out without there being something else driving that experiment). I think 
the changes should be re-landed so people can get us that feedback.

> This also seems like the kind of disruptive change we'd want to announce on 
> Discourse, as discussed in Aaron's recent RFC 
> .
>  Probably should be mentioned in the "Potentially Breaking Changes" section 
> of the release notes too.

+1!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129755

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

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: clang-vendors.
aaron.ballman added a comment.

Adding the clang-vendors group for awareness of potential disruptions from 
these changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129755

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

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D126907#3846591 , @erichkeane 
wrote:

> In D126907#3844788 , @BertalanD 
> wrote:
>
>> Hi @erichkeane,
>>
>> This change broke compilation of this program 
>> (https://godbolt.org/z/KrWGvcf8h; reduced from 
>> https://github.com/SerenityOS/ladybird):
>>
>>   template
>>   constexpr bool IsSame = false;
>>   
>>   template
>>   constexpr bool IsSame = true;
>>   
>>   template
>>   struct Foo {
>>   template
>>   Foo(U&&) requires (!IsSame);
>>   };
>>   
>>   template<>
>>   struct Foo : Foo {
>>   using Foo::Foo;
>>   };
>>   
>>   Foo test() { return 0; }
>>
>>
>>
>>   :18:27: error: invalid reference to function 'Foo': constraints 
>> not satisfied
>>   Foo test() { return 0; }
>> ^
>>   :10:24: note: because substituted constraint expression is 
>> ill-formed: value of type '' is not contextually convertible 
>> to 'bool'
>>   Foo(U&&) requires (!IsSame);
>>  ^
>
> Thanks for the report!  I'll look into it ASAP.

Quick note: I believe I understand the cause of this, which requires a bit more 
work than I otherwise would have expected.  I have a candidate patch I'm 
running through my testing right now that should fix this, but it still needs 
cleaning up.  Expect it in the next day or two if all goes well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126907

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

[PATCH] D135680: [clang][ARM] follow GCC behavior for defining __SOFTFP__

[Ties Stuij via Phabricator via cfe-commits]

stuij created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
stuij requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

GCC behavior regarding defining __SOFTFP__ when (implicitly) specifying
-mfloat-abi=softfp:

- compile without (implicit) FP: define __SOFTFP__
- compile with (implicit) FP: don't define __SOFTFP__

Currently Clang doesn't define __SOFTFP__ when softfp is specified, either with
or without FP. This patch brings Clang in line with GCC behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135680

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/test/Preprocessor/init-arm.c

Index: clang/test/Preprocessor/init-arm.c
===
--- clang/test/Preprocessor/init-arm.c
+++ clang/test/Preprocessor/init-arm.c
@@ -395,199 +395,399 @@
 // ARM-BE:#define __arm 1
 // ARM-BE:#define __arm__ 1
 
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=arm-none-linux-gnueabi -target-feature +soft-float -target-feature +soft-float-abi < /dev/null | FileCheck -match-full-lines -check-prefix ARMEABISOFTFP %s
+// explanation of difference between ARMEABISOFTFP_NOFP and ARMEABISOFTFP_FP
+// below:
+// - compile targets with no FPU should emit __SOFTFP__ 1 when +soft-float-abi
+//   is specified
+// - compile targets with FPU should not emit __SOFTFP__ 1 when +soft-float-abi
+//   is specified
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=arm-none-linux-gnueabi -target-feature +soft-float -target-feature +soft-float-abi < /dev/null | FileCheck -match-full-lines -check-prefix ARMEABISOFTFP_NOFP %s
 //
-// ARMEABISOFTFP-NOT:#define _LP64
-// ARMEABISOFTFP:#define __APCS_32__ 1
-// ARMEABISOFTFP-NOT:#define __ARMEB__ 1
-// ARMEABISOFTFP:#define __ARMEL__ 1
-// ARMEABISOFTFP:#define __ARM_ARCH 4
-// ARMEABISOFTFP:#define __ARM_ARCH_4T__ 1
-// ARMEABISOFTFP-NOT:#define __ARM_BIG_ENDIAN 1
-// ARMEABISOFTFP:#define __ARM_EABI__ 1
-// ARMEABISOFTFP:#define __ARM_PCS 1
-// ARMEABISOFTFP-NOT:#define __ARM_PCS_VFP 1
-// ARMEABISOFTFP:#define __BIGGEST_ALIGNMENT__ 8
-// ARMEABISOFTFP:#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
-// ARMEABISOFTFP:#define __CHAR16_TYPE__ unsigned short
-// ARMEABISOFTFP:#define __CHAR32_TYPE__ unsigned int
-// ARMEABISOFTFP:#define __CHAR_BIT__ 8
-// ARMEABISOFTFP:#define __DBL_DENORM_MIN__ 4.9406564584124654e-324
-// ARMEABISOFTFP:#define __DBL_DIG__ 15
-// ARMEABISOFTFP:#define __DBL_EPSILON__ 2.2204460492503131e-16
-// ARMEABISOFTFP:#define __DBL_HAS_DENORM__ 1
-// ARMEABISOFTFP:#define __DBL_HAS_INFINITY__ 1
-// ARMEABISOFTFP:#define __DBL_HAS_QUIET_NAN__ 1
-// ARMEABISOFTFP:#define __DBL_MANT_DIG__ 53
-// ARMEABISOFTFP:#define __DBL_MAX_10_EXP__ 308
-// ARMEABISOFTFP:#define __DBL_MAX_EXP__ 1024
-// ARMEABISOFTFP:#define __DBL_MAX__ 1.7976931348623157e+308
-// ARMEABISOFTFP:#define __DBL_MIN_10_EXP__ (-307)
-// ARMEABISOFTFP:#define __DBL_MIN_EXP__ (-1021)
-// ARMEABISOFTFP:#define __DBL_MIN__ 2.2250738585072014e-308
-// ARMEABISOFTFP:#define __DECIMAL_DIG__ __LDBL_DECIMAL_DIG__
-// ARMEABISOFTFP:#define __FLT_DENORM_MIN__ 1.40129846e-45F
-// ARMEABISOFTFP:#define __FLT_DIG__ 6
-// ARMEABISOFTFP:#define __FLT_EPSILON__ 1.19209290e-7F
-// ARMEABISOFTFP:#define __FLT_HAS_DENORM__ 1
-// ARMEABISOFTFP:#define __FLT_HAS_INFINITY__ 1
-// ARMEABISOFTFP:#define __FLT_HAS_QUIET_NAN__ 1
-// ARMEABISOFTFP:#define __FLT_MANT_DIG__ 24
-// ARMEABISOFTFP:#define __FLT_MAX_10_EXP__ 38
-// ARMEABISOFTFP:#define __FLT_MAX_EXP__ 128
-// ARMEABISOFTFP:#define __FLT_MAX__ 3.40282347e+38F
-// ARMEABISOFTFP:#define __FLT_MIN_10_EXP__ (-37)
-// ARMEABISOFTFP:#define __FLT_MIN_EXP__ (-125)
-// ARMEABISOFTFP:#define __FLT_MIN__ 1.17549435e-38F
-// ARMEABISOFTFP:#define __FLT_RADIX__ 2
-// ARMEABISOFTFP:#define __INT16_C_SUFFIX__
-// ARMEABISOFTFP:#define __INT16_FMTd__ "hd"
-// ARMEABISOFTFP:#define __INT16_FMTi__ "hi"
-// ARMEABISOFTFP:#define __INT16_MAX__ 32767
-// ARMEABISOFTFP:#define __INT16_TYPE__ short
-// ARMEABISOFTFP:#define __INT32_C_SUFFIX__
-// ARMEABISOFTFP:#define __INT32_FMTd__ "d"
-// ARMEABISOFTFP:#define __INT32_FMTi__ "i"
-// ARMEABISOFTFP:#define __INT32_MAX__ 2147483647
-// ARMEABISOFTFP:#define __INT32_TYPE__ int
-// ARMEABISOFTFP:#define __INT64_C_SUFFIX__ LL
-// ARMEABISOFTFP:#define __INT64_FMTd__ "lld"
-// ARMEABISOFTFP:#define __INT64_FMTi__ "lli"
-// ARMEABISOFTFP:#define __INT64_MAX__ 9223372036854775807LL
-// ARMEABISOFTFP:#define __INT64_TYPE__ long long int
-// ARMEABISOFTFP:#define __INT8_C_SUFFIX__
-// ARMEABISOFTFP:#define __INT8_FMTd__ "hhd"
-// ARMEABISOFTFP:#define __INT8_FMTi__ "hhi"
-// ARMEABISOFTFP:#define __INT8_MAX__ 127
-// ARMEABISOFTFP:#define __INT8_TYPE__ signed char
-// ARMEABISOFTFP:#define __INTMAX_C_SUFFIX__ LL
-// ARMEABISOFTFP:#define __INTMAX_FMTd__ "lld"
-// ARMEABISOFTFP:#define __INTMAX_FMTi__ "lli"
-// ARMEABISOFTFP:#define __INTMAX_MAX__ 9223372036854775807LL
-// ARMEABISOFTFP:#def

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

To the original point I was making about implicit modules (which might've been 
my own confusion due to the term being brought up in D134269 
):

Implicit modules is the situation where the compiler, finding that it needs a 
module while compiling some usage, knows how to go out and spawn a new process 
to create that module and then cache it. The build system never knows about 
modules, their builds or their dependencies.

This patch had some superficial similarities - specifically around having an 
on-disk cache of modules that the user/build system/etc invoking the compiler 
isn't necessarily aware of/managing/invalidating/observing/etc. But it does 
differ in an important way from implicit modules in that the compiler won't 
implicitly build modules - you still have to specify the modules and their 
usage as separate compilations in-order (ie: modules need to be explicitly 
built before their usage). I think that makes a big difference to this being 
feasible, at least in the small scale.

The remaining concern is that this feature should likely not be used by a build 
system - because it won't know the dependencies (or, if it does know the 
dependencies then the build system, not the compiler, should be managing the 
BMIs) & so won't know how to schedule things for maximum parallelism without 
incorrect ordering, and correct rebuilding of dependencies when necessary.

So maybe we could limit the "Hello, World" situation only to cases where the 
compiler is producing temporary object files? I know this probably sounds 
absurd, but coming back to a point I made earlier about .pcm files being named 
with the same scheme as .o files - so if the .o file goes into the temp 
directory, the .pcm goes there in the same manner - the driver could, 
potentially, add that module (`-fmodule-file` or whatever is necessary) to all 
subsequent compilations on the same command line (eg: `clang++ x.cppm y.cppm 
z.cpp a.o` - so x.cppm gets built temporarily, y.cppm gets built temporarily 
and gets `-fmodule-file=/tmp/x.pcm` or whatever, then z.cpp gets built with 
`-fmodule-file=/tmp/x.pcm -fmodule-file=/tmp/y.pcm` and then the whole thing 
gets linked as usual)

This ensures the feature doesn't creep into build systems where it might be 
more confusing than helpful, given the implicit dependencies between build 
commands. It's admittedly going to be confusing to users that they can't 
trivially split their command line out into multiple, but I think that's 
probably the tipping point in complexity for modules builds where users should 
reach for a build system or live with the complexity of passing .pcm files 
explicitly between these steps.



Sorry I haven't been following the discussion between this and what @iains is 
proposing. I guess/roughly understand that @iains is proposing something like 
the module-mapper support? Which I rather like the idea of, though understand 
it has more compiler surface area/complex interaction with the mapping service.

I guess @iains is proposing that the mapper could be used to implement/address 
this "Hello, World" situation in some way? (I guess that looks maybe /more/ 
like implicit modules than this proposal - in that the module mapper does mean 
that during one compilation, another compilation might run - and for a 
build-system agnostic module mapper that boils down to the problems of build 
parallelism that implicit modules have (though it had a lot of success with 
that model, not having to modify build systems/being able to ship sooner/etc - 
it's been around for years, and as much as we've learned of its limitations, 
it's hard to argue with the success/suggest that it wasn't the right tradeoff 
(I have some reservations still, but that's more an social/after-work ramble 
than technical)))

In any case, it looks like there's design discussions to be had? Not sure if 
this is the right place to have them, but maybe it is. It might be easier to 
discuss them on discourse with hopefully some relatively narrow examples with 
command lines shown, etc. (as already some of the conversation seems to be 
fragmented between this change and the doc change)


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

https://reviews.llvm.org/D134267

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

[PATCH] D135614: [OpenMP][CUDA][AMDGPU] Accept case insensitive subarchitecture names

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

I am not sure whether it is a good idea to allow gfx90A in `--offload-arch`, 
since it is not documented in LLVM AMDGPU usage. @b-sumner @arsenm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135614

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

[PATCH] D135614: [OpenMP][CUDA][AMDGPU] Accept case insensitive subarchitecture names

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

I don't really have an opinion here. I'd probably lean towards a "did you mean" 
kind of warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135614

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

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

[Nicolas van Kempen via Phabricator via cfe-commits]

nicovank added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131939

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

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/test/Frontend/frame-diags.c:1
+// TODO: This is not really a functional test yet, and needs to be rewritten. 
+// currently its just a copy of backend-diagnostics.c, and all of the run/test 

I know this is a draft and has a TODO but:
Something like this could go into cross-project-tests, but an llvm change 
should have an llvm test, not a clang test.  You could use clang to generate IR 
once, and keep instructions for regenerating the IR as comments in the test 
file.  We do this a lot for debug info tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};

tbaeder wrote:
> cor3ntin wrote:
> > tahonermann wrote:
> > > As others already noted, additional testing of multicharacter literals 
> > > and UCNs (including named universal characters like 
> > > `\N{LATIN_CAPITAL_LETTER_E}` would be beneficial. Some tests of character 
> > > escapes like `\t` wouldn't hurt either.
> > > 
> > > Clang does not yet support use of `-fexec-charset` to set the literal 
> > > encoding (execution character set) to anything other than UTF-8 though 
> > > work on that has been done (see D93031). If such work was completed, it 
> > > would be useful to run some tests against a non-UTF-8 encoding. Maybe 
> > > next year.
> > Yes, wide **multicharacter** literals, that's was important information 
> > missing, thanks for spotting that.
> > 
> > I have mixed feeling about adding tests for escape sequences.  Their 
> > replacement doesn't happen during constant evaluation.
> > We shouldn't replicate the lexing tests here.
> > 
> > but we should compare string literal with byte values. Testing a string 
> > literal against another one doesn't ensure the code units are correct if 
> > both are equally miss evaluated.
> > 
> > Also we could add explicit tests for null termination here as they are 
> > added as part of evaluation in theory - but then again that's also 
> > something clang does earlier.
> > 
> > If we want we could consider enabling the byte code interpreter on the 
> > existing lexing test files, i actually think that's the better way to deal 
> > with the escape sequences tests.
> I changed the first test that inspects all characters of a string to 
> comparing with integers instead. Do you have a suggestion for what lexing 
> tests to enable the constant interpreter in?
I think good candidates are

Lexer/char-escapes.c
Lexer/char-escapes-delimited.c
Lexer/char-literal.cpp


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

https://reviews.llvm.org/D135366

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

[PATCH] D135614: [OpenMP][CUDA][AMDGPU] Accept case insensitive subarchitecture names

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

I don't particularly see a need for this.  I am not opposed to a "did you mean" 
in the error diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135614

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

[PATCH] D135614: [OpenMP][CUDA][AMDGPU] Accept case insensitive subarchitecture names

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

Also, we may want to use uppercase for other purposes in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135614

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

[PATCH] D135488: [codegen][WIP] Display stack layouts in console

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added inline comments.



Comment at: clang/test/Frontend/frame-diags.c:1
+// TODO: This is not really a functional test yet, and needs to be rewritten. 
+// currently its just a copy of backend-diagnostics.c, and all of the run/test 

probinson wrote:
> I know this is a draft and has a TODO but:
> Something like this could go into cross-project-tests, but an llvm change 
> should have an llvm test, not a clang test.  You could use clang to generate 
> IR once, and keep instructions for regenerating the IR as comments in the 
> test file.  We do this a lot for debug info tests.
I totally agree.  I planned to follow the existing examples for 
‘frame-larger-than’ with tests in llvm on behavior etc., and checks for the 
diagnostics being plumbed correctly in clang. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

(trying not derail this discussion further)

- Yes, the alternate solution proposed for the "Hello World" case would work 
with the module mapper interface.  However, the initial discussion was simply 
about being able to produce both the .PCM and .O artefacts from one invocation 
of the FE.
- It seemed reasonable to mention (since it's not vapourware, there are draft 
patches), but clearly some technical issues to address.
- I do not think this patch fully addresses the issue of harmonising GCC and 
clang's command lines for modular builds (because it does not deal with 
discovery of modular code in 'normally named' sources), and my investigation of 
trying to do this in the driver suggests that it would be much more complex 
than doing it in the FE.

In the discussions, we have uncovered some other things that should be 
discussed elsewhere too.
(so I agree any further discussion of the alternate interface can be moved 
elsewhere - either wait for the rebased patches or on discourse).


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

https://reviews.llvm.org/D134267

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

What really worries me that you are talking about one phase compilation and 
there is a module cache. Even if it is one phase compilation you must pass all 
dependent modules on the command line.


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

https://reviews.llvm.org/D134267

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

The one phase compilation should work with `-fno-implicit-modules`!


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

https://reviews.llvm.org/D134267

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

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Because I was looking, some other `assert(0)` -> `llvm_unreachable` cleanups 
(though, yes, even the earliest cleanups include some assert(0) -> 
report_fatal_error, but for externally/user-reachable failures, like invalid 
bitcode, I think). Some of these are more blanket/wide reaching than others, 
for sure. (& no doubt the naive search through the commit log doesn't find all 
the cleanups)

I /think/ when I was looking I did find one from 2017 that Richard Smith 
approved that included llvm_unreachable in the Clang C API, in terms of more 
recent precedent for some of this... but can't find that again right now.

rGabd1561f15e 
:[LLDBAssert]
 Use unreachable instead of assert(0)
rGf9da10ce4544ea66fe6fad5b943d3700d192a1e1 
:Change to 
assert(0,x) to llvm_unreachable(x)
rG7a247f709baa2cd19111af9e18965df4e419949a 
:Turn 
effective assert(0) into llvm_unreachable
rG35b2f75733c98e5904c5a75f8bcedeb96c4f4eda 
:Convert 
some assert(0) to llvm_unreachable or fold an 'if' condition into the assert.
rGd8d43191d8afe2c4b5b2d3be62cd73ddc3ddc6c9 
:Replace 
some assert(0)'s with llvm_unreachable.
rG2a30d7889fc54c8a74d73b79be3dd030bac41b06 
:Replace 
some assert(0)'s with llvm_unreachable.
rG0039f3f0607702f2d16d60addff74c67869e2144 
:Replace 
some assert(0)'s with llvm_unreachable.
rGc7193c48d9d7e44e9fd0c39205e8b7cfbf5d5458 
:Convert 
assert(0) to llvm_unreachable to silence a warning about Addend being 
uninitialized in default case.
rG7b7a67c5c8daa051e42837dc3e5e65adab9cf09c 
:[ARM64] 
Fix 'assert("...")' to be 'assert(0 && "...")'. Otherwise, it is no assert at 
all. ;] Some of these should probably be switched to llvm_unreachable, but I 
didn't want to perturb the behavior in this patch.
rGeaa3a7efab65d0a65ddda7fdb7e5fbdbd5f897ad 
:Use 
llvm_unreachable instead of assert(0)
rGd7fd95a5c1eb19e5754281b540f09e86ced1b9d4 
:Change 
assert(0 && "text") to llvm_unreachable(0 && "text")
rG2962d9599e463265edae599285bbc6351f1cc0ef 
:Suggest 
llvm_unreachable over assert(0).
rG2e007de42de48dc05bdb7aa9d3c9e8902ee720fe 
:Revert 
"Target/AMDGPU/AMDILIntrinsicInfo.cpp: Use llvm_unreachable() in nonreturn 
function, instead of assert(0)."
rGdc4261794fdbf2e3001f369d8b8bbd77eb923602 
:Target/AMDGPU/AMDILIntrinsicInfo.cpp:
 Use llvm_unreachable() in nonreturn function, instead of assert(0).
rG751eb3d2b30d90069b1797a31f8e489f0763c585 
:use 
llvm_unreachable() instead of assert(0) for invalid enum values in switch 
statements
rGbdf39a46a302df7cb07e948a6780265b3335fbda 
:Convert 
assert(0) to llvm_unreachable.
rGeb455832b4c7cb1046ab9fb9f6f8ca40202874a4 
:Silence 
various build warnings from Hexagon backend that show up in release builds. 
Mostly converting 'assert(0)' to 'llvm_unreachable' to silence warnings about 
missing returns. Also fold some variable declarations into asserts to prevent 
the variables from being unused in release builds.
rGabd1561f15ee466c0fd9abeede2cdcde2ebb2cec 
:[LLDBAssert]
 Use unreachable instead of assert(0)
rGf9da10ce4544ea66fe6fad5b943d3700d192a1e1 
:Change to 
assert(0,x) to llvm_unreachable(x)
rG7a247f709baa2cd19111af9e18965df4e419949a 
:Turn 
effective assert(0) into llvm_unreachable
rG35b2f75733c98e5904c5a75f8bcedeb96c4f4eda 
:Convert 
some assert(0) to llvm_unreachable or fold an 'if' condition into the assert.
rGd8d43191d8afe2c4b5b2d3be62cd73ddc3ddc6c9 
:Replace 
some assert(0)'s with llvm_unreachable.
rG2a30d7889fc54c8a74d73b79be3dd030bac41b06 

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D134267#3849673 , @iains wrote:

> (trying not derail this discussion further)
>
> - Yes, the alternate solution proposed for the "Hello World" case would work 
> with the module mapper interface.  However, the initial discussion was simply 
> about being able to produce both the .PCM and .O artefacts from one 
> invocation of the FE.

*nod* If there's some other patch that does just that - produces a .o/.pcm 
together in a single invocation, maybe that's a less contentious/easier patch 
to review/discuss/approve/commit. The number of process invocations involved in 
that (whether the driver, under the hood, invokes clang once to generate a .pcm 
and another to generate the .o) I'm less fussed about - I think that's 
something that can be improved separately - especially once/if someone wants to 
change the .pcm format to be lighter weight (not enough to generate the .o 
from, but enough for compilations /using/ the module to do so) - that'll drive 
a need for a monolithic .cppm -> {.o,.pcm} action.

> - It seemed reasonable to mention (since it's not vapourware, there are draft 
> patches), but clearly some technical issues to address.

For sure.

> - I do not think this patch fully addresses the issue of harmonising GCC and 
> clang's command lines for modular builds (because it does not deal with 
> discovery of modular code in 'normally named' sources), and my investigation 
> of trying to do this in the driver suggests that it would be much more 
> complex than doing it in the FE.

Yes, that would be hard to implement in the driver - but my personal taste is 
that if the compiler's going to treat the input differently, or produce 
different outputs, it should be observable on the command line - either through 
different flags (`-x`) or a different file extension. I think that's been 
historically true? Though I realize it also comes up against "but we have all 
this C++ with these existing file extensions" and especially if GCC doesn't 
agree on that philosophy, Clang's going to have a hard time making a stand 
there... :/


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

https://reviews.llvm.org/D134267

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

[PATCH] D135682: Fix false positive related to handling of [[noreturn]] function pointers

[Arseniy Zaostrovnykh via Phabricator via cfe-commits]

arseniy-sonar created this revision.
arseniy-sonar added reviewers: xazax.hun, martong, NoQ, Szelethus, steakhal.
arseniy-sonar added a project: clang.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
arseniy-sonar requested review of this revision.
Herald added a subscriber: cfe-commits.

Before this change, the NoReturnFunctionChecker was missing function pointers 
with a [[noreturn]] attribute, while CFG was constructed taking that into 
account, which lead CSA to take impossible paths. The reason was that the 
NoReturnFunctionChecker was looking for the attribute in the type of the entire 
call expression rather than the type of the function being called.

This change makes the [[noreturn]] attribute of a function pointer visible to 
NoReturnFunctionChecker. This leads to a more coherent behavior of the CSA on 
the AST involving


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135682

Files:
  clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
  clang/test/Analysis/no-return.c


Index: clang/test/Analysis/no-return.c
===
--- /dev/null
+++ clang/test/Analysis/no-return.c
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+typedef void(fatal_fun)() __attribute__((__noreturn__));
+fatal_fun* fatal_fptr;
+void fatal_decl() __attribute__((__noreturn__));
+
+int rng();
+
+/// This code calls a [[noreturn]] function pointer, which used to be handled
+/// inconsistently between AST builder and CSA.
+/// In the result, CSA produces a path where this function returns non-0.
+int return_zero_or_abort_by_fnptr() {
+  if (rng()) fatal_fptr();
+  return 0;
+}
+
+/// This function calls a [[noreturn]] function.
+/// If it does return, it always returns 0.
+int return_zero_or_abort_by_direct_fun() {
+  if (rng()) fatal_decl();
+  return 0;
+}
+
+/// Trigger a division by zero issue depending on the return value
+/// of the called functions.
+int caller() {
+  int x = 0;
+  // The following if branches must never be taken.
+  if (return_zero_or_abort_by_fnptr())
+return 1 / x; // no-warning: Dead code.
+  if (return_zero_or_abort_by_direct_fun())
+return 1 / x; // no-warning: Dead code.
+
+  // Make sure the warning is still reported when viable.
+  return 1 / x; // expected-warning {{Division by zero}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
@@ -44,9 +44,11 @@
   if (const FunctionDecl *FD = dyn_cast_or_null(CE.getDecl()))
 BuildSinks = FD->hasAttr() || FD->isNoReturn();
 
-  const Expr *Callee = CE.getOriginExpr();
-  if (!BuildSinks && Callee)
-BuildSinks = getFunctionExtInfo(Callee->getType()).getNoReturn();
+  if (const CallExpr* CExpr = dyn_cast_or_null(CE.getOriginExpr());
+  CExpr && !BuildSinks) {
+if (const Expr *C = CExpr->getCallee())
+  BuildSinks = getFunctionExtInfo(C->getType()).getNoReturn();
+  }
 
   if (!BuildSinks && CE.isGlobalCFunction()) {
 if (const IdentifierInfo *II = CE.getCalleeIdentifier()) {


Index: clang/test/Analysis/no-return.c
===
--- /dev/null
+++ clang/test/Analysis/no-return.c
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+typedef void(fatal_fun)() __attribute__((__noreturn__));
+fatal_fun* fatal_fptr;
+void fatal_decl() __attribute__((__noreturn__));
+
+int rng();
+
+/// This code calls a [[noreturn]] function pointer, which used to be handled
+/// inconsistently between AST builder and CSA.
+/// In the result, CSA produces a path where this function returns non-0.
+int return_zero_or_abort_by_fnptr() {
+  if (rng()) fatal_fptr();
+  return 0;
+}
+
+/// This function calls a [[noreturn]] function.
+/// If it does return, it always returns 0.
+int return_zero_or_abort_by_direct_fun() {
+  if (rng()) fatal_decl();
+  return 0;
+}
+
+/// Trigger a division by zero issue depending on the return value
+/// of the called functions.
+int caller() {
+  int x = 0;
+  // The following if branches must never be taken.
+  if (return_zero_or_abort_by_fnptr())
+return 1 / x; // no-warning: Dead code.
+  if (return_zero_or_abort_by_direct_fun())
+return 1 / x; // no-warning: Dead code.
+
+  // Make sure the warning is still reported when viable.
+  return 1 / x; // expected-warning {{Division by zero}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
@@ -44,9 +44,11 @@
   if (const FunctionDecl *FD = dyn_cast_or_null(CE.getDecl()))
 BuildSinks = FD->hasAttr() || FD->isNo

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};

cor3ntin wrote:
> tbaeder wrote:
> > cor3ntin wrote:
> > > tahonermann wrote:
> > > > As others already noted, additional testing of multicharacter literals 
> > > > and UCNs (including named universal characters like 
> > > > `\N{LATIN_CAPITAL_LETTER_E}` would be beneficial. Some tests of 
> > > > character escapes like `\t` wouldn't hurt either.
> > > > 
> > > > Clang does not yet support use of `-fexec-charset` to set the literal 
> > > > encoding (execution character set) to anything other than UTF-8 though 
> > > > work on that has been done (see D93031). If such work was completed, it 
> > > > would be useful to run some tests against a non-UTF-8 encoding. Maybe 
> > > > next year.
> > > Yes, wide **multicharacter** literals, that's was important information 
> > > missing, thanks for spotting that.
> > > 
> > > I have mixed feeling about adding tests for escape sequences.  Their 
> > > replacement doesn't happen during constant evaluation.
> > > We shouldn't replicate the lexing tests here.
> > > 
> > > but we should compare string literal with byte values. Testing a string 
> > > literal against another one doesn't ensure the code units are correct if 
> > > both are equally miss evaluated.
> > > 
> > > Also we could add explicit tests for null termination here as they are 
> > > added as part of evaluation in theory - but then again that's also 
> > > something clang does earlier.
> > > 
> > > If we want we could consider enabling the byte code interpreter on the 
> > > existing lexing test files, i actually think that's the better way to 
> > > deal with the escape sequences tests.
> > I changed the first test that inspects all characters of a string to 
> > comparing with integers instead. Do you have a suggestion for what lexing 
> > tests to enable the constant interpreter in?
> I think good candidates are
> 
> Lexer/char-escapes.c
> Lexer/char-escapes-delimited.c
> Lexer/char-literal.cpp
Of those, only `Lexer/char-escapes.c` does much validation of literal values. I 
prefer the approach Timm has already taken relative to those tests.

It looks like we don't have an existing set of Sema tests for character and 
string literals. How about we move this test under `clang/test/Sema`? That 
would be the appropriate place to exercise values relative to `-fexec-charset` 
support for non-UTF-8 encodings in the future. If that sounds amenable, then 
I'd like the test split to exercise character and string literals separately.

The character literal tests don't really belong in a test named `arrays.cpp` as 
is.


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

https://reviews.llvm.org/D135366

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

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D135551#3847962 , @dblaikie wrote:

> In D135551#3847607 , @aaron.ballman 
> wrote:
>
>> In D135551#3847444 , @dblaikie 
>> wrote:
>>
>>> I thought this was settled quite a while ago and enshrined in the style 
>>> guide: https://llvm.org/docs/CodingStandards.html#assert-liberally
>>>
>>> `assert(0)` should not be used if something is reachable. We shouldn't have 
>>> a "this violates an invariant, but if you don't have asserts enabled you do 
>>> get some maybe-acceptable behavior".
>>>
>>> I feel fairly strongly that any cases of "reachable asserts" should be 
>>> changed to valid error handling or `llvm_report_error` and remaining 
>>> `assert(0)` should be transformed to `llvm_unreachable`. (also, ideally, 
>>> don't use branch-to-`unreachable` where possible, instead assert the 
>>> condition - in cases where the `if` has side effects, sometimes that's the 
>>> nicest way to write it, but might be clearer, if more verbose to use a 
>>> local variable for the condition, then assert that the variable is true 
>>> (and have the requisite "variable might be unused" cast))
>>
>> I would be okay with that, but that's not what this patch was doing -- it 
>> was changing `assert(0)` into an `llvm_unreachable` more mechanically, and I 
>> don't think that's a valid transformation. The key, to me, is not losing the 
>> distinction between "reaching here is a programming mistake that you'd make 
>> during active development" vs "we never expect to reach this patch and want 
>> to optimize accordingly."
>
> I don't really think those are different things, though. Violating invariants 
> is UB and there's no discussion to be had about how the program (in a 
> non-asserts build) behaves when those invariants are violated - all bets are 
> off, whether it's assert or unreachable.

I think they're the same thing in terms of runtime behavior, but I feel (rather 
strongly) that they're different in terms of documentation when reading the 
source. This code pattern exists and keeps coming up year after year, which is 
sufficient to inform me that the community thinks there is *some* distinction 
to be made there. Also, the fact that we have report_fatal_error *and* 
unreachable APIs signals that we already understand there's a distinction 
between "reaching here will never happen; optimize accordingly" and "reaching 
here is a surprising mistake".

>> `__builtin_unreachable` changes the debugging landscape far too much for me 
>> to want to see folks using it for "reaching here is a programming mistake" 
>> situations, *especially* in RelWithDebInfo builds where optimizations are 
>> enabled and may result in surprising call stacks and time travel debugging.
>
> Generally LLVM's pretty hard to fathom in a non-asserts build anyway, right? 
> (that's the first thing any of us do is reproduce with an assertions build 
> that may fail miles away from where a crash occurred because an invariant was 
> violated much earlier) - that `cast` won't crash/will continue on happily in 
> a non-asserts build seems like a much larger hole to debuggability of a 
> non-asserts build than any unreachable?

This might be true -- personally, I tend to only use debug builds with MSVC 
because RelWithDebInfo isn't sufficient for my daily needs. However, I've 
definitely heard of folks who use RelWithDebInfo for their daily work 
(RelWithDebInfo + Asserts specifically, IIRC) because of the improved build 
times and runtime performance; we should be sure we're not disrupting that 
workflow too much.

 Historically, we've usually used llvm_unreachable for situations where 
 we're saying "this code cannot be reached; if it can, something else has 
 gone seriously wrong." For example, in code like: int foo(SomeEnum E) { 
 switch (E) { case One: return 1; default: return 2; } 
 llvm_unreachable("getting here would be mysterious"); } and we've used 
 assert(0) for situations where we're saying "this code is possible to 
 reach only when there were mistakes elsewhere which broke invariants we 
 were relying on."
>>>
>>> I don't think those are different things though - violating invariants is 
>>> ~= something going seriously wrong.
>>>
>>> (sorry, I guess I should debate this on the thread instead of here - but I 
>>> think most of the folks on that thread did agree with the LLVM style 
>>> guide/the direction here)
>>>
>>> This, I think was also discussed about a decade ago in the LLVM community 
>>> and resulted in r166821/2962d9599e463265edae599285bbc6351f1cc0ef which 
>>> specifically "Suggests llvm_unreachable over assert(0)" and is the policy 
>>> of LLVM - this change is consistent with that policy.
>>
>> I don't have the context for those changes in my email either, but 
>> regardless of what we thought ten years ago, we have code in the code base 
>>

[PATCH] D135682: Fix false positive related to handling of [[noreturn]] function pointers

[Balázs Benics via Phabricator via cfe-commits]

steakhal resigned from this revision.
steakhal added a comment.
Herald added a subscriber: steakhal.

I already reviewed this downstream.
I'll let someone else approve this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135682

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

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/test/AST/Interp/arrays.cpp:143
+
+};

tahonermann wrote:
> cor3ntin wrote:
> > tbaeder wrote:
> > > cor3ntin wrote:
> > > > tahonermann wrote:
> > > > > As others already noted, additional testing of multicharacter 
> > > > > literals and UCNs (including named universal characters like 
> > > > > `\N{LATIN_CAPITAL_LETTER_E}` would be beneficial. Some tests of 
> > > > > character escapes like `\t` wouldn't hurt either.
> > > > > 
> > > > > Clang does not yet support use of `-fexec-charset` to set the literal 
> > > > > encoding (execution character set) to anything other than UTF-8 
> > > > > though work on that has been done (see D93031). If such work was 
> > > > > completed, it would be useful to run some tests against a non-UTF-8 
> > > > > encoding. Maybe next year.
> > > > Yes, wide **multicharacter** literals, that's was important information 
> > > > missing, thanks for spotting that.
> > > > 
> > > > I have mixed feeling about adding tests for escape sequences.  Their 
> > > > replacement doesn't happen during constant evaluation.
> > > > We shouldn't replicate the lexing tests here.
> > > > 
> > > > but we should compare string literal with byte values. Testing a string 
> > > > literal against another one doesn't ensure the code units are correct 
> > > > if both are equally miss evaluated.
> > > > 
> > > > Also we could add explicit tests for null termination here as they are 
> > > > added as part of evaluation in theory - but then again that's also 
> > > > something clang does earlier.
> > > > 
> > > > If we want we could consider enabling the byte code interpreter on the 
> > > > existing lexing test files, i actually think that's the better way to 
> > > > deal with the escape sequences tests.
> > > I changed the first test that inspects all characters of a string to 
> > > comparing with integers instead. Do you have a suggestion for what lexing 
> > > tests to enable the constant interpreter in?
> > I think good candidates are
> > 
> > Lexer/char-escapes.c
> > Lexer/char-escapes-delimited.c
> > Lexer/char-literal.cpp
> Of those, only `Lexer/char-escapes.c` does much validation of literal values. 
> I prefer the approach Timm has already taken relative to those tests.
> 
> It looks like we don't have an existing set of Sema tests for character and 
> string literals. How about we move this test under `clang/test/Sema`? That 
> would be the appropriate place to exercise values relative to 
> `-fexec-charset` support for non-UTF-8 encodings in the future. If that 
> sounds amenable, then I'd like the test split to exercise character and 
> string literals separately.
> 
> The character literal tests don't really belong in a test named `arrays.cpp` 
> as is.
> Of those, only Lexer/char-escapes.c does much validation of literal values. I 
> prefer the approach Timm has already taken relative to those tests.

We can do both, I was not arguing against the test we have here, I'm happy with 
those :)
I'm opposed to duplicate tests for escape sequences here.  using the new 
interpreter on tests that already exist (in addition of the existing run 
commands) is pretty easy and cheap to do.

I don't have opinions how the new interpreter tests are organized.
Ideally we would have a complete set of test that is equally suitable for both 
constexpr engines, but maybe that's something that does not need to be done as 
part of this PR 


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

https://reviews.llvm.org/D135366

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

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

>> Generally LLVM's pretty hard to fathom in a non-asserts build anyway, right? 
>> (that's the first thing any of us do is reproduce with an assertions build 
>> that may fail miles away from where a crash occurred because an invariant 
>> was violated much earlier) - that `cast` won't crash/will continue on 
>> happily in a non-asserts build seems like a much larger hole to 
>> debuggability of a non-asserts build than any unreachable?
>
> This might be true -- personally, I tend to only use debug builds with MSVC 
> because RelWithDebInfo isn't sufficient for my daily needs. However, I've 
> definitely heard of folks who use RelWithDebInfo for their daily work 
> (RelWithDebInfo + Asserts specifically, IIRC) because of the improved build 
> times and runtime performance; we should be sure we're not disrupting that 
> workflow too much.

Changing `assert(0)` to `llvm_unreachable` does not change the behavior of any 
+Asserts build. The behavior of unreachable is the same as assert(0) in a 
+Asserts build.

Is this observation enough to undeadlock this conversation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135551

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

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D134267#3849712 , @dblaikie wrote:

> In D134267#3849673 , @iains wrote:
>
>> 



>> - I do not think this patch fully addresses the issue of harmonising GCC and 
>> clang's command lines for modular builds (because it does not deal with 
>> discovery of modular code in 'normally named' sources), and my investigation 
>> of trying to do this in the driver suggests that it would be much more 
>> complex than doing it in the FE.
>
> Yes, that would be hard to implement in the driver - but my personal taste is 
> that if the compiler's going to treat the input differently, or produce 
> different outputs, it should be observable on the command line - either 
> through different flags (`-x`) or a different file extension. I think that's 
> been historically true? Though I realize it also comes up against "but we 
> have all this C++ with these existing file extensions" and especially if GCC 
> doesn't agree on that philosophy, Clang's going to have a hard time making a 
> stand there... :/

I think that ship has sailed, GCC does, indeed, already take an input like 
"foo.cxx" and automatically determine that it should emit a BMI as well as the 
.O and does so.

It is (IMO, at least) reasonable to forecast that there will be proponents of 
both "we should have extensions to disambiguate" and " the compiler should be 
able to work it out standardised C++ and not force us to rename files in a 
strange way".   The degree of support for either camp will no doubt include 
religious fervour ...

As compiler engineers, I'd say that regardless of which view we might take for 
our own code, we have to cater for both.


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

https://reviews.llvm.org/D134267

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

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D135551#3849816 , @dblaikie wrote:

>>> Generally LLVM's pretty hard to fathom in a non-asserts build anyway, 
>>> right? (that's the first thing any of us do is reproduce with an assertions 
>>> build that may fail miles away from where a crash occurred because an 
>>> invariant was violated much earlier) - that `cast` won't crash/will 
>>> continue on happily in a non-asserts build seems like a much larger hole to 
>>> debuggability of a non-asserts build than any unreachable?
>>
>> This might be true -- personally, I tend to only use debug builds with MSVC 
>> because RelWithDebInfo isn't sufficient for my daily needs. However, I've 
>> definitely heard of folks who use RelWithDebInfo for their daily work 
>> (RelWithDebInfo + Asserts specifically, IIRC) because of the improved build 
>> times and runtime performance; we should be sure we're not disrupting that 
>> workflow too much.
>
> Changing `assert(0)` to `llvm_unreachable` does not change the behavior of 
> any +Asserts build. The behavior of unreachable is the same as assert(0) in a 
> +Asserts build.
>
> Is this observation enough to undeadlock this conversation?

No, it doesn't address the key point I have which is that I want different APIs 
to express intent instead of using `llvm_unreachable` in all cases. To me, it 
is a mistake to label functionally reachable code as being unconditionally 
unreachable. It requires everyone reading that code to understand our nuances 
to know that the API signals aspirationally unreachable code rather than 
functionally unreachable code. These are different scenarios and I don't want 
to lose that distinction in the places where it matters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135551

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

[PATCH] D135397: [clang][dataflow] Add support for a Top value in boolean formulas.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:157
+  TopValue &createTop() {
+return takeOwnership(std::make_unique());
+  }

gribozavr2 wrote:
> ymandel wrote:
> > xazax.hun wrote:
> > > gribozavr2 wrote:
> > > > Should we be creating a new top every time, or should it be a singleton 
> > > > like true and false?
> > > > 
> > > > It seems like we explicitly don't care about its identity (or else it 
> > > > would be isomorphic to AtomicBool).
> > > Good point, a singleton Top might actually simplify some parts of the 
> > > code.
> > Good question. That was my initial implementation, but it ran into the 
> > problem that the solver (in `buildBooleanFormula`) maps each unique (by 
> > pointer) subexpression into a fresh SAT variable in . If we use a singleton 
> > Top, we need to modify that algorithm appropriately. I'm open to 
> > suggestions, though, because of the advantages of a singleton Top.
> > 
> > If we assume that a given `Top` is never actually shared in a boolean 
> > formula (unlike other subexpressions, which may be shared), then we can use 
> > a singleton and special case it in `buildBooleanFormula`. I think that's a 
> > good assumption, but am not sure. Thoughts?
> I see. Could you add some direct tests for the SAT solver with formulas that 
> include Top to check the behavior we care about?
> 
Given the discussion so far, it looks like having a singleton top has its own 
problems. I'm fine with the current solution for now, let's see if the overall 
approach works and we will see if this needs any further optimization later. +1 
to Dmitry, some unit tests like Top && Top generated multiple times will not 
yield the same expression could be useful. If someone in the future tries to 
introduce singleton Top, they will see those tests fail and see why we did not 
have that in the first place and what problems need to be solved to introduce 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135397

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

[PATCH] D135335: [HLSL] Add Resource kind for HLSLResourceAttr.

[Xiang Li via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb267ac49e764: [HLSL] Add Resource kind for HLSLResourceAttr. 
(authored by python3kgae).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135335

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Sema/HLSLExternalSemaSource.cpp
  clang/test/AST/HLSL/RWBuffer-AST.hlsl


Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -38,7 +38,7 @@
 // CHECK-NEXT: CXXRecordDecl 0x{{[0-9A-Fa-f]+}} <>  implicit class RWBuffer definition
 
 // CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
-// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit 
UAV
+// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit 
UAV TypedBuffer
 // CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  
implicit h 'element_type *'
 
 // CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  
operator[] 'element_type &const (unsigned int) const'
@@ -66,5 +66,5 @@
 // CHECK: TemplateArgument type 'float'
 // CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
 // CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
-// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit 
UAV
+// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit 
UAV TypedBuffer
 // CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   
implicit referenced h 'float *'
Index: clang/lib/Sema/HLSLExternalSemaSource.cpp
===
--- clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -116,11 +116,12 @@
   }
 
   BuiltinTypeDeclBuilder &
-  annotateResourceClass(HLSLResourceAttr::ResourceClass RC) {
+  annotateResourceClass(HLSLResourceAttr::ResourceClass RC,
+HLSLResourceAttr::ResourceKind RK) {
 if (Record->isCompleteDefinition())
   return *this;
 Record->addAttr(
-HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RC));
+HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RC, RK));
 return *this;
   }
 
@@ -501,6 +502,7 @@
   .addHandleMember()
   .addDefaultHandleConstructor(*SemaPtr, ResourceClass::UAV)
   .addArraySubscriptOperators()
-  .annotateResourceClass(HLSLResourceAttr::UAV)
+  .annotateResourceClass(HLSLResourceAttr::UAV,
+ HLSLResourceAttr::TypedBuffer)
   .completeDefinition();
 }
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -4037,7 +4037,24 @@
   let Args = [EnumArgument<"ResourceType", "ResourceClass",
["SRV", "UAV", "CBuffer", "Sampler"],
["SRV", "UAV", "CBuffer", "Sampler"]
-   >];
+   >,
+  EnumArgument<"ResourceShape", "ResourceKind",
+   ["Texture1D", "Texture2D", "Texture2DMS",
+"Texture3D", "TextureCube", "Texture1DArray",
+"Texture2DArray", "Texture2DMSArray",
+"TextureCubeArray", "TypedBuffer", "RawBuffer",
+"StructuredBuffer", "CBufferKind", "SamplerKind",
+"TBuffer", "RTAccelerationStructure", 
"FeedbackTexture2D",
+"FeedbackTexture2DArray"],
+   ["Texture1D", "Texture2D", "Texture2DMS",
+"Texture3D", "TextureCube", "Texture1DArray",
+"Texture2DArray", "Texture2DMSArray",
+"TextureCubeArray", "TypedBuffer", "RawBuffer",
+"StructuredBuffer", "CBufferKind", "SamplerKind",
+"TBuffer", "RTAccelerationStructure", 
"FeedbackTexture2D",
+"FeedbackTexture2DArray"]
+>
+  ];
   let Documentation = [InternalOnly];
 }
 


Index: clang/test/AST/HLSL/RWBuffer-AST.hlsl
===
--- clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -38,7 +38,7 @@
 // CHECK-NEXT: CXXRecordDecl 0x{{[0-9A-Fa-f]+}} <>  implicit class RWBuffer definition
 
 // CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
-// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV
+// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit UAV TypedBuffer
 // CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  implicit h 'element_type *'
 
 // CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  operator[] 'element_type &const (unsigned int) const'
@@ -66,5 +66,5 @@
 // CHECK: TemplateArgumen

[clang] b267ac4 - [HLSL] Add Resource kind for HLSLResourceAttr.

[Xiang Li via cfe-commits]

Author: Xiang Li
Date: 2022-10-11T09:41:14-07:00
New Revision: b267ac49e764c65322cf772647ff26d6732e5134

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

LOG: [HLSL] Add Resource kind for HLSLResourceAttr.

A new EnumArgument ResourceKind is added for HLSLResourceAttr.
This will be use to get resource kind instead of parse it from the type name.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/include/clang/Basic/Attr.td
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/test/AST/HLSL/RWBuffer-AST.hlsl

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index fa718b51d8134..fddf097daaafd 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4037,7 +4037,24 @@ def HLSLResource : InheritableAttr {
   let Args = [EnumArgument<"ResourceType", "ResourceClass",
["SRV", "UAV", "CBuffer", "Sampler"],
["SRV", "UAV", "CBuffer", "Sampler"]
-   >];
+   >,
+  EnumArgument<"ResourceShape", "ResourceKind",
+   ["Texture1D", "Texture2D", "Texture2DMS",
+"Texture3D", "TextureCube", "Texture1DArray",
+"Texture2DArray", "Texture2DMSArray",
+"TextureCubeArray", "TypedBuffer", "RawBuffer",
+"StructuredBuffer", "CBufferKind", "SamplerKind",
+"TBuffer", "RTAccelerationStructure", 
"FeedbackTexture2D",
+"FeedbackTexture2DArray"],
+   ["Texture1D", "Texture2D", "Texture2DMS",
+"Texture3D", "TextureCube", "Texture1DArray",
+"Texture2DArray", "Texture2DMSArray",
+"TextureCubeArray", "TypedBuffer", "RawBuffer",
+"StructuredBuffer", "CBufferKind", "SamplerKind",
+"TBuffer", "RTAccelerationStructure", 
"FeedbackTexture2D",
+"FeedbackTexture2DArray"]
+>
+  ];
   let Documentation = [InternalOnly];
 }
 

diff  --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp 
b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 681154d52cb33..3794a989be11a 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -116,11 +116,12 @@ struct BuiltinTypeDeclBuilder {
   }
 
   BuiltinTypeDeclBuilder &
-  annotateResourceClass(HLSLResourceAttr::ResourceClass RC) {
+  annotateResourceClass(HLSLResourceAttr::ResourceClass RC,
+HLSLResourceAttr::ResourceKind RK) {
 if (Record->isCompleteDefinition())
   return *this;
 Record->addAttr(
-HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RC));
+HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RC, RK));
 return *this;
   }
 
@@ -501,6 +502,7 @@ void 
HLSLExternalSemaSource::completeBufferType(CXXRecordDecl *Record) {
   .addHandleMember()
   .addDefaultHandleConstructor(*SemaPtr, ResourceClass::UAV)
   .addArraySubscriptOperators()
-  .annotateResourceClass(HLSLResourceAttr::UAV)
+  .annotateResourceClass(HLSLResourceAttr::UAV,
+ HLSLResourceAttr::TypedBuffer)
   .completeDefinition();
 }

diff  --git a/clang/test/AST/HLSL/RWBuffer-AST.hlsl 
b/clang/test/AST/HLSL/RWBuffer-AST.hlsl
index 80f77f97d2ef0..0929462e51831 100644
--- a/clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ b/clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -38,7 +38,7 @@ RWBuffer Buffer;
 // CHECK-NEXT: CXXRecordDecl 0x{{[0-9A-Fa-f]+}} <>  implicit class RWBuffer definition
 
 // CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
-// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit 
UAV
+// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit 
UAV TypedBuffer
 // CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>  
implicit h 'element_type *'
 
 // CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  
operator[] 'element_type &const (unsigned int) const'
@@ -66,5 +66,5 @@ RWBuffer Buffer;
 // CHECK: TemplateArgument type 'float'
 // CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
 // CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <> Implicit final
-// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit 
UAV
+// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <> Implicit 
UAV TypedBuffer
 // CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <>   
implicit referenced h 'float *'



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

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

[YingChi Long via Phabricator via cfe-commits]

inclyc added a comment.

> - Prefer llvm_report_error() in any circumstance under which a code path is 
> functionally possible to reach, but only in erroneous executions that signify 
> a mistake on the part of the LLVM developer elsewhere in the program.
> - Prefer llvm_unreachable() in any circumstance under which a code path is 
> believed to be functionally impossible to reach (even if technically possible 
> to reach). The API is now self-documenting to mean "this code really should 
> be totally unreachable".

I think `llvm_unreachable` already has the functionality reporting bugs for 
developer in our implementation, with +Assertions by default


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135551

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

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D135551#3849944 , @inclyc wrote:

>> - Prefer llvm_report_error() in any circumstance under which a code path is 
>> functionally possible to reach, but only in erroneous executions that 
>> signify a mistake on the part of the LLVM developer elsewhere in the program.
>> - Prefer llvm_unreachable() in any circumstance under which a code path is 
>> believed to be functionally impossible to reach (even if technically 
>> possible to reach). The API is now self-documenting to mean "this code 
>> really should be totally unreachable".
>
> I think `llvm_unreachable` already has the functionality reporting bugs for 
> developer in our implementation, with +Assertions by default

Yes, in terms of its runtime behavior. So long as we're not making debugging 
harder for some large group of people, the runtime behavior is not really what 
I'm concerned by though. I'm focusing more on code reviewers and project 
newcomers and whether our code is self-documenting or not. Having a policy to 
use an API that says code is not reachable in situations where that code is 
very much reachable is the crux of my problem -- the API is sometimes a lie 
(and a lie with optimization impacts, at that) and we force everyone to pay the 
cognitive costs associated with that when reading code.

If we end up with two APIs that have the same runtime behavior, I'm okay with 
that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135551

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

[PATCH] D135696: [pseudo] Document disambiguation design progress

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, alextsao1999.
Herald added a project: clang-tools-extra.

Need to take a break from this, so write down where we got to.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135696

Files:
  clang-tools-extra/pseudo/Disambiguation.md

Index: clang-tools-extra/pseudo/Disambiguation.md
===
--- /dev/null
+++ clang-tools-extra/pseudo/Disambiguation.md
@@ -0,0 +1,367 @@
+# Disambiguation
+
+The C++ grammar is highly ambiguous, so the GLR parser produces a forest of
+parses, represented compactly by a DAG.
+A real C++ parser finds the correct parse through semantic analysis: mostly
+resolving names. But we can't do that, as we don't parse the headers.
+
+Our disambiguation phase should take the parse forest, and choose a single parse
+tree that is most likely.
+It might **optionally** use some other hints (e.g. coding style, or what
+specific names tend to mean in this codebase).
+
+There are some grammatical ambiguities that can be resolved without semantic
+analysis, e.g. whether `int {}` is a function-definition.
+We eliminate these earlier e.g., with rule guards. By "disambiguation" we mean
+choosing between interpretations that we can't reject confidently and locally.
+
+## Types of evidence
+
+We have limited information to go on, and strive to use similar heuristics a
+human reader might.
+
+### Likely and unlikely structure
+
+In some cases, the shape of a particular interpretation is unlikely but not
+impossible. For example, the statement `x(a);` might:
+
+- call a function `x` (likely) 
+- construct a temporary of class type `x` (less likely)
+- define a variable `a` of type `x`, which is an alias for e.g. `int`
+  (unlikely!)
+
+We model this as a bonus/penalty for including a particular forest node in the
+chosen parse. For each rule we want to apply, we can write some code to
+recognize the corresponding pattern in the parse tree, and run these recognizers
+at each node to assign the bonuses.
+
+### Interpreting names
+
+Just as resolving names allows a C++ parser to choose the right parse (rejecting
+others), chunks of a parse tree imply things about how names resolve.
+
+Because the same name often means the same thing in different contexts, we can
+apply knowledge from elsewhere. This can be as simple as knowing "`vector` is
+usually a type", and adding bonuses to nodes that include that interpretation.
+
+However we can also transfer knowlegde across the same file we're parsing:
+
+```cpp
+// Is `Builder` a class or a namespace?
+void Builder::build() { ... }
+// ...
+// `Builder` is a type.
+Builder b;
+```
+
+We can use this to understand more-ambiguous code based on names in a section
+we're more sure about. It also pushes us to provide a consistent parse, rather
+than interpreting each occurrence of an unclear name differently.
+
+Again, we can define bonuses/penalties for forest nodes that interpret names,
+but this time those bonuses change as we disambiguate. Specifically:
+
+- we can group identifiers into **classes**, most importantly "all identifiers
+  with text 'foo'" but also "all snake_case identifiers".
+- clusters of nodes immediately above the identifiers in the parse forest are
+  **interpretations**, they bind the identifier to a **kind** such "type",
+  "value", "namespace", "other".
+- for each class we can query information once from an external source (such as
+  an index or hard-coded list), yielding a vector of weights (one per kind)
+- at each point we can compute metrics based on interpretations in the forest:
+   - the number of identifiers in the class that are interpreted as each kind
+ (e.g. *all* remaining interpretations of 'foo' at 3:7 are 'type')
+   - the number of identifiers in the class that *may* be interpereted as each
+ kind (e.g. 'foo' at 3:7 might be a 'type').
+- we can mash these metrics together into a vector of bonuses for a class (e.g.
+  for identifiers with text 'bar', 'type'=>+5, 'namespace'=>+1, 'value'=>-2).
+- these bonuses are assigned to corresponding interpretations in the graph
+
+### Templates
+
+Another aspect of a name is whether it names a template (type or value). This 
+is ambiguous in many more cases since CTAD allowed template arguments to be
+omitted.
+
+A fairly simple heuristic appears sufficient here: things that look like
+templates usually are, so if a node for certain rules exists in the forest
+(e.g. `template-id := template-name < template-argument-list >`) then we treat
+the template name as a probable template, and apply a bonus to every node that
+interprets it that way. We do this even if alternate parses are possible
+(`a < b > :: c` might be a comparison, but is still evidence `a` is a template).
+
+## Algorithm sketch
+
+With static node scores, find

[PATCH] D135696: [pseudo] Document disambiguation design progress

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

The diagrams are rendered by github's markdown view.
My favorite offline tool to preview them is the "Markdown Preview Mermaid 
Support" VSCode extension.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135696

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

[PATCH] D135384: [AIX] Enable the use of the -pg flag

[Michael Francis via Phabricator via cfe-commits]

francii updated this revision to Diff 466852.
francii added a comment.

Updated target triple check to use isOSAIX()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135384

Files:
  clang/test/CodeGen/mcount-aix.c
  llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp


Index: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
===
--- llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/Dominators.h"
@@ -34,9 +35,25 @@
   Func == "__mcount" ||
   Func == "_mcount" ||
   Func == "__cyg_profile_func_enter_bare") {
-FunctionCallee Fn = M.getOrInsertFunction(Func, Type::getVoidTy(C));
-CallInst *Call = CallInst::Create(Fn, "", InsertionPt);
-Call->setDebugLoc(DL);
+const Twine targetTwine(M.getTargetTriple());
+Triple targetTriple(targetTwine);
+if (targetTriple.isOSAIX()) {
+  Type *SizeTy = M.getDataLayout().getIntPtrType(C);
+  Type *SizePtrTy = SizeTy->getPointerTo();
+  GlobalVariable *GV = new GlobalVariable(M, SizeTy, /*isConstant=*/false,
+  GlobalValue::InternalLinkage,
+  ConstantInt::get(SizeTy, 0));
+  CallInst *Call = CallInst::Create(
+  M.getOrInsertFunction(Func,
+FunctionType::get(Type::getVoidTy(C), 
{SizePtrTy},
+  /*isVarArg=*/false)),
+  {GV}, "", InsertionPt);
+  Call->setDebugLoc(DL);
+} else {
+  FunctionCallee Fn = M.getOrInsertFunction(Func, Type::getVoidTy(C));
+  CallInst *Call = CallInst::Create(Fn, "", InsertionPt);
+  Call->setDebugLoc(DL);
+}
 return;
   }
 
Index: clang/test/CodeGen/mcount-aix.c
===
--- /dev/null
+++ clang/test/CodeGen/mcount-aix.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -pg -triple powerpc-ibm-aix7.2.0.0 -S -emit-llvm %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -pg -triple powerpc64-ibm-aix7.2.0.0 -S -emit-llvm %s -o - 
| FileCheck %s -check-prefix=CHECK64
+
+void foo() {
+}
+
+void bar() {
+foo();
+}
+// CHECK: @[[GLOB0:[0-9]+]] = internal global i32 0
+// CHECK: @[[GLOB1:[0-9]+]] = internal global i32 0
+// CHECK64: @[[GLOB0:[0-9]+]] = internal global i64 0
+// CHECK64: @[[GLOB1:[0-9]+]] = internal global i64 0
+// CHECK-LABEL: @foo(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:call void @__mcount(ptr @[[GLOB0]])
+// CHECK64-LABEL: @foo(
+// CHECK64-NEXT:  entry:
+// CHECK64-NEXT:call void @__mcount(ptr @[[GLOB0]])
+// CHECK-LABEL: @bar(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:call void @__mcount(ptr @[[GLOB1]])
+// CHECK64-LABEL: @bar(
+// CHECK64-NEXT:  entry:
+// CHECK64-NEXT:call void @__mcount(ptr @[[GLOB1]])


Index: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
===
--- llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
+++ llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/Transforms/Utils/EntryExitInstrumenter.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/Dominators.h"
@@ -34,9 +35,25 @@
   Func == "__mcount" ||
   Func == "_mcount" ||
   Func == "__cyg_profile_func_enter_bare") {
-FunctionCallee Fn = M.getOrInsertFunction(Func, Type::getVoidTy(C));
-CallInst *Call = CallInst::Create(Fn, "", InsertionPt);
-Call->setDebugLoc(DL);
+const Twine targetTwine(M.getTargetTriple());
+Triple targetTriple(targetTwine);
+if (targetTriple.isOSAIX()) {
+  Type *SizeTy = M.getDataLayout().getIntPtrType(C);
+  Type *SizePtrTy = SizeTy->getPointerTo();
+  GlobalVariable *GV = new GlobalVariable(M, SizeTy, /*isConstant=*/false,
+  GlobalValue::InternalLinkage,
+  ConstantInt::get(SizeTy, 0));
+  CallInst *Call = CallInst::Create(
+  M.getOrInsertFunction(Func,
+FunctionType::get(Type::getVoidTy(C), {SizePtrTy},
+  /*isVarArg=*/false)),
+  {GV}, "", InsertionPt);
+  Call->setDebugLoc(DL);
+} else {
+  FunctionCallee Fn = M.getOrInsertFunction(Func, Type::getVoidTy(C));
+  CallInst *Call = CallInst::Create(Fn, "", InsertionPt);
+ 

[clang] 2c94f75 - [clang-format] update --files help description

[Yuanfang Chen via cfe-commits]

Author: Yuanfang Chen
Date: 2022-10-11T10:25:04-07:00
New Revision: 2c94f75f00af16b6b4c2336e5cbfd8187d8bf33c

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

LOG: [clang-format] update --files help description

correlates the option with reponse file concept.

Reviewed By: HazardyKnusperkeks

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

Added: 


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

Removed: 




diff  --git a/clang/docs/ClangFormat.rst b/clang/docs/ClangFormat.rst
index ec5489785804a..2ad3b50339eba 100644
--- a/clang/docs/ClangFormat.rst
+++ b/clang/docs/ClangFormat.rst
@@ -26,7 +26,7 @@ to format C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# 
code.
   together with s, the files are edited in-place. Otherwise, the
   result is written to the standard output.
 
-  USAGE: clang-format [options] [ ...]
+  USAGE: clang-format [options] [@] [ ...]
 
   OPTIONS:
 
@@ -69,7 +69,8 @@ to format C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# 
code.
 --ferror-limit=  - Set the maximum number of clang-format 
errors to emit
  before stopping (0 = no limit).
  Used only with --dry-run or -n
---files=   - Provide a list of files to run 
clang-format
+--files= - A file containing a list of files to 
process, one
+ per line.
 -i - Inplace edit s, if specified.
 --length=- Format a range of this length (in bytes).
  Multiple ranges can be formatted by 
specifying

diff  --git a/clang/tools/clang-format/ClangFormat.cpp 
b/clang/tools/clang-format/ClangFormat.cpp
index 269bc59506b2d..9e8b881c8fafe 100644
--- a/clang/tools/clang-format/ClangFormat.cpp
+++ b/clang/tools/clang-format/ClangFormat.cpp
@@ -125,9 +125,11 @@ static cl::opt QualifierAlignment(
  "determined by the QualifierAlignment style flag"),
 cl::init(""), cl::cat(ClangFormatCategory));
 
-static cl::opt
-Files("files", cl::desc("Provide a list of files to run clang-format"),
-  cl::init(""), cl::cat(ClangFormatCategory));
+static cl::opt Files(
+"files",
+cl::desc("A file containing a list of files to process, one per line."),
+cl::value_desc("filename"),
+cl::init(""), cl::cat(ClangFormatCategory));
 
 static cl::opt
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),



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

[PATCH] D135115: [clang-format] update --files help description

[Yuanfang Chen 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 rG2c94f75f00af: [clang-format] update --files help description 
(authored by ychen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135115

Files:
  clang/docs/ClangFormat.rst
  clang/tools/clang-format/ClangFormat.cpp


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -125,9 +125,11 @@
  "determined by the QualifierAlignment style flag"),
 cl::init(""), cl::cat(ClangFormatCategory));
 
-static cl::opt
-Files("files", cl::desc("Provide a list of files to run clang-format"),
-  cl::init(""), cl::cat(ClangFormatCategory));
+static cl::opt Files(
+"files",
+cl::desc("A file containing a list of files to process, one per line."),
+cl::value_desc("filename"),
+cl::init(""), cl::cat(ClangFormatCategory));
 
 static cl::opt
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
Index: clang/docs/ClangFormat.rst
===
--- clang/docs/ClangFormat.rst
+++ clang/docs/ClangFormat.rst
@@ -26,7 +26,7 @@
   together with s, the files are edited in-place. Otherwise, the
   result is written to the standard output.
 
-  USAGE: clang-format [options] [ ...]
+  USAGE: clang-format [options] [@] [ ...]
 
   OPTIONS:
 
@@ -69,7 +69,8 @@
 --ferror-limit=  - Set the maximum number of clang-format 
errors to emit
  before stopping (0 = no limit).
  Used only with --dry-run or -n
---files=   - Provide a list of files to run 
clang-format
+--files= - A file containing a list of files to 
process, one
+ per line.
 -i - Inplace edit s, if specified.
 --length=- Format a range of this length (in bytes).
  Multiple ranges can be formatted by 
specifying


Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -125,9 +125,11 @@
  "determined by the QualifierAlignment style flag"),
 cl::init(""), cl::cat(ClangFormatCategory));
 
-static cl::opt
-Files("files", cl::desc("Provide a list of files to run clang-format"),
-  cl::init(""), cl::cat(ClangFormatCategory));
+static cl::opt Files(
+"files",
+cl::desc("A file containing a list of files to process, one per line."),
+cl::value_desc("filename"),
+cl::init(""), cl::cat(ClangFormatCategory));
 
 static cl::opt
 Verbose("verbose", cl::desc("If set, shows the list of processed files"),
Index: clang/docs/ClangFormat.rst
===
--- clang/docs/ClangFormat.rst
+++ clang/docs/ClangFormat.rst
@@ -26,7 +26,7 @@
   together with s, the files are edited in-place. Otherwise, the
   result is written to the standard output.
 
-  USAGE: clang-format [options] [ ...]
+  USAGE: clang-format [options] [@] [ ...]
 
   OPTIONS:
 
@@ -69,7 +69,8 @@
 --ferror-limit=  - Set the maximum number of clang-format errors to emit
  before stopping (0 = no limit).
  Used only with --dry-run or -n
---files=   - Provide a list of files to run clang-format
+--files= - A file containing a list of files to process, one
+ per line.
 -i - Inplace edit s, if specified.
 --length=- Format a range of this length (in bytes).
  Multiple ranges can be formatted by specifying
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: erichkeane.
aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/ODRDiagsEmitter.h:119
 
+  /// Diagnose ODR mismatch in attributes between 2 Decl.
+  ///





Comment at: clang/lib/AST/ODRDiagsEmitter.cpp:159
 
 bool ODRDiagsEmitter::diagnoseSubMismatchTypedef(
 const NamedDecl *FirstRecord, StringRef FirstModule, StringRef 
SecondModule,

Typedefs can have attributes, so should this be updated as well? (alignment, 
ext vector types, mode attributes, etc all come to mind)



Comment at: clang/lib/AST/ODRDiagsEmitter.cpp:302
+llvm::raw_string_ostream OutputStream(FullText);
+A->printPretty(OutputStream, Ctx.getPrintingPolicy());
+return OutputStream.str();

Do we want to have more control over the printing policy here? e.g., do we 
really want to claim an ODR difference if one module's printing policy 
specifies indentation of 8 and another specifies indentation of 4? Or if one 
module prints `restrict` while another prints `__restrict`, etc?



Comment at: clang/lib/AST/ODRDiagsEmitter.cpp:311
+   << FirstModule.empty() << FirstModule << (FirstContainer != nullptr)
+   << FirstDecl->getIdentifier() << DiffType;
+  };

You can probably drop the `getIdentifier()` here as the diagnostics engine 
knows how to print named declarations properly already.



Comment at: clang/lib/AST/ODRDiagsEmitter.cpp:318
+   << SecondModule << (FirstContainer != nullptr)
+   << SecondDecl->getIdentifier() << DiffType;
+  };

Same here.



Comment at: clang/lib/AST/ODRHash.cpp:479-480
+
+  llvm::copy_if(D->attrs(), std::back_inserter(HashableAttrs),
+[](const Attr *A) { return !A->isImplicit(); });
+}

ChuanqiXu wrote:
> vsapsai wrote:
> > I'm not sure `isImplicit` is the best indicator of attributes to check, so 
> > suggestions in this area are welcome. I think we can start strict and relax 
> > some of the checks if needed.
> > 
> > If people have strong opinions that some attributes shouldn't be ignored, 
> > we can add them to the tests to avoid regressions. Personally, I believe 
> > that alignment and packed attributes should never be silently ignored.
> Agreed. I feel `isImplicit` is enough for now.
The tricky part is -- sometimes certain attributes add additional implicit 
attributes and those implicit attributes matter 
(https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclAttr.cpp#L9380).
 And some attributes seem to just do the wrong thing entirely: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclAttr.cpp#L7344

So I think `isImplicit()` is a good approximation, but I'm more wondering what 
the principle is for whether an attribute should or should not be considered 
part of the ODR hash. Type attributes, attributes that impact object layout, 
etc all seem like they should almost certainly be part of ODR hashing. Others 
are a bit more questionable though.

I think this is something that may need a per-attribute flag in Attr.td so 
attributes can opt in or out of it because I'm not certain what ODR issues 
could stem from `[[maybe_unused]]` or `[[deprecated]]` disagreements across 
module boundaries.



Comment at: clang/lib/AST/ODRHash.cpp:494
+
+  // FIXME: This should be auto-generated as part of Attr.td
+  switch (A->getKind()) {

100% agreed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135472

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

[PATCH] D133338: [clang][PowerPC] PPC64 VAArg use coerced integer type for direct aggregate fits in register

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

This seems be to a genuine target difference, right?  PPC64 has this ABI rule:

> An aggregate or union smaller than one doubleword in size is padded so that 
> it appears in the least significant bits of the doubleword.

which overrides the standard rule for passing aggregates:

> Fixed size aggregates and unions passed by value are mapped to as many 
> doublewords of the parameter save area as the value uses in memory. 
> Aggregrates and unions are aligned according to their alignment requirements. 
> This may result in doublewords being skipped for alignment.

Other big-endian targets don't do this to non-fundamental types as far as I can 
tell.  So I don't really get the TODO in the comment; this does seem to be 
something that ABIs need to pass in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D18

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

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Most of my concerns change based on the feedback others have given, so after 
those are dealt with, I'll do another depever view.




Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:852
+def err_module_odr_violation_attribute : Error<
+  "%q0 has different definitions in different modules; first difference is "
+  "%select{definition in module '%2'|defined here}1 found"

Wowzer these are tough to read... can you provide a magic-decoder ring for me 
of some sort?



Comment at: clang/lib/AST/ODRHash.cpp:479-480
+
+  llvm::copy_if(D->attrs(), std::back_inserter(HashableAttrs),
+[](const Attr *A) { return !A->isImplicit(); });
+}

aaron.ballman wrote:
> ChuanqiXu wrote:
> > vsapsai wrote:
> > > I'm not sure `isImplicit` is the best indicator of attributes to check, 
> > > so suggestions in this area are welcome. I think we can start strict and 
> > > relax some of the checks if needed.
> > > 
> > > If people have strong opinions that some attributes shouldn't be ignored, 
> > > we can add them to the tests to avoid regressions. Personally, I believe 
> > > that alignment and packed attributes should never be silently ignored.
> > Agreed. I feel `isImplicit` is enough for now.
> The tricky part is -- sometimes certain attributes add additional implicit 
> attributes and those implicit attributes matter 
> (https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclAttr.cpp#L9380).
>  And some attributes seem to just do the wrong thing entirely: 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclAttr.cpp#L7344
> 
> So I think `isImplicit()` is a good approximation, but I'm more wondering 
> what the principle is for whether an attribute should or should not be 
> considered part of the ODR hash. Type attributes, attributes that impact 
> object layout, etc all seem like they should almost certainly be part of ODR 
> hashing. Others are a bit more questionable though.
> 
> I think this is something that may need a per-attribute flag in Attr.td so 
> attributes can opt in or out of it because I'm not certain what ODR issues 
> could stem from `[[maybe_unused]]` or `[[deprecated]]` disagreements across 
> module boundaries.
I don't think 'isImplicit' is particularly good.  I think the idea of 
auto-adding 'type' attributes and having the 'rest' be analyzed to figure out 
which are important.

Alternatively, I wonder if we're better off just adding ALL attributes and 
seeing what the fallout is. We can later decide when we don't want them to be 
ODR significant (which, might be OTHERWISE meaningful later!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135472

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

[PATCH] D134371: [clang-doc] Add typedef/using information.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 466858.
brettw added a comment.

Fixed -Wall issues.


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

https://reviews.llvm.org/D134371

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.h
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,15 +28,16 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace, "path/to/A/Namespace");
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/Namespace");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildFunctions.back().Access = AccessSpecifier::AS_none;
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
+ InfoType::IT_namespace,
+ "path/to/A/Namespace");
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/Namespace");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Functions.back().Access = AccessSpecifier::AS_none;
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -100,8 +101,8 @@
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
-  I.Bases.back().ChildFunctions.emplace_back();
-  I.Bases.back().ChildFunctions.back().Name = "InheritedFunctionOne";
+  I.Bases.back().Children.Functions.emplace_back();
+  I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
   I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
@@ -109,12 +110,12 @@
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/r");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -330,6 +331,30 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
+TEST(YAMLGeneratorTest, enumTypedefYAML) {
+  TypedefInfo I;
+  I.Name = "MyUsing";
+  I.Underlying = TypeInfo("int");
+  I.IsUsing = true;
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'MyUsing'
+Underlying:
+  Name:'int'
+IsUsing: true
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
 TEST(YAMLGeneratorTest, emitCommentYAML) {
   FunctionInfo I;
   I.Name = "f";
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/Serializ

[PATCH] D134371: [clang-doc] Add typedef/using information.

[Brett Wilson via Phabricator via cfe-commits]

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

New patch up to fix warnings that caused the revert.


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

https://reviews.llvm.org/D134371

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

[PATCH] D135490: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic

[Steven Wu via Phabricator via cfe-commits]

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

Ok with me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135490

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

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

[YingChi Long via Phabricator via cfe-commits]

inclyc added a comment.

In D135551#3849983 , @aaron.ballman 
wrote:

> In D135551#3849944 , @inclyc wrote:
>
>>> - Prefer llvm_report_error() in any circumstance under which a code path is 
>>> functionally possible to reach, but only in erroneous executions that 
>>> signify a mistake on the part of the LLVM developer elsewhere in the 
>>> program.
>>> - Prefer llvm_unreachable() in any circumstance under which a code path is 
>>> believed to be functionally impossible to reach (even if technically 
>>> possible to reach). The API is now self-documenting to mean "this code 
>>> really should be totally unreachable".
>>
>> I think `llvm_unreachable` already has the functionality reporting bugs for 
>> developer in our implementation, with +Assertions by default
>
> Yes, in terms of its runtime behavior. So long as we're not making debugging 
> harder for some large group of people, the runtime behavior is not really 
> what I'm concerned by though. I'm focusing more on code reviewers and project 
> newcomers and whether our code is self-documenting or not. Having a policy to 
> use an API that says code is not reachable in situations where that code is 
> very much reachable is the crux of my problem -- the API is sometimes a lie 
> (and a lie with optimization impacts, at that) and we force everyone to pay 
> the cognitive costs associated with that when reading code.
>
> If we end up with two APIs that have the same runtime behavior, I'm okay with 
> that.

Could you elaborate on "aspirationally" vs "functionally" unreachable code here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135551

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

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

Looks good to me except for one nit. And the test still fails on Windows for 
some reason.




Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1612
+  time_t TT = *getPreprocessorOpts().SourceDateEpoch;
+  std::tm *TM = std::gmtime(&TT);
   Result = asctime(TM);

To be consistent with existing code, it might be better to use `std::localtime` 
instead of `std::gmtime`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135045

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

[PATCH] D134853: [clang-format] Correctly annotate UDLs as OverloadedOperator

[Emilia Dreamer via Phabricator via cfe-commits]

rymiel updated this revision to Diff 466862.
rymiel marked an inline comment as done.
rymiel added a comment.

Handle UDLs with and without spaces

Thank you @owenpan, I was not actually aware UDLs without spaces were valid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134853

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -376,6 +376,66 @@
   EXPECT_TOKEN(Tokens[11], tok::amp, TT_PointerOrReference);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsOverloadedOperators) {
+  auto Tokens = annotate("x.operator+()");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::plus, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator=()");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::equal, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator+=()");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::plusequal, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator,()");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::comma, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator()()");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::r_paren, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator[]()");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  // EXPECT_TOKEN(Tokens[3], tok::l_square, TT_OverloadedOperator);
+  // EXPECT_TOKEN(Tokens[4], tok::r_square, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator\"\"_a()");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator\"\" _a()");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::identifier, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator\"\"_long_literal()");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator\"\" _long_literal()");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::identifier, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator\"\"if()");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator\"\"s()");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_OverloadedOperatorLParen);
+  Tokens = annotate("x.operator\"\" s()");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::string_literal, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[4], tok::identifier, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::l_paren, TT_OverloadedOperatorLParen);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresClausesAndConcepts) {
   auto Tokens = annotate("template \n"
  "concept C = (Foo && Bar) && (Bar && Baz);");
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10153,6 +10153,14 @@
   // verifyFormat("void f() { operator*(a & a); }");
   // verifyFormat("void f() { operator&(a, b * b); }");
 
+  verifyFormat("void f() { return operator()(x) * b; }");
+  verifyFormat("void f() { return operator[](x) * b; }");
+  verifyFormat("void f() { return operator\"\"_a(x) * b; }");
+  verifyFormat("void f() { return operator\"\" _a(x) * b; }");
+  verifyFormat("void f() { return operator\"\"s(x) * b; }");
+  verifyForm

[PATCH] D129443: [clang-format] Add option for aligning requires clause body

[Emilia Dreamer via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Format/FormatTest.cpp:20036
   Style.Language = FormatStyle::LK_Cpp;
+  CHECK_PARSE_BOOL(AlignRequiresClauseBody);
   CHECK_PARSE_BOOL(AlignTrailingComments);

owenpan wrote:
> HazardyKnusperkeks wrote:
> > Please add the parsing test for the enum.
> @rymiel Is this done?
The code this comment was added on has been completely removed; so i guess i'll 
mark it as done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129443

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

[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/records.cpp:139
+  // ref-note {{in call to 'foo()'}}
+};

The other thing I think we need some tests for are constructor and destructor 
calls where the `this` pointer may be a bit surprising because it needs 
adjustments. For example, with multiple inheritance where the `this` pointer 
may need to be adjusted to get to the fields of the object, and ensuring the 
correct constructors are called in the correct order.

Another case is with virtual functions (I'm assuming there's no vtable support 
yet and so that's less interesting, but it will become interesting once we get 
there so you may want to keep it in mind).


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

https://reviews.llvm.org/D134699

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

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D129755#3849540 , @aaron.ballman 
wrote:

>>> In D129755#3842744 , @hans wrote:
>>>
 
>>>
>>> I'll give you some time to reply before I reland, but I can't see a bug 
>>> here. For the next time, as already pointed out, give the involved people 
>>> some time to reply before you revert (unless it's a crash). We have to be 
>>> able to continue working on the compiler without having Google revert 
>>> changes all the time because we produce new warnings.
>>
>> We've seen reports here of various breakages from two different projects in 
>> short time after your patch. The general policy is to revert to green, so I 
>> think that was the right call.
>
> I don't think it was the wrong call per se, but it was an aggressive revert 
> IMO. Chromium tends to revert changes very quickly and without discussion, 
> and I'm hoping we can change that slightly to include more up-front 
> discussion before a revert. It's one thing to revert when the patch author 
> isn't responsive, but reverting immediately and without discussion when the 
> commit message explicitly states there is a change in behavior isn't how I 
> would expect the revert policy to be interpreted.

In general I think us reverting fast is a good thing. Reverts and relands are 
cheap, and reverting early prevents more people from being exposed to 
breakages, which are expensive to investigate. Minimizing the amount time where 
the source tree is in a bad state seems like a good thing to me, and reverts 
also tend to become harder and more disruptive with time as more work lands on 
top.

I certainly don't want us to come across as aggressive though, and since for 
this patch it turned out that the new false positives were expected, in 
hindsight I probably shouldn't have been so quick to revert.

Since the bug that Gulfem reported is fixed, relanding sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129755

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