[PATCH] D94067: [clang][ASTImporter] Fix a possible assertion failure `NeedsInjectedClassNameType(Decl)'.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:2901
+  // Skip the declaration if injected type is already set.
+  if (isa(RI->getTypeForDecl()))
+continue;

shafik wrote:
> Is this to fix the bug or is this for efficiency sake?
This is not needed for the fix, it was used in the first version of the fix 
(still only for efficiency). In the current form this looks like unrelated 
change (the old fix included other code at the same location) so I am not 
against removing this part (but add it in a separate change).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94067

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

[PATCH] D89490: Introduce __attribute__((darwin_abi))

[Adrien Guinet via Phabricator via cfe-commits]

aguinet added a comment.

In D89490#2481306 , @rjmccall wrote:

> I do feel like this is a terrible idea, sorry.  It's a *very* niche use case 
> to be dedicating a new compiler feature to, and it's a feature that demands a 
> lot from the internal compiler architecture in ways that other features don't.

I'd say it's as much as a niche use case as `__attribute__((ms_abi))`? But the 
"niche use case" is Wine, which I believe has a consequent user base. I'm not 
saying the goal of this patch is to be able to do Wine for ARM64/iOS, but I 
believe that will help make very helpful tools for developers that target that 
OS (and don't have access to development devices, that is, basically everyone 
not working at Apple).

> If you really need to build code with the Darwin ABI that runs on Linux, can 
> you not achieve it by building with `-S` and then hacking up the resulting 
> assembly files?

After having written this patch and looked at the differences in assembly, that 
seemed far from trivial to achieve. But I'd be happy to look at POC of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

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

[PATCH] D94060: [OpenMP][AMDGPU] Use AMDGPU_KERNEL calling convention for entry function

[Pushpinder Singh via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4909cb1a0fe9: [OpenMP][AMDGPU] Use AMDGPU_KERNEL calling 
convention for entry function (authored by pdhaliwal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94060

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/amdgcn_target_codegen.cpp


Index: clang/test/OpenMP/amdgcn_target_codegen.cpp
===
--- clang/test/OpenMP/amdgcn_target_codegen.cpp
+++ clang/test/OpenMP/amdgcn_target_codegen.cpp
@@ -9,7 +9,7 @@
 #define N 1000
 
 int test_amdgcn_target_tid_threads() {
-// CHECK-LABEL: define weak void @{{.*}}test_amdgcn_target_tid_threads
+// CHECK-LABEL: define weak amdgpu_kernel void 
@{{.*}}test_amdgcn_target_tid_threads
 
   int arr[N];
 
@@ -25,7 +25,7 @@
 }
 
 int test_amdgcn_target_tid_threads_simd() {
-// CHECK-LABEL: define weak void @{{.*}}test_amdgcn_target_tid_threads_simd
+// CHECK-LABEL: define weak amdgpu_kernel void 
@{{.*}}test_amdgcn_target_tid_threads_simd
 
   int arr[N];
 
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6471,6 +6471,8 @@
 OutlinedFnID = llvm::ConstantExpr::getBitCast(OutlinedFn, CGM.Int8PtrTy);
 OutlinedFn->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
 OutlinedFn->setDSOLocal(false);
+if (CGM.getTriple().isAMDGCN())
+  OutlinedFn->setCallingConv(llvm::CallingConv::AMDGPU_KERNEL);
   } else {
 std::string Name = getName({EntryFnName, "region_id"});
 OutlinedFnID = new llvm::GlobalVariable(


Index: clang/test/OpenMP/amdgcn_target_codegen.cpp
===
--- clang/test/OpenMP/amdgcn_target_codegen.cpp
+++ clang/test/OpenMP/amdgcn_target_codegen.cpp
@@ -9,7 +9,7 @@
 #define N 1000
 
 int test_amdgcn_target_tid_threads() {
-// CHECK-LABEL: define weak void @{{.*}}test_amdgcn_target_tid_threads
+// CHECK-LABEL: define weak amdgpu_kernel void @{{.*}}test_amdgcn_target_tid_threads
 
   int arr[N];
 
@@ -25,7 +25,7 @@
 }
 
 int test_amdgcn_target_tid_threads_simd() {
-// CHECK-LABEL: define weak void @{{.*}}test_amdgcn_target_tid_threads_simd
+// CHECK-LABEL: define weak amdgpu_kernel void @{{.*}}test_amdgcn_target_tid_threads_simd
 
   int arr[N];
 
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6471,6 +6471,8 @@
 OutlinedFnID = llvm::ConstantExpr::getBitCast(OutlinedFn, CGM.Int8PtrTy);
 OutlinedFn->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
 OutlinedFn->setDSOLocal(false);
+if (CGM.getTriple().isAMDGCN())
+  OutlinedFn->setCallingConv(llvm::CallingConv::AMDGPU_KERNEL);
   } else {
 std::string Name = getName({EntryFnName, "region_id"});
 OutlinedFnID = new llvm::GlobalVariable(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 4909cb1 - [OpenMP][AMDGPU] Use AMDGPU_KERNEL calling convention for entry function

[Pushpinder Singh via cfe-commits]

Author: Pushpinder Singh
Date: 2021-01-06T02:03:30-05:00
New Revision: 4909cb1a0fe9f2494ccbadc2856b6ddfc70051b5

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

LOG: [OpenMP][AMDGPU] Use AMDGPU_KERNEL calling convention for entry function

AMDGPU backend requires entry functions/kernels to have AMDGPU_KERNEL
calling convention for proper linking.

Reviewed By: JonChesterfield

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/amdgcn_target_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index c15f6350b95e..a3b24039365b 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6471,6 +6471,8 @@ void CGOpenMPRuntime::emitTargetOutlinedFunctionHelper(
 OutlinedFnID = llvm::ConstantExpr::getBitCast(OutlinedFn, CGM.Int8PtrTy);
 OutlinedFn->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
 OutlinedFn->setDSOLocal(false);
+if (CGM.getTriple().isAMDGCN())
+  OutlinedFn->setCallingConv(llvm::CallingConv::AMDGPU_KERNEL);
   } else {
 std::string Name = getName({EntryFnName, "region_id"});
 OutlinedFnID = new llvm::GlobalVariable(

diff  --git a/clang/test/OpenMP/amdgcn_target_codegen.cpp 
b/clang/test/OpenMP/amdgcn_target_codegen.cpp
index 416ed06083b0..701211d449ca 100644
--- a/clang/test/OpenMP/amdgcn_target_codegen.cpp
+++ b/clang/test/OpenMP/amdgcn_target_codegen.cpp
@@ -9,7 +9,7 @@
 #define N 1000
 
 int test_amdgcn_target_tid_threads() {
-// CHECK-LABEL: define weak void @{{.*}}test_amdgcn_target_tid_threads
+// CHECK-LABEL: define weak amdgpu_kernel void 
@{{.*}}test_amdgcn_target_tid_threads
 
   int arr[N];
 
@@ -25,7 +25,7 @@ int test_amdgcn_target_tid_threads() {
 }
 
 int test_amdgcn_target_tid_threads_simd() {
-// CHECK-LABEL: define weak void @{{.*}}test_amdgcn_target_tid_threads_simd
+// CHECK-LABEL: define weak amdgpu_kernel void 
@{{.*}}test_amdgcn_target_tid_threads_simd
 
   int arr[N];
 



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

[PATCH] D92812: [X86] AMD Znver3 (Family 19H) Enablement

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper 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/D92812/new/

https://reviews.llvm.org/D92812

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

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

[Hongtao Yu via Phabricator via cfe-commits]

hoy added a comment.

In D93747#2481304 , @tmsriram wrote:

> In D93747#2481223 , @tmsriram wrote:
>
>> In D93747#2481163 , @dblaikie wrote:
>>
>>> In D93747#2481095 , @hoy wrote:
>>>
 In D93747#2481073 , @dblaikie 
 wrote:

> In D93747#2481053 , @tmsriram 
> wrote:
>
>> In D93747#2480442 , @dblaikie 
>> wrote:
>>
>>> @tmsriram - any ideas what your case/example was like that might've 
>>> caused degraded debugging experience? Would be good to understand if 
>>> we're producing some bad DWARF with this patch/if there might be some 
>>> way to avoid it (as it seems like gdb can handle mangled 
>>> names/overloading in C in this example I've tried)
>>
>> I haven't seen anything that caused degraded debugging experience.  I am 
>> interested in this as we do look at this field for the purposes of 
>> profile attribtution for calls that are inlined.  My comment was that we 
>> don't need to create this if it didn't already exist.  I am not fully 
>> aware of what would happen if we did it all the time.
>
> Ah, sorry, I got confused as to who's comment I was reading. I see it was 
> @hoy who said:
>
>> If set, the gdb debugger will use that field to match the user input and 
>> set breakpoints. Therefore, giving DW_AT_linkage_name a uniquefied name 
>> prevents the debugger from setting a breakpoint based on source names 
>> unless the user specifies a decorated name.
>>
>> Hence, this approach sounds like a workaround for us when the profile 
>> quality matters more than debugging experience.
>
> So I'm still a bit confused. My test doesn't seem to demonstrate the 
> issue with setting a linkage name preventing the debugger from setting a 
> breakpoint based on the source name?
>
> Suggesting that gdb isn't using the DW_AT_name at all for "break 
> " but instead demangling and stripping off the extras from 
> the linkage name, and since it can't demangle this uniquified name 
> (unlike the mangled name used when using the overloadable attribute) that 
> degrades the debugger user experience? I'd have to test that in more 
> detail/with some hand-hacked DWARF.

 Yes, I think in your case the linage name can be demangled by the 
 debugger. In my previous experiment, the uniquefied names could not be 
 demangled therefore I was not able to breakpoint.
>>>
>>> Ah, did some more testing. Seems it's because it isn't a classically 
>>> mangled name.
>
> The simplest fix I can think of is to emit the base 10 number of the md5hash. 
>  That would preserve all the existing uniqueness and be demangler friendly.  
> @hoy @dblaikie  WDYT?

I think using the base 10 form of md5hash is a smart workaround. Thanks for the 
suggestion.

>> Yep, that's the issue
>>
>>   $ c++filt _Z3foov.__uniq
>>   foo() [clone .__uniq]
>>   
>>   $ c++filt _Z3foov.__uniq.123
>>   foo() [clone .__uniq.123]
>>   
>>   $ c++filt._Z3foov.__uniq.123abc
>>   _Z3foov.__uniq.123abc
>>
>> The demangler does not understand a mix of numeric and alpha-numeric. It can 
>> be either but not both and md5hashes contain both.  So we might have to 
>> process the hash string or do something different to keep it demangler 
>> friendly.
>>
>>> It might be possible for this uniquifying step to check if the name is 
>>> mangled (does it start with "_Z") and if it isn't mangled, it could mangle 
>>> it (check the length of the name, then "_Zv") and add the 
>>> unique suffix. That looks to me like it preserves the original debugging 
>>> behavior?
>>>
>>> (has the problem that we don't actually know the mangling scheme at this 
>>> point - we do know it up in clang (where it might be Itanium mangling or 
>>> Microsoft mangling), might point again to the possibility this feature is 
>>> more suitable to implement in the frontend rather than a middle end pass. 
>>> And also the 'v' in the mangling is 'void return type', which is a lie - 
>>> not sure if we could do something better there)

Doing name unification in the frontend sounds like the ultimate solution and 
since the frontend has all the knowledge about the name mangler. I think for 
now we can go with the solution @tmsriram suggested, i.e., using the base 10 
form of md5 hash.

>>> I think it's pretty important that we don't break tradeoff debuggability 
>>> for profile accuracy. It'll make for a difficult tradeoff for our/any users.
>>
>> Agreed, I didn't expect this.
>>
>>> (side note: using the original source file name seems problematic - I know 
>>> in google at least, the same source file is sometimes built into more than 
>>> one 

[PATCH] D93301: [flang][driver] Add support for `-c` and `-emit-obj`

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX accepted this revision.
SouraVX added a comment.

LGTM! - Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93301

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Are you committed to the name `__ibm128`?  I think a name that says something 
about floating point would be better than just a company name and a number.  
"double double" is the common name for this format pretty much everywhere, and 
while I certainly would *not* suggest actually spelling it `double double` 
(i.e. with repeated type specifiers0), you could certainly use something like 
`__doubledouble`.


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

https://reviews.llvm.org/D93377

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

[PATCH] D89490: Introduce __attribute__((darwin_abi))

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I do feel like this is a terrible idea, sorry.  It's a *very* niche use case to 
be dedicating a new compiler feature to, and it's a feature that demands a lot 
from the internal compiler architecture in ways that other features don't.

If you really need to build code with the Darwin ABI that runs on Linux, can 
you not achieve it by building with `-S` and then hacking up the resulting 
assembly files?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89490

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

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In D93747#2481223 , @tmsriram wrote:

> In D93747#2481163 , @dblaikie wrote:
>
>> In D93747#2481095 , @hoy wrote:
>>
>>> In D93747#2481073 , @dblaikie 
>>> wrote:
>>>
 In D93747#2481053 , @tmsriram 
 wrote:

> In D93747#2480442 , @dblaikie 
> wrote:
>
>> @tmsriram - any ideas what your case/example was like that might've 
>> caused degraded debugging experience? Would be good to understand if 
>> we're producing some bad DWARF with this patch/if there might be some 
>> way to avoid it (as it seems like gdb can handle mangled 
>> names/overloading in C in this example I've tried)
>
> I haven't seen anything that caused degraded debugging experience.  I am 
> interested in this as we do look at this field for the purposes of 
> profile attribtution for calls that are inlined.  My comment was that we 
> don't need to create this if it didn't already exist.  I am not fully 
> aware of what would happen if we did it all the time.

 Ah, sorry, I got confused as to who's comment I was reading. I see it was 
 @hoy who said:

> If set, the gdb debugger will use that field to match the user input and 
> set breakpoints. Therefore, giving DW_AT_linkage_name a uniquefied name 
> prevents the debugger from setting a breakpoint based on source names 
> unless the user specifies a decorated name.
>
> Hence, this approach sounds like a workaround for us when the profile 
> quality matters more than debugging experience.

 So I'm still a bit confused. My test doesn't seem to demonstrate the issue 
 with setting a linkage name preventing the debugger from setting a 
 breakpoint based on the source name?

 Suggesting that gdb isn't using the DW_AT_name at all for "break >>> name>" but instead demangling and stripping off the extras from the 
 linkage name, and since it can't demangle this uniquified name (unlike the 
 mangled name used when using the overloadable attribute) that degrades the 
 debugger user experience? I'd have to test that in more detail/with some 
 hand-hacked DWARF.
>>>
>>> Yes, I think in your case the linage name can be demangled by the debugger. 
>>> In my previous experiment, the uniquefied names could not be demangled 
>>> therefore I was not able to breakpoint.
>>
>> Ah, did some more testing. Seems it's because it isn't a classically mangled 
>> name.

The simplest fix I can think of is to emit the base 10 number of the md5hash.  
That would preserve all the existing uniqueness and be demangler friendly.  
@hoy @dblaikie  WDYT?

> Yep, that's the issue
>
>   $ c++filt _Z3foov.__uniq
>   foo() [clone .__uniq]
>   
>   $ c++filt _Z3foov.__uniq.123
>   foo() [clone .__uniq.123]
>   
>   $ c++filt._Z3foov.__uniq.123abc
>   _Z3foov.__uniq.123abc
>
> The demangler does not understand a mix of numeric and alpha-numeric. It can 
> be either but not both and md5hashes contain both.  So we might have to 
> process the hash string or do something different to keep it demangler 
> friendly.
>
>> It might be possible for this uniquifying step to check if the name is 
>> mangled (does it start with "_Z") and if it isn't mangled, it could mangle 
>> it (check the length of the name, then "_Zv") and add the 
>> unique suffix. That looks to me like it preserves the original debugging 
>> behavior?
>>
>> (has the problem that we don't actually know the mangling scheme at this 
>> point - we do know it up in clang (where it might be Itanium mangling or 
>> Microsoft mangling), might point again to the possibility this feature is 
>> more suitable to implement in the frontend rather than a middle end pass. 
>> And also the 'v' in the mangling is 'void return type', which is a lie - not 
>> sure if we could do something better there)
>>
>> I think it's pretty important that we don't break tradeoff debuggability for 
>> profile accuracy. It'll make for a difficult tradeoff for our/any users.
>
> Agreed, I didn't expect this.
>
>> (side note: using the original source file name seems problematic - I know 
>> in google at least, the same source file is sometimes built into more than 
>> one library/form with different preprocessor defines, so this may not 
>> produce as unique a name as you are expecting?)
>
> It was a best effort and I think the hashing can be improved by using more 
> signals other than just the module name if needed.  For hard data though, 
> this does significantly improve performance which clearly comes from better 
> profile attribution so it does something.




Repository:
  rG LLVM Github Monorepo

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

[PATCH] D88298: Fix MaterializeTemporaryExpr's type when its an incomplete array.

[John McCall via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D88298

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

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

People can't actually just copy/paste from the comment anyway because of the 
comment characters on the line, and I don't think anyone will misunderstand the 
example if the backslash is missing.  It's silly that GCC has a problem with 
this, but since it does, let's just work around the problem and drop the 
backslash rather than having one comment block in the header use a different 
style.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92409

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

[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Without bothering to look it up, I would guess that the attribute-parsing code 
used to generically handle the ambiguity between identifier expressions and 
identifier attribute arguments by just always parsing simple identifiers as 
identifier arguments, making it Sema's responsibility to turn that back into an 
expression.  At some point, the parser was made sensitive to the actual 
attribute being parsed, but we never bothered to simplify Sema.  At any rate, 
the parser does now know exactly which argument of which attribute it's 
parsing, so there's zero reason for it to force this complexity on Sema 
anymore; if we find a case that parses identifier arguments, we should fix it 
in the parser to parse an expression instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94092

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

[clang] a032a4e - [-Wcalled-once-parameter][NFC] Fix operator precedence warning

[Yang Fan via cfe-commits]

Author: Yang Fan
Date: 2021-01-06T12:16:30+08:00
New Revision: a032a4e7998c9adc7faea9e7b8e36a9552d3503b

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

LOG: [-Wcalled-once-parameter][NFC] Fix operator precedence warning

Added: 


Modified: 
clang/lib/Analysis/CalledOnceCheck.cpp

Removed: 




diff  --git a/clang/lib/Analysis/CalledOnceCheck.cpp 
b/clang/lib/Analysis/CalledOnceCheck.cpp
index 2eff97640dfa..6b7d3790e3e5 100644
--- a/clang/lib/Analysis/CalledOnceCheck.cpp
+++ b/clang/lib/Analysis/CalledOnceCheck.cpp
@@ -573,8 +573,8 @@ class CalledOnceChecker : public 
ConstStmtVisitor {
 CheckConventionalParameters(CheckConventionalParameters),
 CurrentState(0) {
 initDataStructures();
-assert(size() == 0 ||
-   !States.empty() && "Data structures are inconsistent");
+assert((size() == 0 || !States.empty()) &&
+   "Data structures are inconsistent");
   }
 
   
//===--===//



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

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In D93747#2481163 , @dblaikie wrote:

> In D93747#2481095 , @hoy wrote:
>
>> In D93747#2481073 , @dblaikie wrote:
>>
>>> In D93747#2481053 , @tmsriram 
>>> wrote:
>>>
 In D93747#2480442 , @dblaikie 
 wrote:

> @tmsriram - any ideas what your case/example was like that might've 
> caused degraded debugging experience? Would be good to understand if 
> we're producing some bad DWARF with this patch/if there might be some way 
> to avoid it (as it seems like gdb can handle mangled names/overloading in 
> C in this example I've tried)

 I haven't seen anything that caused degraded debugging experience.  I am 
 interested in this as we do look at this field for the purposes of profile 
 attribtution for calls that are inlined.  My comment was that we don't 
 need to create this if it didn't already exist.  I am not fully aware of 
 what would happen if we did it all the time.
>>>
>>> Ah, sorry, I got confused as to who's comment I was reading. I see it was 
>>> @hoy who said:
>>>
 If set, the gdb debugger will use that field to match the user input and 
 set breakpoints. Therefore, giving DW_AT_linkage_name a uniquefied name 
 prevents the debugger from setting a breakpoint based on source names 
 unless the user specifies a decorated name.

 Hence, this approach sounds like a workaround for us when the profile 
 quality matters more than debugging experience.
>>>
>>> So I'm still a bit confused. My test doesn't seem to demonstrate the issue 
>>> with setting a linkage name preventing the debugger from setting a 
>>> breakpoint based on the source name?
>>>
>>> Suggesting that gdb isn't using the DW_AT_name at all for "break >> name>" but instead demangling and stripping off the extras from the linkage 
>>> name, and since it can't demangle this uniquified name (unlike the mangled 
>>> name used when using the overloadable attribute) that degrades the debugger 
>>> user experience? I'd have to test that in more detail/with some hand-hacked 
>>> DWARF.
>>
>> Yes, I think in your case the linage name can be demangled by the debugger. 
>> In my previous experiment, the uniquefied names could not be demangled 
>> therefore I was not able to breakpoint.
>
> Ah, did some more testing. Seems it's because it isn't a classically mangled 
> name.

Yep, that's the issue

  $ c++filt _Z3foov.__uniq
  foo() [clone .__uniq]
  
  $ c++filt _Z3foov.__uniq.123
  foo() [clone .__uniq.123]
  
  $ c++filt._Z3foov.__uniq.123abc
  _Z3foov.__uniq.123abc

The demangler does not understand a mix of numeric and alpha-numeric. It can be 
either but not both and md5hashes contain both.  So we might have to process 
the hash string or do something different to keep it demangler friendly.

> It might be possible for this uniquifying step to check if the name is 
> mangled (does it start with "_Z") and if it isn't mangled, it could mangle it 
> (check the length of the name, then "_Zv") and add the unique 
> suffix. That looks to me like it preserves the original debugging behavior?
>
> (has the problem that we don't actually know the mangling scheme at this 
> point - we do know it up in clang (where it might be Itanium mangling or 
> Microsoft mangling), might point again to the possibility this feature is 
> more suitable to implement in the frontend rather than a middle end pass. And 
> also the 'v' in the mangling is 'void return type', which is a lie - not sure 
> if we could do something better there)
>
> I think it's pretty important that we don't break tradeoff debuggability for 
> profile accuracy. It'll make for a difficult tradeoff for our/any users.

Agreed, I didn't expect this.

> (side note: using the original source file name seems problematic - I know in 
> google at least, the same source file is sometimes built into more than one 
> library/form with different preprocessor defines, so this may not produce as 
> unique a name as you are expecting?)

It was a best effort and I think the hashing can be improved by using more 
signals other than just the module name if needed.  For hard data though, this 
does significantly improve performance which clearly comes from better profile 
attribution so it does something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93747

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

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D93747#2481095 , @hoy wrote:

> In D93747#2481073 , @dblaikie wrote:
>
>> In D93747#2481053 , @tmsriram wrote:
>>
>>> In D93747#2480442 , @dblaikie 
>>> wrote:
>>>
 @tmsriram - any ideas what your case/example was like that might've caused 
 degraded debugging experience? Would be good to understand if we're 
 producing some bad DWARF with this patch/if there might be some way to 
 avoid it (as it seems like gdb can handle mangled names/overloading in C 
 in this example I've tried)
>>>
>>> I haven't seen anything that caused degraded debugging experience.  I am 
>>> interested in this as we do look at this field for the purposes of profile 
>>> attribtution for calls that are inlined.  My comment was that we don't need 
>>> to create this if it didn't already exist.  I am not fully aware of what 
>>> would happen if we did it all the time.
>>
>> Ah, sorry, I got confused as to who's comment I was reading. I see it was 
>> @hoy who said:
>>
>>> If set, the gdb debugger will use that field to match the user input and 
>>> set breakpoints. Therefore, giving DW_AT_linkage_name a uniquefied name 
>>> prevents the debugger from setting a breakpoint based on source names 
>>> unless the user specifies a decorated name.
>>>
>>> Hence, this approach sounds like a workaround for us when the profile 
>>> quality matters more than debugging experience.
>>
>> So I'm still a bit confused. My test doesn't seem to demonstrate the issue 
>> with setting a linkage name preventing the debugger from setting a 
>> breakpoint based on the source name?
>>
>> Suggesting that gdb isn't using the DW_AT_name at all for "break > name>" but instead demangling and stripping off the extras from the linkage 
>> name, and since it can't demangle this uniquified name (unlike the mangled 
>> name used when using the overloadable attribute) that degrades the debugger 
>> user experience? I'd have to test that in more detail/with some hand-hacked 
>> DWARF.
>
> Yes, I think in your case the linage name can be demangled by the debugger. 
> In my previous experiment, the uniquefied names could not be demangled 
> therefore I was not able to breakpoint.

Ah, did some more testing. Seems it's because it isn't a classically mangled 
name.

It might be possible for this uniquifying step to check if the name is mangled 
(does it start with "_Z") and if it isn't mangled, it could mangle it (check 
the length of the name, then "_Zv") and add the unique suffix. 
That looks to me like it preserves the original debugging behavior?

(has the problem that we don't actually know the mangling scheme at this point 
- we do know it up in clang (where it might be Itanium mangling or Microsoft 
mangling), might point again to the possibility this feature is more suitable 
to implement in the frontend rather than a middle end pass. And also the 'v' in 
the mangling is 'void return type', which is a lie - not sure if we could do 
something better there)

I think it's pretty important that we don't break tradeoff debuggability for 
profile accuracy. It'll make for a difficult tradeoff for our/any users.

(side note: using the original source file name seems problematic - I know in 
google at least, the same source file is sometimes built into more than one 
library/form with different preprocessor defines, so this may not produce as 
unique a name as you are expecting?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93747

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

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

[Hongtao Yu via Phabricator via cfe-commits]

hoy added a comment.

In D93747#2481073 , @dblaikie wrote:

> In D93747#2481053 , @tmsriram wrote:
>
>> In D93747#2480442 , @dblaikie wrote:
>>
>>> @tmsriram - any ideas what your case/example was like that might've caused 
>>> degraded debugging experience? Would be good to understand if we're 
>>> producing some bad DWARF with this patch/if there might be some way to 
>>> avoid it (as it seems like gdb can handle mangled names/overloading in C in 
>>> this example I've tried)
>>
>> I haven't seen anything that caused degraded debugging experience.  I am 
>> interested in this as we do look at this field for the purposes of profile 
>> attribtution for calls that are inlined.  My comment was that we don't need 
>> to create this if it didn't already exist.  I am not fully aware of what 
>> would happen if we did it all the time.
>
> Ah, sorry, I got confused as to who's comment I was reading. I see it was 
> @hoy who said:
>
>> If set, the gdb debugger will use that field to match the user input and set 
>> breakpoints. Therefore, giving DW_AT_linkage_name a uniquefied name prevents 
>> the debugger from setting a breakpoint based on source names unless the user 
>> specifies a decorated name.
>>
>> Hence, this approach sounds like a workaround for us when the profile 
>> quality matters more than debugging experience.
>
> So I'm still a bit confused. My test doesn't seem to demonstrate the issue 
> with setting a linkage name preventing the debugger from setting a breakpoint 
> based on the source name?
>
> Suggesting that gdb isn't using the DW_AT_name at all for "break  name>" but instead demangling and stripping off the extras from the linkage 
> name, and since it can't demangle this uniquified name (unlike the mangled 
> name used when using the overloadable attribute) that degrades the debugger 
> user experience? I'd have to test that in more detail/with some hand-hacked 
> DWARF.

Yes, I think in your case the linage name can be demangled by the debugger. In 
my previous experiment, the uniquefied names could not be demangled therefore I 
was not able to breakpoint.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93747

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

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D93747#2481053 , @tmsriram wrote:

> In D93747#2480442 , @dblaikie wrote:
>
>> @tmsriram - any ideas what your case/example was like that might've caused 
>> degraded debugging experience? Would be good to understand if we're 
>> producing some bad DWARF with this patch/if there might be some way to avoid 
>> it (as it seems like gdb can handle mangled names/overloading in C in this 
>> example I've tried)
>
> I haven't seen anything that caused degraded debugging experience.  I am 
> interested in this as we do look at this field for the purposes of profile 
> attribtution for calls that are inlined.  My comment was that we don't need 
> to create this if it didn't already exist.  I am not fully aware of what 
> would happen if we did it all the time.

Ah, sorry, I got confused as to who's comment I was reading. I see it was @hoy 
who said:

> If set, the gdb debugger will use that field to match the user input and set 
> breakpoints. Therefore, giving DW_AT_linkage_name a uniquefied name prevents 
> the debugger from setting a breakpoint based on source names unless the user 
> specifies a decorated name.
>
> Hence, this approach sounds like a workaround for us when the profile quality 
> matters more than debugging experience.

So I'm still a bit confused. My test doesn't seem to demonstrate the issue with 
setting a linkage name preventing the debugger from setting a breakpoint based 
on the source name?

Suggesting that gdb isn't using the DW_AT_name at all for "break " but instead demangling and stripping off the extras from the linkage 
name, and since it can't demangle this uniquified name (unlike the mangled name 
used when using the overloadable attribute) that degrades the debugger user 
experience? I'd have to test that in more detail/with some hand-hacked DWARF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93747

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

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

[Hongtao Yu via Phabricator via cfe-commits]

hoy added a comment.

In D93747#2481053 , @tmsriram wrote:

> In D93747#2480442 , @dblaikie wrote:
>
 In D93747#2470178 , @tmsriram 
 wrote:

> In D93747#2469556 , @hoy wrote:
>
>>> In D93656 , @dblaikie wrote:
>>> Though the C case is interesting - it means you'll end up with C 
>>> functions with the same DWARF 'name' but different linkage name. I 
>>> don't know what that'll do to DWARF consumers - I guess they'll 
>>> probably be OK-ish with it, as much as they are with C++ overloading. I 
>>> think there are some cases of C name mangling so it's probably 
>>> supported/OK-ish with DWARF Consumers. Wouldn't hurt for you to take a 
>>> look/see what happens in that case with a debugger like gdb/check other 
>>> cases of C name mangling to see what DWARF they end up creating (with 
>>> both GCC and Clang).
>>
>> I did a quick experiment with C name managing with GCC and -flto. It 
>> turns out the `DW_AT_linkage_name` field of `DW_TAG_subprogram` is never 
>> set for C programs. If set, the gdb debugger will use that field to 
>> match the user input and set breakpoints. Therefore, giving 
>> `DW_AT_linkage_name` a uniquefied name prevents the debugger from 
>> setting a breakpoint based on source names unless the user specifies a 
>> decorated name.
>>
>> Hence, this approach sounds like a workaround for us when the profile 
>> quality matters more than debugging experience. I'm inclined to have it 
>> under a switch. What do you think?
>
> Just a thought, we could always check if rawLinkageName is set and only 
> set it when it is not null.  That seems safe without needing the option. 
> Not a strong opinion.

 Was this thread concluded? Doesn't look like any check was added?
>>>
>>> Thanks for the detailed analysis! Moving forward I would like to see a more 
>>> clear contract about debug linkage names between the compiler and debugger 
>>> as a guideline to code cloning work. For this patch, I'm adding a switch 
>>> for it with a default value "on" since AutoFDO and propeller are the 
>>> primary consumers of the work here and they mostly care about profile 
>>> quality more than debugging experience. But correct me if I'm wrong and 
>>> I'll flip the default value.
>>
>> Presumably people are going to want to debug these binaries - I'm not sure 
>> it's a "one or the other" situation as it sounds like @tmsriram was 
>> suggesting by only modifying the linkage name when it's already set this 
>> might produce a better debugging experience, if I'm following the discussion 
>> correctly?
>>
>> But I'm pretty sure there are places where C supports name mangling, so I 
>> wouldn't mind understanding the gdb behavior in more detail - perhaps 
>> there's a way to make it work better.
>>
>> Using Clang's __attribute__((overloadable)) support, I was able to produce a 
>> C program with mangled names, with DW_AT_linkage_names on those functions, 
>> like this:
>>
>>   $ cat test.c
>>   void __attribute__((overloadable)) f(int i) {
>>   }
>>   void __attribute__((overloadable)) f(long l) {
>>   }
>>   int main() {
>> f(3);
>> f(5l);
>>   }
>>   blaikie@blaikie-linux2:~/dev/scratch$ clang-tot test.c -g
>>   blaikie@blaikie-linux2:~/dev/scratch$ llvm-dwarfdump-tot a.out
>>   a.out:  file format elf64-x86-64
>>   
>>   .debug_info contents:
>>   0x: Compile Unit: length = 0x009e, format = DWARF32, version = 
>> 0x0004, abbr_offset = 0x, addr_size = 0x08 (next unit at 0x00a2)
>>   
>>   0x000b: DW_TAG_compile_unit
>> DW_AT_producer("clang version 12.0.0 
>> (g...@github.com:llvm/llvm-project.git 
>> 20013e0940dea465a173c3c7ce60f0e419242ef8)")
>> DW_AT_language(DW_LANG_C99)
>> DW_AT_name("test.c")
>> DW_AT_stmt_list   (0x)
>> DW_AT_comp_dir
>> ("/usr/local/google/home/blaikie/dev/scratch")
>> DW_AT_low_pc  (0x00401110)
>> DW_AT_high_pc (0x0040114c)
>>   
>>   0x002a:   DW_TAG_subprogram
>>   DW_AT_low_pc(0x00401110)
>>   DW_AT_high_pc   (0x00401119)
>>   DW_AT_frame_base(DW_OP_reg6 RBP)
>>   DW_AT_linkage_name  ("_Z1fi")
>>   DW_AT_name  ("f")
>>   DW_AT_decl_file 
>> ("/usr/local/google/home/blaikie/dev/scratch/test.c")
>>   DW_AT_decl_line (1)
>>   DW_AT_prototyped(true)
>>   DW_AT_external  (true)
>>   
>>   0x0043: DW_TAG_formal_parameter
>> DW_AT_location(DW_OP_fbreg -4)

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

[Yang Fan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG74f93bc373d0: [Sema] Fix deleted function problem in 
implicitly movable test (authored by nullptr.cpp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92936

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/SemaCXX/warn-return-std-move.cpp

Index: clang/test/SemaCXX/warn-return-std-move.cpp
===
--- clang/test/SemaCXX/warn-return-std-move.cpp
+++ clang/test/SemaCXX/warn-return-std-move.cpp
@@ -324,11 +324,29 @@
 // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:12}:"std::move(d)"
 }
 
-void ok_throw1() { Derived d; throw d; }
+void ok_throw1() {
+  Derived d;
+  throw d;
+}
 void ok_throw2(Derived d) { throw d; }
-void ok_throw3(Derived& d) { throw d; }
+void ok_throw3(Derived ) { throw d; }
 void ok_throw4(Derived d) { throw std::move(d); }
-void ok_throw5(Derived& d) { throw std::move(d); }
-void ok_throw6(Derived& d) { throw static_cast(d); }
+void ok_throw5(Derived ) { throw std::move(d); }
+void ok_throw6(Derived ) { throw static_cast(d); }
 void ok_throw7(TriviallyCopyable d) { throw d; }
 void ok_throw8(OnlyCopyable d) { throw d; }
+
+namespace test_delete {
+struct Base {
+  Base();
+  Base(Base &&) = delete;
+  Base(Base const &);
+};
+
+struct Derived : public Base {};
+
+Base test_ok() {
+  Derived d;
+  return d;
+}
+} // namespace test_delete
Index: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
===
--- /dev/null
+++ clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -verify=expected %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -verify=expected %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -verify=expected %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fcxx-exceptions -verify=expected %s
+
+namespace test_delete_function {
+struct A1 {
+  A1();
+  A1(const A1 &);
+  A1(A1 &&) = delete; // expected-note {{'A1' has been explicitly marked deleted here}}
+};
+A1 test1() {
+  A1 a;
+  return a; // expected-error {{call to deleted constructor of 'test_delete_function::A1'}}
+}
+
+struct A2 {
+  A2();
+  A2(const A2 &);
+
+private:
+  A2(A2 &&); // expected-note {{declared private here}}
+};
+A2 test2() {
+  A2 a;
+  return a; // expected-error {{calling a private constructor of class 'test_delete_function::A2'}}
+}
+
+struct C {};
+
+struct B1 {
+  B1(C &);
+  B1(C &&) = delete; // expected-note {{'B1' has been explicitly marked deleted here}}
+};
+B1 test3() {
+  C c;
+  return c; // expected-error {{conversion function from 'test_delete_function::C' to 'test_delete_function::B1' invokes a deleted function}}
+}
+
+struct B2 {
+  B2(C &);
+
+private:
+  B2(C &&); // expected-note {{declared private here}}
+};
+B2 test4() {
+  C c;
+  return c; // expected-error {{calling a private constructor of class 'test_delete_function::B2'}}
+}
+} // namespace test_delete_function
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3118,11 +3118,16 @@
 /// If move-initialization is not possible, such that we must fall back to
 /// treating the operand as an lvalue, we will leave Res in its original
 /// invalid state.
-static void TryMoveInitialization(Sema , const InitializedEntity ,
+///
+/// \returns Whether we need to do the second overload resolution. If the first
+/// overload resolution fails, or if the first overload resolution succeeds but
+/// the selected constructor/operator doesn't match the additional criteria, we
+/// need to do the second overload resolution.
+static bool TryMoveInitialization(Sema , const InitializedEntity ,
   const VarDecl *NRVOCandidate,
   QualType ResultType, Expr *,
   bool ConvertingConstructorsOnly,
-  ExprResult ) {
+  bool IsDiagnosticsCheck, ExprResult ) {
   ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
 CK_NoOp, Value, VK_XValue, FPOptionsOverride());
 
@@ -3133,8 +3138,11 @@
 
   InitializationSequence Seq(S, Entity, Kind, InitExpr);
 
-  if (!Seq)
-return;
+  bool NeedSecondOverloadResolution = true;
+  if (!Seq &&
+  (IsDiagnosticsCheck || Seq.getFailedOverloadResult() != OR_Deleted)) {
+return NeedSecondOverloadResolution;
+  }
 
   for (const InitializationSequence::Step  : Seq.steps()) {
 if (Step.Kind != InitializationSequence::SK_ConstructorInitialization &&
@@ -3177,6 +3185,7 @@
   

[clang] 74f93bc - [Sema] Fix deleted function problem in implicitly movable test

[Yang Fan via cfe-commits]

Author: Yang Fan
Date: 2021-01-06T10:05:40+08:00
New Revision: 74f93bc373d089e757bb65cf8b30b63a4eae8b69

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

LOG: [Sema] Fix deleted function problem in implicitly movable test

In implicitly movable test, a two-stage overload resolution is performed.
If the first overload resolution selects a deleted function, Clang directly
performs the second overload resolution, without checking whether the
deleted function matches the additional criteria.

This patch fixes the above problem.

Reviewed By: Quuxplusone

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

Added: 
clang/test/CXX/class/class.init/class.copy.elision/p3.cpp

Modified: 
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaCXX/warn-return-std-move.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 6d2e6094e79c..b5f31bf403d4 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -4115,11 +4115,13 @@ static void TryConstructorInitialization(Sema ,
 IsListInit);
   }
   if (Result) {
-Sequence.SetOverloadFailure(IsListInit ?
-  InitializationSequence::FK_ListConstructorOverloadFailed 
:
-  InitializationSequence::FK_ConstructorOverloadFailed,
-Result);
-return;
+Sequence.SetOverloadFailure(
+IsListInit ? InitializationSequence::FK_ListConstructorOverloadFailed
+   : InitializationSequence::FK_ConstructorOverloadFailed,
+Result);
+
+if (Result != OR_Deleted)
+  return;
   }
 
   bool HadMultipleCandidates = (CandidateSet.size() > 1);
@@ -4140,31 +4142,45 @@ static void TryConstructorInitialization(Sema ,
 return;
   }
 
-  // C++11 [dcl.init]p6:
-  //   If a program calls for the default initialization of an object
-  //   of a const-qualified type T, T shall be a class type with a
-  //   user-provided default constructor.
-  // C++ core issue 253 proposal:
-  //   If the implicit default constructor initializes all subobjects, no
-  //   initializer should be required.
-  // The 253 proposal is for example needed to process libstdc++ headers in 
5.x.
   CXXConstructorDecl *CtorDecl = cast(Best->Function);
-  if (Kind.getKind() == InitializationKind::IK_Default &&
-  Entity.getType().isConstQualified()) {
-if (!CtorDecl->getParent()->allowConstDefaultInit()) {
-  if (!maybeRecoverWithZeroInitialization(S, Sequence, Entity))
-Sequence.SetFailed(InitializationSequence::FK_DefaultInitOfConst);
+  if (Result != OR_Deleted) {
+// C++11 [dcl.init]p6:
+//   If a program calls for the default initialization of an object
+//   of a const-qualified type T, T shall be a class type with a
+//   user-provided default constructor.
+// C++ core issue 253 proposal:
+//   If the implicit default constructor initializes all subobjects, no
+//   initializer should be required.
+// The 253 proposal is for example needed to process libstdc++ headers
+// in 5.x.
+if (Kind.getKind() == InitializationKind::IK_Default &&
+Entity.getType().isConstQualified()) {
+  if (!CtorDecl->getParent()->allowConstDefaultInit()) {
+if (!maybeRecoverWithZeroInitialization(S, Sequence, Entity))
+  Sequence.SetFailed(InitializationSequence::FK_DefaultInitOfConst);
+return;
+  }
+}
+
+// C++11 [over.match.list]p1:
+//   In copy-list-initialization, if an explicit constructor is chosen, the
+//   initializer is ill-formed.
+if (IsListInit && !Kind.AllowExplicit() && CtorDecl->isExplicit()) {
+  Sequence.SetFailed(InitializationSequence::FK_ExplicitConstructor);
   return;
 }
   }
 
-  // C++11 [over.match.list]p1:
-  //   In copy-list-initialization, if an explicit constructor is chosen, the
-  //   initializer is ill-formed.
-  if (IsListInit && !Kind.AllowExplicit() && CtorDecl->isExplicit()) {
-Sequence.SetFailed(InitializationSequence::FK_ExplicitConstructor);
+  // [class.copy.elision]p3:
+  // In some copy-initialization contexts, a two-stage overload resolution
+  // is performed.
+  // If the first overload resolution selects a deleted function, we also
+  // need the initialization sequence to decide whether to perform the second
+  // overload resolution.
+  // For deleted functions in other contexts, there is no need to get the
+  // initialization sequence.
+  if (Result == OR_Deleted && Kind.getKind() != InitializationKind::IK_Copy)
 return;
-  }
 
   // Add the constructor initialization step. Any cv-qualification conversion 
is
   // subsumed by the initialization.
@@ -5258,9 +5274,17 @@ static void 

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

[Sriraman Tallam via Phabricator via cfe-commits]

tmsriram added a comment.

In D93747#2480442 , @dblaikie wrote:

>>> In D93747#2470178 , @tmsriram 
>>> wrote:
>>>
 In D93747#2469556 , @hoy wrote:

>> In D93656 , @dblaikie wrote:
>> Though the C case is interesting - it means you'll end up with C 
>> functions with the same DWARF 'name' but different linkage name. I don't 
>> know what that'll do to DWARF consumers - I guess they'll probably be 
>> OK-ish with it, as much as they are with C++ overloading. I think there 
>> are some cases of C name mangling so it's probably supported/OK-ish with 
>> DWARF Consumers. Wouldn't hurt for you to take a look/see what happens 
>> in that case with a debugger like gdb/check other cases of C name 
>> mangling to see what DWARF they end up creating (with both GCC and 
>> Clang).
>
> I did a quick experiment with C name managing with GCC and -flto. It 
> turns out the `DW_AT_linkage_name` field of `DW_TAG_subprogram` is never 
> set for C programs. If set, the gdb debugger will use that field to match 
> the user input and set breakpoints. Therefore, giving 
> `DW_AT_linkage_name` a uniquefied name prevents the debugger from setting 
> a breakpoint based on source names unless the user specifies a decorated 
> name.
>
> Hence, this approach sounds like a workaround for us when the profile 
> quality matters more than debugging experience. I'm inclined to have it 
> under a switch. What do you think?

 Just a thought, we could always check if rawLinkageName is set and only 
 set it when it is not null.  That seems safe without needing the option. 
 Not a strong opinion.
>>>
>>> Was this thread concluded? Doesn't look like any check was added?
>>
>> Thanks for the detailed analysis! Moving forward I would like to see a more 
>> clear contract about debug linkage names between the compiler and debugger 
>> as a guideline to code cloning work. For this patch, I'm adding a switch for 
>> it with a default value "on" since AutoFDO and propeller are the primary 
>> consumers of the work here and they mostly care about profile quality more 
>> than debugging experience. But correct me if I'm wrong and I'll flip the 
>> default value.
>
> Presumably people are going to want to debug these binaries - I'm not sure 
> it's a "one or the other" situation as it sounds like @tmsriram was 
> suggesting by only modifying the linkage name when it's already set this 
> might produce a better debugging experience, if I'm following the discussion 
> correctly?
>
> But I'm pretty sure there are places where C supports name mangling, so I 
> wouldn't mind understanding the gdb behavior in more detail - perhaps there's 
> a way to make it work better.
>
> Using Clang's __attribute__((overloadable)) support, I was able to produce a 
> C program with mangled names, with DW_AT_linkage_names on those functions, 
> like this:
>
>   $ cat test.c
>   void __attribute__((overloadable)) f(int i) {
>   }
>   void __attribute__((overloadable)) f(long l) {
>   }
>   int main() {
> f(3);
> f(5l);
>   }
>   blaikie@blaikie-linux2:~/dev/scratch$ clang-tot test.c -g
>   blaikie@blaikie-linux2:~/dev/scratch$ llvm-dwarfdump-tot a.out
>   a.out:  file format elf64-x86-64
>   
>   .debug_info contents:
>   0x: Compile Unit: length = 0x009e, format = DWARF32, version = 
> 0x0004, abbr_offset = 0x, addr_size = 0x08 (next unit at 0x00a2)
>   
>   0x000b: DW_TAG_compile_unit
> DW_AT_producer("clang version 12.0.0 
> (g...@github.com:llvm/llvm-project.git 
> 20013e0940dea465a173c3c7ce60f0e419242ef8)")
> DW_AT_language(DW_LANG_C99)
> DW_AT_name("test.c")
> DW_AT_stmt_list   (0x)
> DW_AT_comp_dir
> ("/usr/local/google/home/blaikie/dev/scratch")
> DW_AT_low_pc  (0x00401110)
> DW_AT_high_pc (0x0040114c)
>   
>   0x002a:   DW_TAG_subprogram
>   DW_AT_low_pc(0x00401110)
>   DW_AT_high_pc   (0x00401119)
>   DW_AT_frame_base(DW_OP_reg6 RBP)
>   DW_AT_linkage_name  ("_Z1fi")
>   DW_AT_name  ("f")
>   DW_AT_decl_file 
> ("/usr/local/google/home/blaikie/dev/scratch/test.c")
>   DW_AT_decl_line (1)
>   DW_AT_prototyped(true)
>   DW_AT_external  (true)
>   
>   0x0043: DW_TAG_formal_parameter
> DW_AT_location(DW_OP_fbreg -4)
> DW_AT_name("i")
> DW_AT_decl_file   
> ("/usr/local/google/home/blaikie/dev/scratch/test.c")
> 

[PATCH] D78058: option to write files to memory instead of disk

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

dexonsmith added a comment.

In D78058#2480411 , @dexonsmith wrote:

> In D78058#2471735 , @sammccall wrote:
>
>> @dexonsmith thanks for sharing!
>> Some initial thoughts since abstracting outputs is something we're starting 
>> to care about too...
>
> Thanks for looking.

I think I've found a fairly clean way to get the write-through memory buffer 
optimization with pluggable backends; still working through the other feedback, 
but hoping to have the patch / RFC out soonish.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78058

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

[PATCH] D83892: [clang][cli] Port CodeGen option flags to new option parsing system

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Reposting my comment here, since this is where the discussion was:

I think this change broke -gline-tables-only functionality. I compared object 
files before and after this change. The new object file had a large .debug_loc 
section, which is only present when full debug info is enabled, and is not 
desired in this case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83892

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

[PATCH] D91806: [InstCombine] Update valueCoversEntireFragment to use TypeSize

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

this lgtm with all other reviewer's concerns addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91806

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

[PATCH] D89844: [Clang][OpenMP] Fixed an issue of segment fault when using target nowait

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9670
  Info.MapTypesArray, Info.MappersArray, Info,
- {/*ForEndTask=*/false, HasDependClauses});
+ {/*ForEndTask=*/false, RequiresOuterTask});
 InputInfo.NumberOfTargetItems = Info.NumberOfPtrs;

mikerice wrote:
> It seems this change is no longer in the compiler, as it was reverted by the 
> next commit to this file.  Was that intentional?  See: 
> https://reviews.llvm.org/D90101.
Yeah, it is not needed anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89844

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

[PATCH] D89844: [Clang][OpenMP] Fixed an issue of segment fault when using target nowait

[Mike Rice via Phabricator via cfe-commits]

mikerice added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9670
  Info.MapTypesArray, Info.MappersArray, Info,
- {/*ForEndTask=*/false, HasDependClauses});
+ {/*ForEndTask=*/false, RequiresOuterTask});
 InputInfo.NumberOfTargetItems = Info.NumberOfPtrs;

It seems this change is no longer in the compiler, as it was reverted by the 
next commit to this file.  Was that intentional?  See: 
https://reviews.llvm.org/D90101.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89844

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

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/test/tools/llvm-cov/branch-noShowBranch.test:3
+// RUN: llvm-profdata merge %S/Inputs/branch-c-general.proftext -o %t.profdata
+// RUN: llvm-cov show %S/Inputs/branch-c-general.o32l -instr-profile 
%t.profdata -path-equivalence=/tmp,%S %S/branch-c-general.c | FileCheck %s
+// RUN: llvm-cov report %S/Inputs/branch-c-general.o32l 
--show-branch-summary=false -instr-profile %t.profdata -show-functions 
-path-equivalence=/tmp,%S %S/branch-c-general.c | FileCheck %s 
-check-prefix=REPORT

alanphipps wrote:
> dblaikie wrote:
> > This test depends on another test file as input which is generally not done 
> > in the LLVM test suite - inputs to tests are placed in the "Inputs" 
> > subdirectory, rather than in the test directory itself. I realize a few 
> > other tests already here (demangle, hideUnexpectedSubviews, style, and 
> > threads) already do this - but it'd be good to not add more cases (& fix 
> > those existing cases where possible)
> > 
> > Could you fix this so this test (& possibly others, if you have the 
> > opportunity/bandwidth) doesn't depend on other test files, but only on 
> > files in the Inputs subdirectory?
> Certainly I can do that -- I believe there are two other tests in which I did 
> this other than this test: branch-export-json.txt and 
> branch-export-lcov.test.  I will adjust those tests that I added within the 
> next days but I don't think I have the bandwidth to change other tests that 
> also do this.
> 
> 
Sure thing, thanks a bunch!

At least having fewer instances of the unfavorable idioms means they're less 
likely to be repeated in the future, even if there are some leftover 
instances/anachronisms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84467

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

[PATCH] D93772: [Clang][Driver] Fix read-after-free when using /clang:

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk 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/D93772/new/

https://reviews.llvm.org/D93772

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

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

[Alan Phipps via Phabricator via cfe-commits]

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



Comment at: llvm/test/tools/llvm-cov/branch-noShowBranch.test:3
+// RUN: llvm-profdata merge %S/Inputs/branch-c-general.proftext -o %t.profdata
+// RUN: llvm-cov show %S/Inputs/branch-c-general.o32l -instr-profile 
%t.profdata -path-equivalence=/tmp,%S %S/branch-c-general.c | FileCheck %s
+// RUN: llvm-cov report %S/Inputs/branch-c-general.o32l 
--show-branch-summary=false -instr-profile %t.profdata -show-functions 
-path-equivalence=/tmp,%S %S/branch-c-general.c | FileCheck %s 
-check-prefix=REPORT

dblaikie wrote:
> This test depends on another test file as input which is generally not done 
> in the LLVM test suite - inputs to tests are placed in the "Inputs" 
> subdirectory, rather than in the test directory itself. I realize a few other 
> tests already here (demangle, hideUnexpectedSubviews, style, and threads) 
> already do this - but it'd be good to not add more cases (& fix those 
> existing cases where possible)
> 
> Could you fix this so this test (& possibly others, if you have the 
> opportunity/bandwidth) doesn't depend on other test files, but only on files 
> in the Inputs subdirectory?
Certainly I can do that -- I believe there are two other tests in which I did 
this other than this test: branch-export-json.txt and branch-export-lcov.test.  
I will adjust those tests that I added within the next days but I don't think I 
have the bandwidth to change other tests that also do this.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84467

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

[PATCH] D84467: Add support for Branch Coverage in LLVM Source-Based Code Coverage

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/test/tools/llvm-cov/branch-noShowBranch.test:3
+// RUN: llvm-profdata merge %S/Inputs/branch-c-general.proftext -o %t.profdata
+// RUN: llvm-cov show %S/Inputs/branch-c-general.o32l -instr-profile 
%t.profdata -path-equivalence=/tmp,%S %S/branch-c-general.c | FileCheck %s
+// RUN: llvm-cov report %S/Inputs/branch-c-general.o32l 
--show-branch-summary=false -instr-profile %t.profdata -show-functions 
-path-equivalence=/tmp,%S %S/branch-c-general.c | FileCheck %s 
-check-prefix=REPORT

This test depends on another test file as input which is generally not done in 
the LLVM test suite - inputs to tests are placed in the "Inputs" subdirectory, 
rather than in the test directory itself. I realize a few other tests already 
here (demangle, hideUnexpectedSubviews, style, and threads) already do this - 
but it'd be good to not add more cases (& fix those existing cases where 
possible)

Could you fix this so this test (& possibly others, if you have the 
opportunity/bandwidth) doesn't depend on other test files, but only on files in 
the Inputs subdirectory?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84467

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

[PATCH] D36475: [analyzer] Add "create_sink" annotation support to MagentaHandleChecker

[Haowei Wu via Phabricator via cfe-commits]

haowei abandoned this revision.
haowei added a comment.
Herald added subscribers: steakhal, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.

An updated version was landed in f4c26d993bdc 
 . This 
diff is no longer needed.


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

https://reviews.llvm.org/D36475

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

[PATCH] D36251: [analyzer] Suppress warning when bug path contains noreturn function or return from main in MagentaHandleChecker

[Haowei Wu via Phabricator via cfe-commits]

haowei abandoned this revision.
haowei added a comment.
Herald added subscribers: steakhal, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.

An updated version was landed in f4c26d993bdc 
 . This 
diff is no longer needed.


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

https://reviews.llvm.org/D36251

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

[PATCH] D36024: [analyzer] Improved bug reporting in MagentaHandleChecker

[Haowei Wu via Phabricator via cfe-commits]

haowei abandoned this revision.
haowei added a comment.
Herald added subscribers: steakhal, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.

An updated version was landed in f4c26d993bdc 
 . This 
diff is no longer needed.


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

https://reviews.llvm.org/D36024

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

[PATCH] D36023: [analyzer] Add array support for MagentaHandleChecker

[Haowei Wu via Phabricator via cfe-commits]

haowei abandoned this revision.
haowei added a comment.
Herald added subscribers: steakhal, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.

An updated version was landed in f4c26d993bdc 
 . This 
diff is no longer needed.


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

https://reviews.llvm.org/D36023

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

[PATCH] D36022: [analyzer] Add handle misuse analysis to MagentaHandleChecker

[Haowei Wu via Phabricator via cfe-commits]

haowei abandoned this revision.
haowei added a comment.
Herald added subscribers: steakhal, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.

An updated version was landed in f4c26d993bdc 
 . This 
diff is no longer needed.


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

https://reviews.llvm.org/D36022

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

[PATCH] D35968: [analyzer] Add MagentaHandleChecker for the Magenta kernel

[Haowei Wu via Phabricator via cfe-commits]

haowei abandoned this revision.
haowei added a comment.
Herald added subscribers: steakhal, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.

An updated version was landed in f4c26d993bdc 
 . This 
diff is no longer needed.


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

https://reviews.llvm.org/D35968

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

[PATCH] D94067: [clang][ASTImporter] Fix a possible assertion failure `NeedsInjectedClassNameType(Decl)'.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:2901
+  // Skip the declaration if injected type is already set.
+  if (isa(RI->getTypeForDecl()))
+continue;

Is this to fix the bug or is this for efficiency sake?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94067

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

[PATCH] D94131: [clang-tidy] Use new mapAnyOf matcher

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 314739.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94131

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SpuriouslyWakeUpFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousStringCompareCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
  clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp

Index: clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -66,10 +66,10 @@
   substTemplateTypeParmType(hasReplacementType(ConstReferenceOrValue;
   auto UsedAsConstRefOrValueArg = forEachArgumentWithParam(
   DeclRefToVar, parmVarDecl(hasType(ConstReferenceOrValueOrReplaced)));
-  Matches = match(findAll(callExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
-  extractNodesByIdTo(Matches, "declRef", DeclRefs);
-  Matches =
-  match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
+  Matches = match(
+  findAll(
+  mapAnyOf(callExpr, cxxConstructExpr).with(UsedAsConstRefOrValueArg)),
+  Stmt, Context);
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
   // References and pointers to const assignments.
   Matches =
Index: clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
@@ -37,11 +37,10 @@
   has(compoundStmt(hasAnySubstatement(returnStmt(unless(has(expr())
   .bind("return"))),
   this);
-  auto CompoundContinue =
-  has(compoundStmt(hasAnySubstatement(continueStmt())).bind("continue"));
   Finder->addMatcher(
-  stmt(anyOf(forStmt(), cxxForRangeStmt(), whileStmt(), doStmt()),
-   CompoundContinue),
+  mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt)
+  .with(hasBody(compoundStmt(hasAnySubstatement(continueStmt()))
+.bind("continue"))),
   this);
 }
 
Index: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -56,28 +56,26 @@
   const char *ExprName = "__booleanContextExpr";
   auto Result =
   expr(expr().bind(ExprName),
-   anyOf(hasParent(varDecl(hasType(booleanType(,
+   anyOf(hasParent(
+ mapAnyOf(varDecl, fieldDecl).with(hasType(booleanType(,
  hasParent(cxxConstructorDecl(
  hasAnyConstructorInitializer(cxxCtorInitializer(
  withInitializer(expr(equalsBoundNode(ExprName))),
  forField(hasType(booleanType())),
- hasParent(fieldDecl(hasType(booleanType(,
  hasParent(stmt(anyOf(
  explicitCastExpr(hasDestinationType(booleanType())),
- ifStmt(hasCondition(expr(equalsBoundNode(ExprName,
- doStmt(hasCondition(expr(equalsBoundNode(ExprName,
- whileStmt(hasCondition(expr(equalsBoundNode(ExprName,
- forStmt(hasCondition(expr(equalsBoundNode(ExprName,
- conditionalOperator(
- hasCondition(expr(equalsBoundNode(ExprName,
+ mapAnyOf(ifStmt, doStmt, whileStmt, forStmt,
+  conditionalOperator)
+ .with(hasCondition(expr(equalsBoundNode(ExprName,
  parenListExpr(hasParent(varDecl(hasType(booleanType(),
  parenExpr(hasParent(
  explicitCastExpr(hasDestinationType(booleanType(),
  returnStmt(forFunction(returns(booleanType(,
  

[PATCH] D94127: [ASTMatchers] Add mapAnyOf matcher

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

This is part of a series of commits. See them in context here: 
https://github.com/steveire/llvm-project/commits/mapAnyOf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94127

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

[clang] b12e473 - Allow dependent alias template specializations in the preferred_name

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2021-01-05T15:33:51-08:00
New Revision: b12e4735317ec96e1b35deee68b90d62a23a9353

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

LOG: Allow dependent alias template specializations in the preferred_name
attribute.

This was intended to work, but didn't match the checks because these
types are modeled as TemplateSpecializationTypes not TypedefTypes.

Added: 


Modified: 
clang/include/clang/AST/Type.h
clang/lib/AST/TypePrinter.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaTemplate/attributes.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 684005c4876d..143a05cba6ad 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -2093,6 +2093,7 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
   bool isAtomicType() const;// C11 _Atomic()
   bool isUndeducedAutoType() const; // C++11 auto or
 // C++14 decltype(auto)
+  bool isTypedefNameType() const;   // typedef or alias template
 
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
   bool is##Id##Type() const;
@@ -7066,6 +7067,15 @@ inline bool Type::isOverloadableType() const {
   return isDependentType() || isRecordType() || isEnumeralType();
 }
 
+/// Determines whether this type is written as a typedef-name.
+inline bool Type::isTypedefNameType() const {
+  if (getAs())
+return true;
+  if (auto *TST = getAs())
+return TST->isTypeAlias();
+  return false;
+}
+
 /// Determines whether this type can decay to a pointer type.
 inline bool Type::canDecayToPointerType() const {
   return isFunctionType() || isArrayType();

diff  --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 54c451291a07..d1882ac1a3b3 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -116,6 +116,8 @@ namespace {
 static bool canPrefixQualifiers(const Type *T, bool 
);
 void spaceBeforePlaceHolder(raw_ostream );
 void printTypeSpec(NamedDecl *D, raw_ostream );
+void printTemplateId(const TemplateSpecializationType *T, raw_ostream ,
+ bool FullyQualify);
 
 void printBefore(QualType T, raw_ostream );
 void printAfter(QualType T, raw_ostream );
@@ -1352,9 +1354,17 @@ void TypePrinter::printRecordBefore(const RecordType *T, 
raw_ostream ) {
   // Print the preferred name if we have one for this type.
   for (const auto *PNA : T->getDecl()->specific_attrs()) {
 if (declaresSameEntity(PNA->getTypedefType()->getAsCXXRecordDecl(),
-   T->getDecl()))
-  return printTypeSpec(
-  PNA->getTypedefType()->castAs()->getDecl(), OS);
+   T->getDecl())) {
+  // Find the outermost typedef or alias template.
+  QualType T = PNA->getTypedefType();
+  while (true) {
+if (auto *TT = dyn_cast(T))
+  return printTypeSpec(TT->getDecl(), OS);
+if (auto *TST = dyn_cast(T))
+  return printTemplateId(TST, OS, /*FullyQualify=*/true);
+T = T->getLocallyUnqualifiedSingleStepDesugaredType();
+  }
+}
   }
 
   printTag(T->getDecl(), OS);
@@ -1416,20 +1426,32 @@ void TypePrinter::printSubstTemplateTypeParmPackAfter(
   printTemplateTypeParmAfter(T->getReplacedParameter(), OS);
 }
 
-void TypePrinter::printTemplateSpecializationBefore(
-const TemplateSpecializationType 
*T,
-raw_ostream ) {
+void TypePrinter::printTemplateId(const TemplateSpecializationType *T,
+  raw_ostream , bool FullyQualify) {
   IncludeStrongLifetimeRAII Strong(Policy);
-  T->getTemplateName().print(OS, Policy);
 
-  const TemplateParameterList *TPL = nullptr;
-  if (TemplateDecl *TD = T->getTemplateName().getAsTemplateDecl())
-TPL = TD->getTemplateParameters();
+  TemplateDecl *TD = T->getTemplateName().getAsTemplateDecl();
+  if (FullyQualify && TD) {
+if (!Policy.SuppressScope)
+  AppendScope(TD->getDeclContext(), OS, TD->getDeclName());
+
+IdentifierInfo *II = TD->getIdentifier();
+OS << II->getName();
+  } else {
+T->getTemplateName().print(OS, Policy);
+  }
 
+  const TemplateParameterList *TPL = TD ? TD->getTemplateParameters() : 
nullptr;
   printTemplateArgumentList(OS, T->template_arguments(), Policy, TPL);
   spaceBeforePlaceHolder(OS);
 }
 
+void TypePrinter::printTemplateSpecializationBefore(
+const TemplateSpecializationType 
*T,
+raw_ostream ) {
+  printTemplateId(T, OS, false);
+}
+
 void 

[PATCH] D94131: [clang-tidy] Use new mapAnyOf matcher

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added subscribers: kbarton, xazax.hun, nemanjai.
steveire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94131

Files:
  clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SpuriouslyWakeUpFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousStringCompareCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp
  clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp
  clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
  clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp

Index: clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -66,10 +66,10 @@
   substTemplateTypeParmType(hasReplacementType(ConstReferenceOrValue;
   auto UsedAsConstRefOrValueArg = forEachArgumentWithParam(
   DeclRefToVar, parmVarDecl(hasType(ConstReferenceOrValueOrReplaced)));
-  Matches = match(findAll(callExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
-  extractNodesByIdTo(Matches, "declRef", DeclRefs);
-  Matches =
-  match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
+  Matches = match(
+  findAll(
+  mapAnyOf(callExpr, cxxConstructExpr).with(UsedAsConstRefOrValueArg)),
+  Stmt, Context);
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
   // References and pointers to const assignments.
   Matches =
Index: clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
@@ -37,11 +37,10 @@
   has(compoundStmt(hasAnySubstatement(returnStmt(unless(has(expr())
   .bind("return"))),
   this);
-  auto CompoundContinue =
-  has(compoundStmt(hasAnySubstatement(continueStmt())).bind("continue"));
   Finder->addMatcher(
-  stmt(anyOf(forStmt(), cxxForRangeStmt(), whileStmt(), doStmt()),
-   CompoundContinue),
+  mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt)
+  .with(hasBody(compoundStmt(hasAnySubstatement(continueStmt()))
+.bind("continue"))),
   this);
 }
 
Index: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -56,28 +56,26 @@
   const char *ExprName = "__booleanContextExpr";
   auto Result =
   expr(expr().bind(ExprName),
-   anyOf(hasParent(varDecl(hasType(booleanType(,
+   anyOf(hasParent(
+ mapAnyOf(varDecl, fieldDecl).with(hasType(booleanType(,
  hasParent(cxxConstructorDecl(
  hasAnyConstructorInitializer(cxxCtorInitializer(
  withInitializer(expr(equalsBoundNode(ExprName))),
  forField(hasType(booleanType())),
- hasParent(fieldDecl(hasType(booleanType(,
  hasParent(stmt(anyOf(
  explicitCastExpr(hasDestinationType(booleanType())),
- ifStmt(hasCondition(expr(equalsBoundNode(ExprName,
- doStmt(hasCondition(expr(equalsBoundNode(ExprName,
- whileStmt(hasCondition(expr(equalsBoundNode(ExprName,
- forStmt(hasCondition(expr(equalsBoundNode(ExprName,
- conditionalOperator(
- hasCondition(expr(equalsBoundNode(ExprName,
+ mapAnyOf(ifStmt, doStmt, whileStmt, forStmt,
+  conditionalOperator)
+ .with(hasCondition(expr(equalsBoundNode(ExprName,
  parenListExpr(hasParent(varDecl(hasType(booleanType(),
  parenExpr(hasParent(
  

[PATCH] D94130: [ASTMatchers] Add support for CXXRewrittenBinaryOperator

[Stephen Kelly via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94130

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/unittests/AST/ASTTraverserTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -3259,6 +3259,527 @@
   EXPECT_TRUE(matches(Code, traverse(TK_AsIs, lambdaImplicitCapture)));
   EXPECT_FALSE(matches(
   Code, traverse(TK_IgnoreUnlessSpelledInSource, lambdaImplicitCapture)));
+
+  Code = R"cpp(
+struct S {};
+
+struct HasOpEq
+{
+bool operator==(const S& other)
+{
+return true;
+}
+};
+
+void binop()
+{
+HasOpEq s1;
+S s2;
+if (s1 != s2)
+return;
+}
+)cpp";
+  {
+auto M = unaryOperator(
+hasOperatorName("!"),
+has(cxxOperatorCallExpr(hasOverloadedOperatorName("==";
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
+EXPECT_FALSE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
+  {
+auto M = declRefExpr(to(varDecl(hasName("s1";
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
+  {
+auto M = cxxOperatorCallExpr(hasOverloadedOperatorName("=="));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
+EXPECT_FALSE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
+  {
+auto M = cxxOperatorCallExpr(hasOverloadedOperatorName("!="));
+EXPECT_FALSE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
+EXPECT_FALSE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
+  auto withDescendants = [](StringRef lName, StringRef rName) {
+return stmt(hasDescendant(declRefExpr(to(varDecl(hasName(lName),
+hasDescendant(declRefExpr(to(varDecl(hasName(rName));
+  };
+  {
+auto M = cxxRewrittenBinaryOperator(withDescendants("s1", "s2"));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
+  {
+auto M = cxxRewrittenBinaryOperator(
+has(declRefExpr(to(varDecl(hasName("s1"),
+has(declRefExpr(to(varDecl(hasName("s2"));
+EXPECT_FALSE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
+  {
+EXPECT_TRUE(matchesConditionally(
+Code,
+traverse(TK_AsIs,
+ cxxRewrittenBinaryOperator(
+ hasOperatorName("!="), hasAnyOperatorName("<", "!="),
+ isComparisonOperator(),
+ hasLHS(ignoringImplicit(
+ declRefExpr(to(varDecl(hasName("s1")),
+ hasRHS(ignoringImplicit(
+ declRefExpr(to(varDecl(hasName("s2")),
+ hasEitherOperand(ignoringImplicit(
+ declRefExpr(to(varDecl(hasName("s2")),
+ hasOperands(ignoringImplicit(
+ declRefExpr(to(varDecl(hasName("s1"),
+ ignoringImplicit(declRefExpr(
+ to(varDecl(hasName("s2",
+true, {"-std=c++20"}));
+EXPECT_TRUE(matchesConditionally(
+Code,
+traverse(TK_IgnoreUnlessSpelledInSource,
+ cxxRewrittenBinaryOperator(
+ hasOperatorName("!="), hasAnyOperatorName("<", "!="),
+ isComparisonOperator(),
+ 

[PATCH] D94129: [ASTMatchers] Add binaryOperation matcher

[Stephen Kelly via Phabricator via cfe-commits]

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

This is a simple utility which allows matching on binaryOperator and
cxxOperatorCallExpr. It can also be extended to support
cxxRewrittenBinaryOperator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94129

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/tools/dump_ast_matchers.py
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -478,6 +478,10 @@
 return;
   }
 
+  if (GetParam().hasDelayedTemplateParsing()) {
+return;
+  }
+
   StringRef Code = R"cpp(
 void F() {
   if (true) {}
@@ -549,6 +553,15 @@
 if (s1 != s2)
 return;
 }
+
+template
+void templ()
+{
+T s1;
+T s2;
+if (s1 != s2)
+return;
+}
 )cpp";
 
   EXPECT_TRUE(matches(
@@ -625,6 +638,30 @@
  .with(hasAnyOperatorName("==", "!="),
forFunction(functionDecl(hasName("opCall")));
 
+  EXPECT_TRUE(matches(
+  Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ binaryOperation(
+ hasOperatorName("!="),
+ forFunction(functionDecl(hasName("binop"))),
+ hasLHS(declRefExpr(to(varDecl(hasName("s1"),
+ hasRHS(declRefExpr(to(varDecl(hasName("s2");
+
+  EXPECT_TRUE(matches(
+  Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ binaryOperation(
+ hasOperatorName("!="),
+ forFunction(functionDecl(hasName("opCall"))),
+ hasLHS(declRefExpr(to(varDecl(hasName("s1"),
+ hasRHS(declRefExpr(to(varDecl(hasName("s2");
+
+  EXPECT_TRUE(matches(
+  Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ binaryOperation(
+ hasOperatorName("!="),
+ forFunction(functionDecl(hasName("templ"))),
+ hasLHS(declRefExpr(to(varDecl(hasName("s1"),
+ hasRHS(declRefExpr(to(varDecl(hasName("s2");
+
   Code = R"cpp(
 struct HasOpBang
 {
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -1973,6 +1973,8 @@
 ///   ostream  int b = 1, c = 1;
 ///   o << b << c;
 /// \endcode
+/// See also the binaryOperation() matcher for more-general matching of binary
+/// uses of this AST node.
 extern const internal::VariadicDynCastAllOfMatcher
 cxxOperatorCallExpr;
 
@@ -2395,6 +2397,7 @@
 /// \code
 ///   !(a || b)
 /// \endcode
+/// See also the binaryOperation() matcher for more-general matching.
 extern const internal::VariadicDynCastAllOfMatcher
 binaryOperator;
 
@@ -2733,6 +2736,56 @@
   NodeMatcher...);
 }
 
+/// Matches nodes which can be used with binary operators
+///
+/// The code
+/// \code
+///   var1 != var2;
+/// \endcode
+/// might be represented in the clang AST as a binaryOperator or a
+/// cxxOperatorCallExpr, depending on
+///
+/// * whether the types of var1 and var2 are fundamental (binaryOperator) or at
+///   least one is a class type (cxxOperatorCallExpr)
+/// * whether the code appears in a template declaration, if at least one of the
+///   vars is a dependent-type (binaryOperator)
+///
+/// This matcher elides details in places where the matchers for the nodes are
+/// compatible.
+///
+/// Given
+/// \code
+///   binaryOperation(
+/// hasOperatorName("!="),
+/// hasLHS(expr().bind("lhs")),
+/// hasRHS(expr().bind("rhs"))
+///   )
+/// \endcode
+/// matches each use of "!=" in:
+/// \code
+///   struct S{
+///   bool operator!=(const S&) const;
+///   };
+///
+///   void foo()
+///   {
+///  1 != 2;
+///  S() != S();
+///   }
+///
+///   template
+///   void templ()
+///   {
+///  1 != 2;
+///  T() != S();
+///   }
+/// \endcode
+template 
+internal::BindableMatcher
+binaryOperation(InnerMatcherType &&... Matchers) {
+  return mapAnyOf(binaryOperator, cxxOperatorCallExpr).with(Matchers...);
+}
+
 /// Matches unary expressions that have a specific type of argument.
 ///
 /// Given
Index: clang/docs/tools/dump_ast_matchers.py
===
--- clang/docs/tools/dump_ast_matchers.py
+++ clang/docs/tools/dump_ast_matchers.py
@@ -126,6 +126,9 @@
 args = "nodeMatcherFunction..."
 docs_result_type = 

[PATCH] D94128: [ASTMatchers] Make cxxOperatorCallExpr matchers API-compatible with n-ary operators

[Stephen Kelly via Phabricator via cfe-commits]

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

This makes them composable with mapAnyOf().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94128

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1313,6 +1313,42 @@
   traverse(TK_AsIs, hasRHS(hasType(pointsTo(qualType());
   EXPECT_TRUE(matches("void x() { 1[\"abc\"]; }", OperatorIntPointer));
   EXPECT_TRUE(notMatches("void x() { \"abc\"[1]; }", OperatorIntPointer));
+
+  StringRef Code = R"cpp(
+struct HasOpEq
+{
+bool operator==(const HasOpEq& other) const
+{
+return true;
+}
+};
+
+void binop()
+{
+HasOpEq s1;
+HasOpEq s2;
+if (s1 == s2)
+return;
+}
+)cpp";
+  auto s1Expr = declRefExpr(to(varDecl(hasName("s1";
+  auto s2Expr = declRefExpr(to(varDecl(hasName("s2";
+  EXPECT_TRUE(matches(
+  Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ cxxOperatorCallExpr(hasOperatorName("=="), hasLHS(s1Expr),
+ hasRHS(s2Expr);
+  EXPECT_TRUE(
+  matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ cxxOperatorCallExpr(hasAnyOperatorName("!=", "=="),
+ hasLHS(s1Expr);
+  EXPECT_TRUE(matches(
+  Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ cxxOperatorCallExpr(hasOperatorName("=="),
+ hasOperands(s1Expr, s2Expr);
+  EXPECT_TRUE(
+  matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ cxxOperatorCallExpr(hasOperatorName("=="),
+ hasEitherOperand(s2Expr);
 }
 
 TEST(MatchBinaryOperator, HasEitherOperand) {
@@ -1461,6 +1497,32 @@
 
   EXPECT_TRUE(matches("void x() { !false; }", OperatorOnFalse));
   EXPECT_TRUE(notMatches("void x() { !true; }", OperatorOnFalse));
+
+  StringRef Code = R"cpp(
+struct HasOpBang
+{
+bool operator!() const
+{
+return false;
+}
+};
+
+void binop()
+{
+HasOpBang s1;
+if (!s1)
+return;
+}
+)cpp";
+  auto s1Expr = declRefExpr(to(varDecl(hasName("s1";
+  EXPECT_TRUE(
+  matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ cxxOperatorCallExpr(hasOperatorName("!"),
+ hasUnaryOperand(s1Expr);
+  EXPECT_TRUE(
+  matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ cxxOperatorCallExpr(hasAnyOperatorName("+", "!"),
+ hasUnaryOperand(s1Expr);
 }
 
 TEST(Matcher, UnaryOperatorTypes) {
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -524,6 +524,158 @@
   EXPECT_TRUE(matches(
   Code, traverse(TK_IgnoreUnlessSpelledInSource,
  mapAnyOf(callExpr, cxxConstructExpr).bind("call";
+
+  Code = R"cpp(
+struct HasOpNeq
+{
+bool operator!=(const HasOpNeq& other) const
+{
+return true;
+}
+};
+
+void binop()
+{
+int s1;
+int s2;
+if (s1 != s2)
+return;
+}
+
+void opCall()
+{
+HasOpNeq s1;
+HasOpNeq s2;
+if (s1 != s2)
+return;
+}
+)cpp";
+
+  EXPECT_TRUE(matches(
+  Code,
+  traverse(TK_IgnoreUnlessSpelledInSource,
+   mapAnyOf(binaryOperator, cxxOperatorCallExpr)
+   .with(hasOperatorName("!="),
+ forFunction(functionDecl(hasName("binop"))),
+ hasLHS(declRefExpr(to(varDecl(hasName("s1"),
+ hasRHS(declRefExpr(to(varDecl(hasName("s2");
+
+  EXPECT_TRUE(matches(
+  Code,
+  traverse(TK_IgnoreUnlessSpelledInSource,
+   mapAnyOf(binaryOperator, cxxOperatorCallExpr)
+   .with(hasOperatorName("!="),
+ forFunction(functionDecl(hasName("opCall"))),
+ hasLHS(declRefExpr(to(varDecl(hasName("s1"),
+ hasRHS(declRefExpr(to(varDecl(hasName("s2");
+
+  EXPECT_TRUE(matches(
+  Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ 

[PATCH] D94127: [ASTMatchers] Add mapAnyOf matcher

[Stephen Kelly via Phabricator via cfe-commits]

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

Make it possible to compose a matcher for different base nodes.

This accepts one or more node matcher functors and zero or more
matchers, composing the latter into the former.

This allows composing of matchers where the same inner matcher name is
used for the same concept, but with a different node functor. Currently,
there is a limitation that the nodes must be in the same "clade", so
while

  mapAnyOf(ifStmt, forStmt).with(hasBody(stmt()))

can be used, functionDecl can not be added to the tuple.

It is possible to use this in clang-query, but it will require changes
to the QueryParser, so is deferred to a future review.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94127

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/tools/dump_ast_matchers.py
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -473,6 +473,59 @@
   EXPECT_TRUE(matches("int F() { return 1; }", integerLiteral(anyOf(;
 }
 
+TEST_P(ASTMatchersTest, MapAnyOf) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+
+  StringRef Code = R"cpp(
+void F() {
+  if (true) {}
+  for ( ; false; ) {}
+}
+)cpp";
+
+  auto trueExpr = cxxBoolLiteral(equals(true));
+  auto falseExpr = cxxBoolLiteral(equals(false));
+
+  EXPECT_TRUE(matches(
+  Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ mapAnyOf(ifStmt, forStmt).with(hasCondition(trueExpr);
+  EXPECT_TRUE(matches(
+  Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ mapAnyOf(ifStmt, forStmt).with(hasCondition(falseExpr);
+
+  Code = R"cpp(
+void func(bool b) {}
+struct S {
+  S(bool b) {}
+};
+void F() {
+  func(false);
+  S s(true);
+}
+)cpp";
+  EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ mapAnyOf(callExpr, cxxConstructExpr)
+ .with(hasArgument(0, trueExpr);
+  EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ mapAnyOf(callExpr, cxxConstructExpr)
+ .with(hasArgument(0, falseExpr);
+
+  EXPECT_TRUE(
+  matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ mapAnyOf(callExpr, cxxConstructExpr)
+ .with(hasArgument(0, expr()),
+   hasDeclaration(functionDecl());
+
+  EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ mapAnyOf(callExpr, cxxConstructExpr;
+
+  EXPECT_TRUE(matches(
+  Code, traverse(TK_IgnoreUnlessSpelledInSource,
+ mapAnyOf(callExpr, cxxConstructExpr).bind("call";
+}
+
 TEST_P(ASTMatchersTest, IsDerivedFrom) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -1987,6 +1987,51 @@
   llvm::Regex::RegexFlags Flags,
   StringRef MatcherID);
 
+template 
+constexpr auto applyMatcherImpl(F &, Tuple &,
+std::index_sequence) {
+  return std::forward(f)(std::get(std::forward(args))...);
+}
+
+template 
+constexpr auto applyMatcher(F &, Tuple &) {
+  return applyMatcherImpl(
+  std::forward(f), std::forward(args),
+  std::make_index_sequence<
+  std::tuple_size::type>::value>());
+}
+
+template  struct AnyOfHelper {
+  std::tuple NodeMatchers;
+  AnyOfHelper(MatcherTypes const &... NodeMatchers)
+  : NodeMatchers(NodeMatchers...) {}
+
+  template  struct GetCladeImpl;
+  template 
+  struct GetCladeImpl> {
+using Type = T;
+  };
+  template  struct GetClade : GetCladeImpl {};
+  using CladeType = typename GetClade::Type;
+
+  template 
+  BindableMatcher with(InnerMatchers &&... InnerMatcher) const {
+// TODO: Use std::apply from c++17
+return VariadicAllOfMatcher()(applyMatcher(
+internal::VariadicOperatorMatcherFunc<
+0, std::numeric_limits::max()>{
+internal::DynTypedMatcher::VO_AnyOf},
+applyMatcher(
+[&](auto... Matcher) {
+  return std::make_tuple(Matcher(
+  

[PATCH] D94126: [ASTMatchers] Make it possible to use empty variadic matchers

[Stephen Kelly via Phabricator via cfe-commits]

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

When debugging a matcher it is convenient to be able to comment out
nested matchers experimentally. This stops working when we have less
than two matchers.

Additionally, the removal of the contraint makes it possible to use
these matchers in generic code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94126

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -184,6 +184,10 @@
hasArgument(1, hasType(pointsTo(recordDecl(hasName("T"),
hasArgument(2, integerLiteral(equals(3))),
hasArgument(3, integerLiteral(equals(4)));
+
+  EXPECT_TRUE(
+  matches("int F() { return 1; }", integerLiteral(allOf(equals(1);
+  EXPECT_TRUE(matches("int F() { return 1; }", integerLiteral(allOf(;
 }
 
 TEST_P(ASTMatchersTest, Has) {
@@ -463,6 +467,10 @@
   EXPECT_TRUE(
   matches("void f() try { } catch (int) { } catch (...) { }",
   cxxCatchStmt(anyOf(hasDescendant(varDecl()), isCatchAll();
+
+  EXPECT_TRUE(
+  matches("int F() { return 1; }", integerLiteral(anyOf(equals(1);
+  EXPECT_TRUE(matches("int F() { return 1; }", integerLiteral(anyOf(;
 }
 
 TEST_P(ASTMatchersTest, IsDerivedFrom) {
@@ -2451,6 +2459,10 @@
   recordDecl(eachOf(has(fieldDecl(hasName("a")).bind("v")),
 has(fieldDecl(hasName("b")).bind("v",
   std::make_unique>("v", 2)));
+
+  EXPECT_TRUE(
+  matches("int F() { return 1; }", integerLiteral(eachOf(equals(1);
+  EXPECT_TRUE(matches("int F() { return 1; }", integerLiteral(eachOf(;
 }
 
 TEST_P(ASTMatchersTest, EachOf_BehavesLikeAnyOfUnlessBothMatch) {
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -399,6 +399,8 @@
ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder,
ArrayRef InnerMatchers) {
+  if (InnerMatchers.empty())
+return true;
   BoundNodesTreeBuilder Result;
   bool Matched = false;
   for (const DynTypedMatcher  : InnerMatchers) {
@@ -416,6 +418,8 @@
   ASTMatchFinder *Finder,
   BoundNodesTreeBuilder *Builder,
   ArrayRef InnerMatchers) {
+  if (InnerMatchers.empty())
+return true;
   for (const DynTypedMatcher  : InnerMatchers) {
 BoundNodesTreeBuilder Result = *Builder;
 if (InnerMatcher.matches(DynNode, Finder, )) {
@@ -952,13 +956,13 @@
 const internal::VariadicDynCastAllOfMatcher
 designatedInitExpr;
 const internal::VariadicOperatorMatcherFunc<
-2, std::numeric_limits::max()>
+0, std::numeric_limits::max()>
 eachOf = {internal::DynTypedMatcher::VO_EachOf};
 const internal::VariadicOperatorMatcherFunc<
-2, std::numeric_limits::max()>
+0, std::numeric_limits::max()>
 anyOf = {internal::DynTypedMatcher::VO_AnyOf};
 const internal::VariadicOperatorMatcherFunc<
-2, std::numeric_limits::max()>
+0, std::numeric_limits::max()>
 allOf = {internal::DynTypedMatcher::VO_AllOf};
 const internal::VariadicOperatorMatcherFunc<1, 1> optionally = {
 internal::DynTypedMatcher::VO_Optionally};
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2637,21 +2637,21 @@
 ///
 /// Usable as: Any Matcher
 extern const internal::VariadicOperatorMatcherFunc<
-2, std::numeric_limits::max()>
+0, std::numeric_limits::max()>
 eachOf;
 
 /// Matches if any of the given matchers matches.
 ///
 /// Usable as: Any Matcher
 extern const internal::VariadicOperatorMatcherFunc<
-2, std::numeric_limits::max()>
+0, std::numeric_limits::max()>
 anyOf;
 
 /// Matches if all given matchers match.
 ///
 /// Usable as: Any Matcher
 extern const internal::VariadicOperatorMatcherFunc<
-2, std::numeric_limits::max()>
+0, std::numeric_limits::max()>
 allOf;
 
 /// Matches any node regardless of the submatcher.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92434: [NFC][AMDGPU] AMDGPU code object V4 ABI documentation

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: llvm/docs/AMDGPUUsage.rst:959
+ = = =
+ ``EF_AMDGPU_FEATURE_XNACK_V2``0x01  Indicates if the ``xnack``
+ target feature is

Where do these flags live?
I do not see them in 
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/BinaryFormat/ELF.h#L738

Do these doc changes reflect a patch that did not land yet?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92434

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

[PATCH] D93763: [clangd] Add a flag to disable the documentLinks LSP request

[Giulio Girardi via Phabricator via cfe-commits]

rapgenic updated this revision to Diff 314718.
rapgenic marked an inline comment as done.

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

https://reviews.llvm.org/D93763

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/test/document-link.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -339,6 +339,15 @@
 Hidden,
 };
 
+opt IncludesAsLinks{
+"includes-as-links",
+cat(Features),
+desc("Provide a document link to the files included with #include "
+ "directive. If set to false, include files can still be opened with "
+ "go to definition feature"),
+init(true),
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -825,6 +834,7 @@
   Opts.PreserveRecoveryASTType = RecoveryASTType;
   Opts.FoldingRanges = FoldingRanges;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
+  Opts.IncludesAsLinks = IncludesAsLinks;
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
   Opts.CodeComplete.Limit = LimitResults;
Index: clang-tools-extra/clangd/test/document-link.test
===
--- clang-tools-extra/clangd/test/document-link.test
+++ clang-tools-extra/clangd/test/document-link.test
@@ -37,6 +37,26 @@
 # CHECK-NEXT:}
 # CHECK-NEXT:  ]
 # CHECK-NEXT:}
+---
+# Go to definition on include (necessary when documentLinks are disabled)
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":15}}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 0,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 0,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "uri": "file://{{.*}}/{{([A-Z]:/)?}}stdint.h"
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
 
 ---
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -61,6 +61,10 @@
 std::function TweakFilter = [](const Tweak ) {
   return !T.hidden(); // only enable non-hidden tweaks.
 };
+/// If set, include files in #include directives are exposed to the clients
+/// as documentLinks. If disabled include files can still be opened with the
+/// go to definition feature.
+bool IncludesAsLinks = true;
   };
 
   ClangdLSPServer(Transport , const ThreadsafeFS ,
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -610,10 +610,6 @@
 {"definitionProvider", true},
 {"implementationProvider", true},
 {"documentHighlightProvider", true},
-{"documentLinkProvider",
- llvm::json::Object{
- {"resolveProvider", false},
- }},
 {"hoverProvider", true},
 {"renameProvider", std::move(RenameProvider)},
 {"selectionRangeProvider", true},
@@ -640,6 +636,12 @@
  llvm::json::Object{{"scopes", buildHighlightScopeLookupTable()}}});
   if (Opts.FoldingRanges)
 Result.getObject("capabilities")->insert({"foldingRangeProvider", true});
+  if (Opts.IncludesAsLinks)
+// Currently we only provide documentLink for #included headers.
+// If that's turned off, let clients avoid sending the request altogether.
+Result.getObject("capabilities")
+->insert({"documentLinkProvider",
+  llvm::json::Object{{"resolveProvider", false}}});
   Reply(std::move(Result));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93763: [clangd] Add a flag to disable the documentLinks LSP request

[Giulio Girardi via Phabricator via cfe-commits]

rapgenic marked 4 inline comments as done.
rapgenic added a comment.

> In the future it may make sense to have other documentLinks too. (e.g. 
> imagine a comment Configures the Frobnicator - we should linkify Frobnicator 
> to point to a class, once documentLink supports target ranges). So we should 
> decide if this feature is "disable documentLink" or "don't include included 
> headers in documentLink". This affects e.g. naming of the flag, which is 
> awkward to change later.

I think it's already like that, in that specific case, and it uses 
goToDefinition instead: if I ctrl+click on a variable/class/function name in a 
comment i can actually go to the definition, if I am not wrong.

As for the naming of the option, maybe the title of the patch is not too 
accurate, however from the option description it's quite clear that it's only 
related to include files, so any additional feature using documentLinks should 
be easy to add while keeping this option too.

> Do we really want to add command-line flags for this or does it rather belong 
> as a config option? (In which case we'd want to change the behavior of the 
> method, not turn the method itself on/off, to allow the config option to vary 
> across files in the usual way).

In my opinion (and also by looking at the already implemented options in 
.clangd config file) the config file seems to be related to project specific 
configurations (compilation flags, indexing, etc.), whereas the option that 
this patch implements is more related to the "editing experience", which is 
something one should not change very often. So I think that it might not be 
worth putting it in the config file.




Comment at: clang-tools-extra/clangd/test/xrefs.test:4
 ---
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"extern
 int x;\nint x = 0;\nint y = x;"}}}
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"#include
 \nextern int x;\nint x = 0;\nint y = x;"}}}
 ---

sammccall wrote:
> Having tests depend on external headers is a bit of a pain downstream - it 
> means they need to run in an environment where the headers are available.
> 
> We do this by necessity in `document-link.test` - maybe we should move this 
> test here?
> (Personally I'm comfortable with this functionality only being covered by 
> unit-tests, as it is now though).
I'm moving the test, as it is related to document links it seems pertinent to 
put it there, if you prefer we can leave it out of course


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

https://reviews.llvm.org/D93763

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

[clang] 16c6e9c - [ASTMatchers] Fix child traversal over range-for loops

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2021-01-05T21:29:37Z
New Revision: 16c6e9c58e9ae50a775945e6b407f1891f353d2f

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

LOG: [ASTMatchers] Fix child traversal over range-for loops

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

Added: 


Modified: 
clang/lib/ASTMatchers/ASTMatchFinder.cpp
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Removed: 




diff  --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp 
b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index 99d95838af61..39bdb94e62c6 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -236,6 +236,20 @@ class MatchChildASTVisitor
 ScopedIncrement ScopedDepth();
 return traverse(TAL);
   }
+  bool TraverseCXXForRangeStmt(CXXForRangeStmt *Node) {
+if (!Finder->isTraversalIgnoringImplicitNodes())
+  return VisitorBase::TraverseCXXForRangeStmt(Node);
+if (!Node)
+  return true;
+ScopedIncrement ScopedDepth();
+if (auto *Init = Node->getInit())
+  if (!match(*Init))
+return false;
+if (!match(*Node->getLoopVariable()) || !match(*Node->getRangeInit()) ||
+!match(*Node->getBody()))
+  return false;
+return VisitorBase::TraverseStmt(Node->getBody());
+  }
   bool TraverseLambdaExpr(LambdaExpr *Node) {
 if (!Finder->isTraversalIgnoringImplicitNodes())
   return VisitorBase::TraverseLambdaExpr(Node);
@@ -575,8 +589,6 @@ class MatchASTVisitor : public 
RecursiveASTVisitor,
 
 if (isTraversalIgnoringImplicitNodes()) {
   IgnoreImplicitChildren = true;
-  if (Node.get())
-ScopedTraversal = true;
 }
 
 ASTNodeNotSpelledInSourceScope RAII(this, ScopedTraversal);

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index e706ea4b2a54..19ab6187d960 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2553,7 +2553,9 @@ struct CtorInitsNonTrivial : NonTrivial
 int arr[2];
 for (auto i : arr)
 {
-
+  if (true)
+  {
+  }
 }
   }
   )cpp";
@@ -2596,6 +2598,33 @@ struct CtorInitsNonTrivial : NonTrivial
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
 EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
+  {
+auto M = cxxForRangeStmt(hasDescendant(ifStmt()));
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+EXPECT_TRUE(matches(
+Code, traverse(TK_AsIs, cxxForRangeStmt(has(declStmt(
+hasSingleDecl(varDecl(hasName("i");
+EXPECT_TRUE(
+matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+   cxxForRangeStmt(has(varDecl(hasName("i")));
+  }
+  {
+EXPECT_TRUE(matches(
+Code, traverse(TK_AsIs, cxxForRangeStmt(has(declStmt(hasSingleDecl(
+varDecl(hasInitializer(declRefExpr(
+to(varDecl(hasName("arr");
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+   cxxForRangeStmt(has(declRefExpr(
+   to(varDecl(hasName("arr");
+  }
+  {
+auto M = cxxForRangeStmt(has(compoundStmt()));
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
   {
 auto M = binaryOperator(hasOperatorName("!="));
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
@@ -2659,7 +2688,8 @@ struct CtorInitsNonTrivial : NonTrivial
  true, {"-std=c++20"}));
   }
   {
-auto M = cxxForRangeStmt(has(declStmt()));
+auto M =
+cxxForRangeStmt(has(declStmt(hasSingleDecl(varDecl(hasName("i"));
 EXPECT_TRUE(
 matchesConditionally(Code, traverse(TK_AsIs, M), true, 
{"-std=c++20"}));
 EXPECT_FALSE(
@@ -2679,6 +2709,19 @@ struct CtorInitsNonTrivial : NonTrivial
 matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
  true, {"-std=c++20"}));
   }
+  {
+auto M = cxxForRangeStmt(
+has(declStmt(hasSingleDecl(varDecl(
+hasName("a"),
+hasInitializer(declRefExpr(to(varDecl(hasName("arr"),
+hasLoopVariable(varDecl(hasName("i"))),
+hasRangeInit(declRefExpr(to(varDecl(hasName("a"));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, 
{"-std=c++20"}));
+EXPECT_TRUE(
+

[PATCH] D94031: [ASTMatchers] Fix child traversal over range-for loops

[Stephen Kelly 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 rG16c6e9c58e9a: [ASTMatchers] Fix child traversal over 
range-for loops (authored by stephenkelly).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94031

Files:
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2553,7 +2553,9 @@
 int arr[2];
 for (auto i : arr)
 {
-
+  if (true)
+  {
+  }
 }
   }
   )cpp";
@@ -2596,6 +2598,33 @@
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
 EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
+  {
+auto M = cxxForRangeStmt(hasDescendant(ifStmt()));
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+EXPECT_TRUE(matches(
+Code, traverse(TK_AsIs, cxxForRangeStmt(has(declStmt(
+hasSingleDecl(varDecl(hasName("i");
+EXPECT_TRUE(
+matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+   cxxForRangeStmt(has(varDecl(hasName("i")));
+  }
+  {
+EXPECT_TRUE(matches(
+Code, traverse(TK_AsIs, cxxForRangeStmt(has(declStmt(hasSingleDecl(
+varDecl(hasInitializer(declRefExpr(
+to(varDecl(hasName("arr");
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+   cxxForRangeStmt(has(declRefExpr(
+   to(varDecl(hasName("arr");
+  }
+  {
+auto M = cxxForRangeStmt(has(compoundStmt()));
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
   {
 auto M = binaryOperator(hasOperatorName("!="));
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
@@ -2659,7 +2688,8 @@
  true, {"-std=c++20"}));
   }
   {
-auto M = cxxForRangeStmt(has(declStmt()));
+auto M =
+cxxForRangeStmt(has(declStmt(hasSingleDecl(varDecl(hasName("i"));
 EXPECT_TRUE(
 matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
 EXPECT_FALSE(
@@ -2679,6 +2709,19 @@
 matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
  true, {"-std=c++20"}));
   }
+  {
+auto M = cxxForRangeStmt(
+has(declStmt(hasSingleDecl(varDecl(
+hasName("a"),
+hasInitializer(declRefExpr(to(varDecl(hasName("arr"),
+hasLoopVariable(varDecl(hasName("i"))),
+hasRangeInit(declRefExpr(to(varDecl(hasName("a"));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
   Code = R"cpp(
 void hasDefaultArg(int i, int j = 0)
 {
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -236,6 +236,20 @@
 ScopedIncrement ScopedDepth();
 return traverse(TAL);
   }
+  bool TraverseCXXForRangeStmt(CXXForRangeStmt *Node) {
+if (!Finder->isTraversalIgnoringImplicitNodes())
+  return VisitorBase::TraverseCXXForRangeStmt(Node);
+if (!Node)
+  return true;
+ScopedIncrement ScopedDepth();
+if (auto *Init = Node->getInit())
+  if (!match(*Init))
+return false;
+if (!match(*Node->getLoopVariable()) || !match(*Node->getRangeInit()) ||
+!match(*Node->getBody()))
+  return false;
+return VisitorBase::TraverseStmt(Node->getBody());
+  }
   bool TraverseLambdaExpr(LambdaExpr *Node) {
 if (!Finder->isTraversalIgnoringImplicitNodes())
   return VisitorBase::TraverseLambdaExpr(Node);
@@ -575,8 +589,6 @@
 
 if (isTraversalIgnoringImplicitNodes()) {
   IgnoreImplicitChildren = true;
-  if (Node.get())
-ScopedTraversal = true;
 }
 
 ASTNodeNotSpelledInSourceScope RAII(this, ScopedTraversal);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93747: Rename debug linkage name with -funique-internal-linkage-names

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

>> In D93747#2470178 , @tmsriram wrote:
>>
>>> In D93747#2469556 , @hoy wrote:
>>>
> In D93656 , @dblaikie wrote:
> Though the C case is interesting - it means you'll end up with C 
> functions with the same DWARF 'name' but different linkage name. I don't 
> know what that'll do to DWARF consumers - I guess they'll probably be 
> OK-ish with it, as much as they are with C++ overloading. I think there 
> are some cases of C name mangling so it's probably supported/OK-ish with 
> DWARF Consumers. Wouldn't hurt for you to take a look/see what happens in 
> that case with a debugger like gdb/check other cases of C name mangling 
> to see what DWARF they end up creating (with both GCC and Clang).

 I did a quick experiment with C name managing with GCC and -flto. It turns 
 out the `DW_AT_linkage_name` field of `DW_TAG_subprogram` is never set for 
 C programs. If set, the gdb debugger will use that field to match the user 
 input and set breakpoints. Therefore, giving `DW_AT_linkage_name` a 
 uniquefied name prevents the debugger from setting a breakpoint based on 
 source names unless the user specifies a decorated name.

 Hence, this approach sounds like a workaround for us when the profile 
 quality matters more than debugging experience. I'm inclined to have it 
 under a switch. What do you think?
>>>
>>> Just a thought, we could always check if rawLinkageName is set and only set 
>>> it when it is not null.  That seems safe without needing the option. Not a 
>>> strong opinion.
>>
>> Was this thread concluded? Doesn't look like any check was added?
>
> Thanks for the detailed analysis! Moving forward I would like to see a more 
> clear contract about debug linkage names between the compiler and debugger as 
> a guideline to code cloning work. For this patch, I'm adding a switch for it 
> with a default value "on" since AutoFDO and propeller are the primary 
> consumers of the work here and they mostly care about profile quality more 
> than debugging experience. But correct me if I'm wrong and I'll flip the 
> default value.

Presumably people are going to want to debug these binaries - I'm not sure it's 
a "one or the other" situation as it sounds like @tmsriram was suggesting by 
only modifying the linkage name when it's already set this might produce a 
better debugging experience, if I'm following the discussion correctly?

But I'm pretty sure there are places where C supports name mangling, so I 
wouldn't mind understanding the gdb behavior in more detail - perhaps there's a 
way to make it work better.

Using Clang's __attribute__((overloadable)) support, I was able to produce a C 
program with mangled names, with DW_AT_linkage_names on those functions, like 
this:

  $ cat test.c
  void __attribute__((overloadable)) f(int i) {
  }
  void __attribute__((overloadable)) f(long l) {
  }
  int main() {
f(3);
f(5l);
  }
  blaikie@blaikie-linux2:~/dev/scratch$ clang-tot test.c -g
  blaikie@blaikie-linux2:~/dev/scratch$ llvm-dwarfdump-tot a.out
  a.out:  file format elf64-x86-64
  
  .debug_info contents:
  0x: Compile Unit: length = 0x009e, format = DWARF32, version = 
0x0004, abbr_offset = 0x, addr_size = 0x08 (next unit at 0x00a2)
  
  0x000b: DW_TAG_compile_unit
DW_AT_producer("clang version 12.0.0 
(g...@github.com:llvm/llvm-project.git 
20013e0940dea465a173c3c7ce60f0e419242ef8)")
DW_AT_language(DW_LANG_C99)
DW_AT_name("test.c")
DW_AT_stmt_list   (0x)
DW_AT_comp_dir("/usr/local/google/home/blaikie/dev/scratch")
DW_AT_low_pc  (0x00401110)
DW_AT_high_pc (0x0040114c)
  
  0x002a:   DW_TAG_subprogram
  DW_AT_low_pc(0x00401110)
  DW_AT_high_pc   (0x00401119)
  DW_AT_frame_base(DW_OP_reg6 RBP)
  DW_AT_linkage_name  ("_Z1fi")
  DW_AT_name  ("f")
  DW_AT_decl_file 
("/usr/local/google/home/blaikie/dev/scratch/test.c")
  DW_AT_decl_line (1)
  DW_AT_prototyped(true)
  DW_AT_external  (true)
  
  0x0043: DW_TAG_formal_parameter
DW_AT_location(DW_OP_fbreg -4)
DW_AT_name("i")
DW_AT_decl_file   
("/usr/local/google/home/blaikie/dev/scratch/test.c")
DW_AT_decl_line   (1)
DW_AT_type(0x0093 "int")
  
  0x0051: NULL
  
  0x0052:   DW_TAG_subprogram
  DW_AT_low_pc(0x00401120)
  DW_AT_high_pc   (0x0040112a)
  

[PATCH] D92812: [X86] AMD Znver3 (Family 19H) Enablement

[Ganesh Gopalasubramanian via Phabricator via cfe-commits]

GGanesh updated this revision to Diff 314703.
GGanesh edited the summary of this revision.
GGanesh added a comment.

Updaing the patch so that the simplified patch adds only few missing znver3 
tests. The subsequent patches will comprehensively enable other znver3 features.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92812

Files:
  clang/test/Driver/x86-march.c
  clang/test/Frontend/x86-target-cpu.c
  llvm/test/MC/X86/x86_long_nop.s


Index: llvm/test/MC/X86/x86_long_nop.s
===
--- llvm/test/MC/X86/x86_long_nop.s
+++ llvm/test/MC/X86/x86_long_nop.s
@@ -15,6 +15,8 @@
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu %s 
-mcpu=znver1 | llvm-objdump -d --no-show-raw-insn - | FileCheck %s 
--check-prefix=LNOP15
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=x86_64-pc-linux-gnu 
-mcpu=znver2 %s | llvm-objdump -d --no-show-raw-insn - | FileCheck %s 
--check-prefix=LNOP15
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu %s 
-mcpu=znver2 | llvm-objdump -d --no-show-raw-insn - | FileCheck %s 
--check-prefix=LNOP15
+# RUN: llvm-mc -filetype=obj -arch=x86 -triple=x86_64-pc-linux-gnu 
-mcpu=znver3 %s | llvm-objdump -d --no-show-raw-insn - | FileCheck %s 
--check-prefix=LNOP15
+# RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu %s 
-mcpu=znver3 | llvm-objdump -d --no-show-raw-insn - | FileCheck %s 
--check-prefix=LNOP15
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu -mcpu=nehalem 
%s | llvm-objdump -d --no-show-raw-insn - | FileCheck --check-prefix=LNOP10 %s
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu 
-mcpu=westmere %s | llvm-objdump -d --no-show-raw-insn - | FileCheck 
--check-prefix=LNOP10 %s
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu 
-mcpu=sandybridge %s | llvm-objdump -d --no-show-raw-insn - | FileCheck 
--check-prefix=LNOP15 %s
Index: clang/test/Frontend/x86-target-cpu.c
===
--- clang/test/Frontend/x86-target-cpu.c
+++ clang/test/Frontend/x86-target-cpu.c
@@ -36,5 +36,6 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-cpu btver2 -verify %s
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-cpu znver1 -verify %s
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-cpu znver2 -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-cpu znver3 -verify %s
 //
 // expected-no-diagnostics
Index: clang/test/Driver/x86-march.c
===
--- clang/test/Driver/x86-march.c
+++ clang/test/Driver/x86-march.c
@@ -179,6 +179,10 @@
 // RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=znver2 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=znver2
 // znver2: "-target-cpu" "znver2"
+//
+// RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=znver3 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=znver3
+// znver3: "-target-cpu" "znver3"
 
 // RUN: %clang -target x86_64 -c -### %s -march=x86-64 2>&1 | FileCheck %s 
--check-prefix=x86-64
 // x86-64: "-target-cpu" "x86-64"


Index: llvm/test/MC/X86/x86_long_nop.s
===
--- llvm/test/MC/X86/x86_long_nop.s
+++ llvm/test/MC/X86/x86_long_nop.s
@@ -15,6 +15,8 @@
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu %s -mcpu=znver1 | llvm-objdump -d --no-show-raw-insn - | FileCheck %s --check-prefix=LNOP15
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=x86_64-pc-linux-gnu -mcpu=znver2 %s | llvm-objdump -d --no-show-raw-insn - | FileCheck %s --check-prefix=LNOP15
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu %s -mcpu=znver2 | llvm-objdump -d --no-show-raw-insn - | FileCheck %s --check-prefix=LNOP15
+# RUN: llvm-mc -filetype=obj -arch=x86 -triple=x86_64-pc-linux-gnu -mcpu=znver3 %s | llvm-objdump -d --no-show-raw-insn - | FileCheck %s --check-prefix=LNOP15
+# RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu %s -mcpu=znver3 | llvm-objdump -d --no-show-raw-insn - | FileCheck %s --check-prefix=LNOP15
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu -mcpu=nehalem %s | llvm-objdump -d --no-show-raw-insn - | FileCheck --check-prefix=LNOP10 %s
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu -mcpu=westmere %s | llvm-objdump -d --no-show-raw-insn - | FileCheck --check-prefix=LNOP10 %s
 # RUN: llvm-mc -filetype=obj -arch=x86 -triple=i686-pc-linux-gnu -mcpu=sandybridge %s | llvm-objdump -d --no-show-raw-insn - | FileCheck --check-prefix=LNOP15 %s
Index: clang/test/Frontend/x86-target-cpu.c
===
--- clang/test/Frontend/x86-target-cpu.c
+++ clang/test/Frontend/x86-target-cpu.c
@@ -36,5 +36,6 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -target-cpu btver2 

[PATCH] D78058: option to write files to memory instead of disk

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

dexonsmith added a comment.

In D78058#2471735 , @sammccall wrote:

> @dexonsmith thanks for sharing!
> Some initial thoughts since abstracting outputs is something we're starting 
> to care about too...

Thanks for looking.

> This doesn't appear to be an extension point - you can write to an InMemoryFS 
> or to real disk, but not to anything else. If we're going to add abstractions 
> around output, I think they should support flexible use in libraries (which 
> doesn't need to cost complexity here: FS vs MemoryFS vs custom storage can 
> implement the same interface).
> With the right interface, I think this obviates the need for RequestedOutput 
> and generalizes it - a caller can install an intercepting output backend that 
> hooks certain paths and delegates the rest to the old backend.
> (Of course such a backend could be provided, but it gets the complexity out 
> of the way of the core abstractions)

I agree with your concerns with the monolithic design. My motivation for it is 
to get the memory optimization of using a write-through memory buffer when the 
output is needed both on-disk and in-memory. I imagine similar concerns if/when 
we add a CAS-based storage option, where it's likely more efficient to ask the 
CAS to materialize the on-disk file than to write it ourselves.

I'll think about whether there's a clean way to model these kinds of 
interactions between storage implementations without a monolithic design; let 
me know if you have any concrete ideas.

The option I see is to have an abstract extension point that allows libraries 
to plug in custom storage, but keep the monolithic design for on-disk and 
in-memory (and eventually CAS).

> I think the lifecycle and allowed operations on outputs would be clearer if 
> an `Output` class was returned, with methods, rather than a stream + handle 
> and various "free" functions (well, methods on OutputManager) than manipulate 
> the handle.
> This would again make it easier to reason about what operations are part of a 
> storage backend: there'd be some `OutputBackend` interface to implement, and 
> it would hand out `Output` objects which again must be implemented. (This 
> would be reminiscent of FileManager + vfs::FileSystem + vfs::File).

I'll think some more about the options here. The current design is motivated by 
how outputs are currently used / referenced in `CompilerInstance` (and how the 
streams are passed around separately to APIs that don't know anything about 
outputs); i.e., the current design seems simple to land as an incremental 
change. But it's probably not hard to make it a bit cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78058

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

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Headers/__clang_hip_runtime_wrapper.h:73-74
+#define __HOST_DEVICE__
\
+  static __host__ __device__ inline __attribute__((always_inline))
+__HOST_DEVICE__ double _Cosh(double x, double y) { return cosh(x) * y; }
+__HOST_DEVICE__ float _FCosh(float x, float y) { return coshf(x) * y; }

I don't think we want to provide a HD implementation.
This will potentially change the meaning of these functions on the host side vs 
what they do in a C++ compilation.
It should probably be just `__device__`.

Next question is -- do we want to provide the definitions, or would just 
declarations be sufficient?
In other words -- are these functions needed for some standard C++ library 
functionality that we expect to work on the GPU?
If it's just to aid with overload resolution, declarations should do. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93638

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

[PATCH] D77598: Integral template argument suffix and cast printing

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

rsmith added inline comments.



Comment at: clang/lib/AST/TemplateBase.cpp:111-115
+  Out << "u8'" << Val << "'";
+else if (T->isUnsignedIntegerType() && T->isChar16Type())
+  Out << "u16'" << Val << "'";
+else if (T->isUnsignedIntegerType() && T->isChar32Type())
+  Out << "u32'" << Val << "'";

reikdas wrote:
> rsmith wrote:
> > This isn't correct: `u8'x'` will print as `u8'120'`. Perhaps you can factor 
> > code to do this properly out of `StmtPrinter::VisitCharacterLiteral`.
> > 
> > Also, the prefix for `char16_t` literals is `u`, not `u16`, and the prefix 
> > for `char32_t` literals is `U`, not `u32`.
> > This isn't correct: `u8'x'` will print as `u8'120'`. Perhaps you can factor 
> > code to do this properly out of `StmtPrinter::VisitCharacterLiteral`.
> 
> I partially addressed this comment. I wasn't able to find a suitable example 
> to test `u8'x'` being printed as `u8'120'`. @rsmith could you please help me 
> by showing me a reproducer?
> 
> 
You'll need a C++20 test (or a test using `-fchar8_t`). Then:
```
template struct A {};
A::B b; // expected-error {{no type named 'B' in 'A'}}
```


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

https://reviews.llvm.org/D77598

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

[clang] 2168942 - [Coverage] Fix Profile test failures from commit rG9f2967bcfe2f

[Alan Phipps via cfe-commits]

Author: Alan Phipps
Date: 2021-01-05T14:53:07-06:00
New Revision: 216894211713bbb1e8beb249f2b008c11a9d8c2c

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

LOG: [Coverage] Fix Profile test failures from commit rG9f2967bcfe2f

Fix test failures with Branch Coverage tests from commit rG9f2967bcfe2f
that failed build on builder clang-x64-windows-msvc while building llvm:
http://lab.llvm.org:8011/#/builders/123/builds/2162

Added: 


Modified: 
clang/test/Profile/branch-logical-mixed.cpp

Removed: 




diff  --git a/clang/test/Profile/branch-logical-mixed.cpp 
b/clang/test/Profile/branch-logical-mixed.cpp
index cbfcf061f152..04b51d81d13b 100644
--- a/clang/test/Profile/branch-logical-mixed.cpp
+++ b/clang/test/Profile/branch-logical-mixed.cpp
@@ -4,7 +4,7 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -main-file-name 
branch-logical-mixed.cpp %s -o - -emit-llvm -fprofile-instrument=clang | 
FileCheck -allow-deprecated-dag-overlap %s
 
 
-// CHECK: @[[FUNC:__profc__Z4funcv]] = private global [61 x i64] 
zeroinitializer
+// CHECK: @[[FUNC:__profc__Z4funcv]] = {{.*}} global [61 x i64] zeroinitializer
 
 
 // CHECK-LABEL: @_Z4funcv()



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

[PATCH] D93587: [hip] Fix HIP version parsing.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:91
 
-void RocmInstallationDetector::ParseHIPVersionFile(llvm::StringRef V) {
+bool RocmInstallationDetector::parseHIPVersionFile(llvm::StringRef V) {
   SmallVector VersionParts;

A comment describing expected input format would be helpful here.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:91
 
-void RocmInstallationDetector::ParseHIPVersionFile(llvm::StringRef V) {
+bool RocmInstallationDetector::parseHIPVersionFile(llvm::StringRef V) {
   SmallVector VersionParts;

tra wrote:
> A comment describing expected input format would be helpful here.
A comment that it returns `true` on failure to parse would be helpful.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:97
   for (auto Part : VersionParts) {
 auto Splits = Part.split('=');
+if (Splits.first == "HIP_VERSION_MAJOR") {

`Part.trim().split()` (or, maybe, `rtrim()`if you only care about EOL) would 
save you trimming in each individual case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93587

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

[PATCH] D93638: [hip] Enable HIP compilation with ` on MSVC.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Headers/cuda_wrappers/ymath.h:16
+
+#pragma clang force_cuda_host_device begin
+

I am wondering whether we only want to do this for windows, since ymath.h may 
be an ordinary users' header file on linux.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93638

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

[PATCH] D94031: [ASTMatchers] Fix child traversal over range-for loops

[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!




Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:245
+ScopedIncrement ScopedDepth();
+if (!match(*Node->getLoopVariable()) || !match(*Node->getRangeInit()) ||
+!match(*Node->getBody()))

steveire wrote:
> aaron.ballman wrote:
> > steveire wrote:
> > > aaron.ballman wrote:
> > > > Should we be traversing the init statement before the loop variable so 
> > > > that the traversals happen in lexical order?
> > > Do you mean that in 
> > > 
> > > ```
> > > for (auto i : arr)
> > > {
> > > }
> > > ```
> > > 
> > > to visit the `arr` before the `auto i`? 
> > > 
> > > I think visiting the `auto i` before the `arr` makes sense.
> > Nope, I mean that in:
> > ```
> > for (int i = 12; auto j : {1, 2, 3, 4}) {}
> > ```
> > we should visit the `int i = 12;` before the `auto j`
> That was actually missing entirely. Added now.
I thought `getRangeInit()` was doing that already, so nice!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94031

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

[PATCH] D92954: [clang-offload-bundler] Add option -list

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:176
+  /// List bundle IDs in \a Input.
+  virtual Error listBundleIDs(MemoryBuffer ) {
+if (Error Err = ReadHeader(Input))

Now that listBundleIDs is only used by ` ListBundleIDsInFile()`, perhaps it 
could all be simplified and moved out of the class.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:882
+// List bundle IDs. Return true if an error was found.
+static Error ListBundleIDsInFile() {
+  // Open Input file.

I'd pass InputFileNames as an argument. Makes it easier to tell what the 
function needs.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:1031-1035
+  Error = true;
+  reportError(createStringError(
+  errc::invalid_argument,
+  "for the --outputs option: must be specified at least once!"));
+}

Does it make sense to continue once we know that CLI options are wrong?
If we just early-exit with an error that may help simplifying the code below a 
bit.





Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:1047
+errc::invalid_argument, "-unbundle and -list cannot be used 
together"));
+  } else if (ListBundleIDs) {
+if (InputFileNames.size() != 1) {

Perhaps we can separate list option processing from bundle/unbundle?

I think if we could do something like this it would be more readable:
```
if (ListBundleIDs) {
  if (Unbundle) {
error...
exit.
  }
  ... other list-specific checks...
  ListBundleIDsInFile(InputFileNames)
  exit 0;
}

// complicated bundle/unbundle logic can proceed without having to bother about 
`list` option.
```


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

https://reviews.llvm.org/D92954

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

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1221
+if (Style.EmptyLineBeforeAccessModifier &&
+PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) &&
+RootToken.NewlinesBefore == 1)

thezbyg wrote:
> HazardyKnusperkeks wrote:
> > thezbyg wrote:
> > > MyDeveloperDay wrote:
> > > > maybe I don't understand the logic but why is this `r_brace` and 
> > > > shouldn't we be looking at the "Last Non Comment" token?
> > > I think that the logic is to only add empty line when access modifier is 
> > > after member function body (`r_brace` indicates this) or declaration of 
> > > field/function. If this check is changed to look at "last non comment" 
> > > token, then empty line will be inserted in code like this:
> > > ```
> > > struct Foo {
> > >   int i;
> > >   //comment
> > > private:
> > >   int j;
> > > }
> > > ```
> > But with the given Name it should add an empty line there! Otherwise you 
> > should use an enum and not a bool to control wether a comment should 
> > suppress the empty line or not. Also you should make the exception(s) clear 
> > with unit tests.
> Current clang-format behavior of access modifier separation was added in 
> commit (fd433365e08da024ddc99dd0d7bce8e89d8c5469) and the test function is 
> called //SeparatesLogicalBlocks//. So could we simply rename this new option 
> to //SeparateLogicalBlocks// and leave the bool type? Option description 
> would contain code examples and further explanation what logical block means. 
> Setting //SeparateLogicalBlocks// option to false would disable empty line 
> insertion, but would not remove existing empty lines.
> 
> Is option name //EmptyLineBeforeAccessModifier// acceptable if enum is used? 
> And how should I name enum values? One enum value could be called //Never//, 
> but other must indicate that empty line is only inserted when access modifier 
> is after member function body or field/function/struct declaration.
> 
> I think that you incorrectly assumed from my previous comment, that only 
> comments suppress empty line insertion. No empty line is inserted in all 
> following code examples:
> ```
> struct Foo {
> private:
>   int j;
> };
> ```
> ```
> struct Foo {
> private:
> public:
>   int j;
> };
> ```
> ```
> struct Foo {
> #ifdef FOO
> private:
> #endif
>   int j;
> };
> ```
> Current clang-format behavior of access modifier separation was added in 
> commit (fd433365e08da024ddc99dd0d7bce8e89d8c5469) and the test function is 
> called //SeparatesLogicalBlocks//. So could we simply rename this new option 
> to //SeparateLogicalBlocks// and leave the bool type? Option description 
> would contain code examples and further explanation what logical block means. 
> Setting //SeparateLogicalBlocks// option to false would disable empty line 
> insertion, but would not remove existing empty lines.
> 
> Is option name //EmptyLineBeforeAccessModifier// acceptable if enum is used? 
> And how should I name enum values? One enum value could be called //Never//, 
> but other must indicate that empty line is only inserted when access modifier 
> is after member function body or field/function/struct declaration.

That now depends on what you want (at least for me), if the name stays 
`EmptyLineBeforeAccessModifier` and it stays a `bool` it should nearly always 
add a space (most likely not directly after the `{`. I think the name would 
still fit for an enum with values like `Always`, `Never`, `DontModify`, and 
similar.

If you change the name it can stay a `bool`, but then needs an explanation with 
examples what the option really means.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1222-1223
+PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) &&
+RootToken.NewlinesBefore == 1)
+  ++Newlines;
+else if (!Style.EmptyLineBeforeAccessModifier &&

thezbyg wrote:
> MyDeveloperDay wrote:
> > HazardyKnusperkeks wrote:
> > > A tab?
> > My experience is that this doesn't mean a tab but just the what phabricator 
> > shows a change in whitespace
> > 
> > it is now 2 levels  of `if` scope indented not 1 so I think it should be 
> > moved over abit
> > 
> > 
> > 
> > 
> Yes, there is no tab in submitted patch, only 6 spaces.
> 
> Is this indented incorrectly?
> ```
> if (PreviousLine && RootToken.isAccessSpecifier()) {
>   if (Style.EmptyLineBeforeAccessModifier &&
>   PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) &&
>   RootToken.NewlinesBefore == 1)
> ++Newlines;
>   else if (!Style.EmptyLineBeforeAccessModifier &&
>RootToken.NewlinesBefore > 1)
> Newlines = 1;
> }
> ```
> I always run `git clang-format-11 HEAD~1` before generating patch file and 
> uploading it here.
I would assume so. I was just irritated by the >>. Everything is fine here.


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

https://reviews.llvm.org/D93846

[PATCH] D94019: Switching Clang UniqueInternalLinkageNamesPass scheduling to using the LLVM one with newpm.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D94019#2478431 , @aeubanks wrote:

> In D94019#2478423 , @dblaikie wrote:
>
>> In D94019#2478378 , @aeubanks wrote:
>>
>>> Oh sorry, yeah this isn't NFC.
>>>
>>> But I still don't think this needs a new test. We're going from a custom 
>>> Clang implementation of adding passes to something more generic that's also 
>>> more tested within LLVM.
>>> So IMO we just need an end to end test, which we already have in 
>>> unique-internal-linkage-names.cpp. The details are tested on the LLVM side.
>>
>> Then could the end to end test be tightened up to demonstrate how this 
>> patch/the change in pass order has produced new/different/desired behavior?
>
> It could be, but again I don't see the point. Rather than thinking about 
> testing within individual changes, I think thinking about the overall state 
> of testing regarding UniqueInternalLinkageNames after this change is better.
> The only Clang test we need is to make sure `PTO.UniqueLinkageNames` is 
> properly flipped, since that is the only thing Clang does regarding 
> UniqueInternalLinkageNames. We have that Clang test in 
> unique-internal-linkage-names.cpp (we could have a test that checks the PTO 
> if it were serialized as you've said, but unique-internal-linkage-names.cpp 
> is fine). It would fail if we didn't do `PTO.UniqueLinkageNames = 
> CodeGenOpts.UniqueInternalLinkageNames;`.

Or if the functionality were implemented as it were before this patch - by 
explicitly adding the pass to the MPM in Clang, rather than by using 
UniqueInternalLinkageNames - to demonstrate & validate that this change had the 
desired effect, there should be a test case.

Yeah, if this code had been written with the PTO flag from the start, I 
probably wouldn't've considered/suggested/required this test to be written - 
but given the code was written that way, and is now being changed, I think that 
change should be tested.

> Testing exactly what `PTO.UniqueLinkageNames` does is better done in LLVM. 
> And that has already been done.

General detailed testing of the functionality should be done in LLVM, I agree - 
but testing that this Clang change does what's intended should be done in Clang 
where the change is made. If MPM and other flags were serialized with IR, we 
would test it that way (if we had a perfect serialization boundary - we 
probably would've previously been testing the pass sequence via some serialized 
form of the pass pipeline, perhaps, then changed to testing a serialized form 
of the PTO values - in lieu of that, we can either test the pass pipeline dump 
or choose as robust an end-to-end IR example as possible that is resilient to 
as many other changes to the LLVM implementation as possible (as, I assume, the 
existing end-to-end test is) while demonstrating the purpose/value of this 
change)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94019

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

[PATCH] D93846: [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier

[Albertas Vyšniauskas via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1221
+if (Style.EmptyLineBeforeAccessModifier &&
+PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) &&
+RootToken.NewlinesBefore == 1)

HazardyKnusperkeks wrote:
> thezbyg wrote:
> > MyDeveloperDay wrote:
> > > maybe I don't understand the logic but why is this `r_brace` and 
> > > shouldn't we be looking at the "Last Non Comment" token?
> > I think that the logic is to only add empty line when access modifier is 
> > after member function body (`r_brace` indicates this) or declaration of 
> > field/function. If this check is changed to look at "last non comment" 
> > token, then empty line will be inserted in code like this:
> > ```
> > struct Foo {
> >   int i;
> >   //comment
> > private:
> >   int j;
> > }
> > ```
> But with the given Name it should add an empty line there! Otherwise you 
> should use an enum and not a bool to control wether a comment should suppress 
> the empty line or not. Also you should make the exception(s) clear with unit 
> tests.
Current clang-format behavior of access modifier separation was added in commit 
(fd433365e08da024ddc99dd0d7bce8e89d8c5469) and the test function is called 
//SeparatesLogicalBlocks//. So could we simply rename this new option to 
//SeparateLogicalBlocks// and leave the bool type? Option description would 
contain code examples and further explanation what logical block means. Setting 
//SeparateLogicalBlocks// option to false would disable empty line insertion, 
but would not remove existing empty lines.

Is option name //EmptyLineBeforeAccessModifier// acceptable if enum is used? 
And how should I name enum values? One enum value could be called //Never//, 
but other must indicate that empty line is only inserted when access modifier 
is after member function body or field/function/struct declaration.

I think that you incorrectly assumed from my previous comment, that only 
comments suppress empty line insertion. No empty line is inserted in all 
following code examples:
```
struct Foo {
private:
  int j;
};
```
```
struct Foo {
private:
public:
  int j;
};
```
```
struct Foo {
#ifdef FOO
private:
#endif
  int j;
};
```



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:1222-1223
+PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) &&
+RootToken.NewlinesBefore == 1)
+  ++Newlines;
+else if (!Style.EmptyLineBeforeAccessModifier &&

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > A tab?
> My experience is that this doesn't mean a tab but just the what phabricator 
> shows a change in whitespace
> 
> it is now 2 levels  of `if` scope indented not 1 so I think it should be 
> moved over abit
> 
> 
> 
> 
Yes, there is no tab in submitted patch, only 6 spaces.

Is this indented incorrectly?
```
if (PreviousLine && RootToken.isAccessSpecifier()) {
  if (Style.EmptyLineBeforeAccessModifier &&
  PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) &&
  RootToken.NewlinesBefore == 1)
++Newlines;
  else if (!Style.EmptyLineBeforeAccessModifier &&
   RootToken.NewlinesBefore > 1)
Newlines = 1;
}
```
I always run `git clang-format-11 HEAD~1` before generating patch file and 
uploading it here.



Comment at: clang/unittests/Format/FormatTest.cpp:8612-8625
+  verifyFormat("struct foo {\n"
+   "#ifdef FOO\n"
+   "private:\n"
+   "#endif\n"
+   "  int i;\n"
+   "  int j;\n"
+   "}\n",

HazardyKnusperkeks wrote:
> Just curios, any reason why the struct is repeated? I don't seem to notice a 
> difference.
> 
> And by the way, you are missing the `;` at the end of the definition, I don't 
> know if that affects clang-format.
Some of the tests have equal expected and code values. I will update them to 
single parameter `verifyFormat()`.

clang-format does not seem to care about missing `;`, but I will add them as 
all other tests have them.


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

https://reviews.llvm.org/D93846

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

[PATCH] D84673: [clang][cli] Port DiagnosticOpts to new option parsing system

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

dexonsmith added inline comments.



Comment at: llvm/utils/TableGen/OptParserEmitter.cpp:102-107
+  std::string getMacroName() const {
+if (KeyPath.startswith("DiagnosticOpts."))
+  return (Twine("DIAG_") + MarshallingInfo::MacroName).str();
+
+return MarshallingInfo::MacroName;
+  }

jansvoboda11 wrote:
> dexonsmith wrote:
> > This seems like a bit of a semantic layering violation. It'd be pretty 
> > unexpected if someone renamed `DiagnosticOpts` in clang that they'd have to 
> > update this code in llvm. Is there another way to solve this problem?
> I don't like it either, but the alternatives I can think of are worse.
> 
> We could add a `string MacroPrefix;` field to LLVM's `Option` class and 
> populate it in Clang's TableGen file:
> 1. Via something like an `IsDiag` multiclass that we'd need to remember to 
> apply to each diagnostic option. I don't like it as it seems error prone and 
> introduces duplication.
> 2. Put all diagnostic options into a single `let MacroPrefix = "DIAG_" in { 
> ... }` block. This removes the duplication, but doesn't ensure an option is 
> in that block iff it's a diagnostic option with `"DiagnosticOpts.*"` keypath.
> 3. More involved approach would be to duplicate the LLVM's `Option` and 
> related stuff in Clang. That would get us a place to put the custom 
> `KeyPath.startswith("DiagnosticOpts.")` logic and then forward to LLVM's 
> `Option` with the appropriate `MacroPrefix`.
> 
> I'll think some more about it.
Doing #1 + #2 seems like an okay tradeoff to me (looking back at the old diff, 
it seems like that's similar to what @dang originally implemented (except 
adding a more generic `MacroPrefix`), and it seems fairly clean / obvious to 
me).

> [...] but doesn't ensure an option is in that block iff it's a diagnostic 
> option with "DiagnosticOpts.*" keypath.

The reason I'm okay with this is that I'm having trouble envisioning how this 
would go wrong practice.
- If someone adds somethings to `DiagnosticOptions`, they're likely to grep for 
how the adjacent field was defined / marshalled, duplicate the line, and modify 
it. I'm not seeing a likely path for them to copy/paste from a non-diagnostic 
option and/or miss adding this to the `let` block.
- If someone accidentally adds something to the `let` block that isn't in 
`DiagnosticOptions`, they'll get a compiler error in `ParseDiagnosticArgs`.

If you're still concerned, I wonder if there's a way to add a check in asserts 
builds that confirms that `ParseDiagnosticArgs` fills in `DiagnosticOptions` 
equivalently to how `createFromCommandLine` does? (and/or could the latter call 
the former as an implementation strategy?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84673

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

[clang] f88a797 - [LoopDeletion] Allows deletion of possibly infinite side-effect free loops

[Atmn Patel via cfe-commits]

Author: Atmn Patel
Date: 2021-01-05T09:56:16-05:00
New Revision: f88a7975210fc995197af4b393e3bb5030e97a5c

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

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

>From C11 and C++11 onwards, a forward-progress requirement has been
introduced for both languages. In the case of C, loops with non-constant
conditionals that do not have any observable side-effects (as defined by
6.8.5p6) can be assumed by the implementation to terminate, and in the
case of C++, this assumption extends to all functions. The clang
frontend will emit the `mustprogress` function attribute for C++
functions (D86233, D85393, D86841) and emit the loop metadata
`llvm.loop.mustprogress` for every loop in C11 or later that has a
non-constant conditional.

This patch modifies LoopDeletion so that only loops with
the `llvm.loop.mustprogress` metadata or loops contained in functions
that are required to make progress (`mustprogress` or `willreturn`) are
checked for observable side-effects. If these loops do not have an
observable side-effect, then we delete them.

Loops without observable side-effects that do not satisfy the above
conditions will not be deleted.

Reviewed By: jdoerfert

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

Added: 
llvm/test/Transforms/LoopDeletion/mustprogress.ll

Modified: 
clang/test/Misc/loop-opt-setup.c
llvm/include/llvm/Transforms/Utils/LoopUtils.h
llvm/lib/Transforms/Scalar/LoopDeletion.cpp
llvm/lib/Transforms/Utils/LoopUtils.cpp
llvm/test/Transforms/LoopDeletion/no-exit-blocks.ll

Removed: 




diff  --git a/clang/test/Misc/loop-opt-setup.c 
b/clang/test/Misc/loop-opt-setup.c
index 9cea02a8bcb3..e9a9c1e8ae2d 100644
--- a/clang/test/Misc/loop-opt-setup.c
+++ b/clang/test/Misc/loop-opt-setup.c
@@ -1,5 +1,6 @@
-// This relies on %clang_cc1, %clang does not emit the block names in Release 
mode.
+// This tests loop unrolling and loop deletion (enabled under -O1)
 // RUN: %clang_cc1 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
+// RUN: %clang_cc1 -std=c99 -O1 -fno-unroll-loops -S -o - %s -emit-llvm | 
FileCheck %s --check-prefix C99
 
 extern int a[16];
 int b = 0;
@@ -25,7 +26,12 @@ void Helper() {
 }
 
 // Check br i1 to make sure the loop is gone, there will still be a label 
branch for the infinite loop.
+// In C99, there was no forward progress requirement, so we expect the 
infinite loop to still exist,
+// but for C11 and onwards, the infinite loop can be deleted.
 // CHECK-LABEL: Helper
-// CHECK: br label
-// CHECK-NOT: br i1
-// CHECK: br label
+// C99: br label
+// C99-NOT: br i1
+// C99: br label
+// CHECK: entry:
+// CHECK-NOT: br i1
+// CHECK-NEXT: ret void

diff  --git a/llvm/include/llvm/Transforms/Utils/LoopUtils.h 
b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
index d606fa954f95..80c6b09d9cf0 100644
--- a/llvm/include/llvm/Transforms/Utils/LoopUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/LoopUtils.h
@@ -255,6 +255,9 @@ bool hasDisableAllTransformsHint(const Loop *L);
 /// Look for the loop attribute that disables the LICM transformation 
heuristics.
 bool hasDisableLICMTransformsHint(const Loop *L);
 
+/// Look for the loop attribute that requires progress within the loop.
+bool hasMustProgress(const Loop *L);
+
 /// The mode sets how eager a transformation should be applied.
 enum TransformationMode {
   /// The pass can use heuristics to determine whether a transformation should

diff  --git a/llvm/lib/Transforms/Scalar/LoopDeletion.cpp 
b/llvm/lib/Transforms/Scalar/LoopDeletion.cpp
index 065db647561e..814cfc7ac6a9 100644
--- a/llvm/lib/Transforms/Scalar/LoopDeletion.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopDeletion.cpp
@@ -128,10 +128,11 @@ static bool isLoopNeverExecuted(Loop *L) {
 
 /// Remove a loop if it is dead.
 ///
-/// A loop is considered dead if it does not impact the observable behavior of
-/// the program other than finite running time. This never removes a loop that
-/// might be infinite (unless it is never executed), as doing so could change
-/// the halting/non-halting nature of a program.
+/// A loop is considered dead either if it does not impact the observable
+/// behavior of the program other than finite running time, or if it is
+/// required to make progress by an attribute such as 'mustprogress' or
+/// 'llvm.loop.mustprogress' and does not make any. This may remove
+/// infinite loops that have been required to make progress.
 ///
 /// This entire process relies pretty heavily on LoopSimplify form and LCSSA in
 /// order to make various safety checks work.
@@ -207,11 +208,13 @@ static LoopDeletionResult deleteLoopIfDead(Loop *L, 
DominatorTree ,
: LoopDeletionResult::Unmodified;

[PATCH] D93702: [clang][cli] NFC: Make marshalling macros reusable

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

dexonsmith added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3040
 
-#define OPTION_WITH_MARSHALLING(   
\
-PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,
\
-HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,  
\
-IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, 
\
-TABLE_INDEX)   
\
-  if ((FLAGS)::CC1Option) {
\
-this->KEYPATH = MERGER(this->KEYPATH, DEFAULT_VALUE);  
\
-if (IMPLIED_CHECK) 
\
-  this->KEYPATH = MERGER(this->KEYPATH, IMPLIED_VALUE);
\
-if (auto MaybeValue =  
\
-NORMALIZER(OPT_##ID, TABLE_INDEX, Args, Diags, Success))   
\
-  this->KEYPATH = MERGER(  
\
-  this->KEYPATH, static_castKEYPATH)>(*MaybeValue));   
\
-  }
-
+#define OPTION_WITH_MARSHALLING PARSE_OPTION_WITH_MARSHALLING
 #include "clang/Driver/Options.inc"

jansvoboda11 wrote:
> dexonsmith wrote:
> > One concern I have with this change is that the macro is using local 
> > variable names; it really would be better to have the macro content local 
> > to the function(s) where the variables are defined.
> > 
> > Can you give more context about why this has to be called from another 
> > place? Maybe there's another way to solve the problem.
> I've provided more context here: D93701.
> 
> We can solve the problem with implicit use of local variables inside the 
> macro by promoting them to macro parameters.
> This will make the forwarding call from `OPTION_WITH_MARSHALLING` to 
> `PARSE_OPTION_WITH_MARSHALLING` longer, as it will need to spell out all 
> parameters, but it would solve your concern I think.
I like that idea. That approach also allows you to drop unused parameters 
entirely in `PARSE_OPTIONS_WITH_MARSHALLING` (only forward the parameters that 
get used).



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3331-3332
+
+#undef GENERATE_OPTION_WITH_MARSHALLING
+#undef PARSE_OPTION_WITH_MARSHALLING

I like the idea of `#undef`'ing these macros to make their scope of use clear, 
but I'm not sure doing it at the end of file is adding a lot of value.

Is there a way of moving the call sites for each to be next to each other in 
the file, so you can `#define` and then `#undef` in a more limited scope? E.g.,
```
// maybe other content...

#define PARSE_OPTION_WITH_MARSHALLING(...) ...
void f1(...) { /* use PARSE_OPTION_WITH_MARSHALLING */ }
void f2(...) { /* use PARSE_OPTION_WITH_MARSHALLING */ }
#undef PARSE_OPTION_WITH_MARSHALLING

// maybe other content...

#define GENERATE_OPTION_WITH_MARSHALLING(...) ...
void f3(...) { /* use GENERATE_OPTION_WITH_MARSHALLING*/ }
void f4(...) { /* use GENERATE_OPTION_WITH_MARSHALLING*/ }
#undef GENERATE_OPTION_WITH_MARSHALLING

// maybe other content...
```
(If so, the code movement should be done in a separate NFC prep commit...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93702

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

[PATCH] D93701: [clang][cli] NFC: Make Diags optional in normalizer

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

dexonsmith added a comment.

In D93701#2477239 , @jansvoboda11 
wrote:

> This will make more sense after looking at D84673 
> . In short: parsing of diagnostic options 
> can happen outside of `CompilerInvocation::createFromArgs`, that's why 
> `ParseDiagnosticArgs` is a free function. Ideally, we'd like to reuse the 
> existing marshalling infrastructure here as well.
>
> `ParseDiagnosticArgs` is usually called with `DiagnosticEngine *Diags = 
> nullptr`. That's because the call happens early on during compiler execution, 
> where we don't have a diagnostic engine yet. This call needs to happen first, 
> to obtain the diagnostic options that are necessary for the engine 
> instantiation.
>
> This patch accommodates this free function by turning all `DiagnosticEngine` 
> references to pointers. Another solution would be to create a "dummy" 
> `DiagnosticsEngine` for the `ParseDiagnosticArgs` calls, but I didn't find a 
> way to do that.

Also, please try put this level of detail in the commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93701

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

[clang] 16f3401 - [Coverage] Fix test failures from commit rG9f2967bcfe2f

[Alan Phipps via cfe-commits]

Author: Alan Phipps
Date: 2021-01-05T13:35:52-06:00
New Revision: 16f3401eae4310c95163269c41d9b45261f0c7c3

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

LOG: [Coverage] Fix test failures from commit rG9f2967bcfe2f

Fix test failures with Branch Coverage tests from commit rG9f2967bcfe2f
that failed build on builder clang-x64-windows-msvc while building llvm:
http://lab.llvm.org:8011/#builders/123/builds/2155

Added: 


Modified: 
clang/test/CoverageMapping/branch-constfolded.cpp
clang/test/CoverageMapping/branch-macros.cpp
clang/test/CoverageMapping/branch-mincounters.cpp
clang/test/CoverageMapping/branch-templates.cpp
clang/test/Profile/branch-logical-mixed.cpp
clang/test/Profile/branch-profdup.cpp

Removed: 




diff  --git a/clang/test/CoverageMapping/branch-constfolded.cpp 
b/clang/test/CoverageMapping/branch-constfolded.cpp
index bb7c675e3ef7..5173286addbb 100644
--- a/clang/test/CoverageMapping/branch-constfolded.cpp
+++ b/clang/test/CoverageMapping/branch-constfolded.cpp
@@ -1,6 +1,6 @@
 // Test that branch regions are not generated for constant-folded conditions.
 
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping 
-dump-coverage-mapping -emit-llvm-only -main-file-name branch-constfolded.cpp 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -main-file-name branch-constfolded.cpp %s | FileCheck %s
 
 // CHECK-LABEL: _Z6fand_0b:
 bool fand_0(bool a) {

diff  --git a/clang/test/CoverageMapping/branch-macros.cpp 
b/clang/test/CoverageMapping/branch-macros.cpp
index d5bc47fbef76..0f9ae791a355 100644
--- a/clang/test/CoverageMapping/branch-macros.cpp
+++ b/clang/test/CoverageMapping/branch-macros.cpp
@@ -1,7 +1,7 @@
 // Test that branch regions are generated for conditions in nested macro
 // expansions.
 
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping 
-dump-coverage-mapping -emit-llvm-only -main-file-name branch-macros.cpp %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -main-file-name branch-macros.cpp %s | FileCheck %s
 
 #define COND1 (a == b)
 #define COND2 (a != b)

diff  --git a/clang/test/CoverageMapping/branch-mincounters.cpp 
b/clang/test/CoverageMapping/branch-mincounters.cpp
index 68e691684970..6d4341cbeafd 100644
--- a/clang/test/CoverageMapping/branch-mincounters.cpp
+++ b/clang/test/CoverageMapping/branch-mincounters.cpp
@@ -1,7 +1,7 @@
 // Test to ensure right number of counters are allocated and used for nested
 // logical operators on branch conditions for branch coverage.
 
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping 
-dump-coverage-mapping -emit-llvm-only -main-file-name branch-logical-mixed.cpp 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -main-file-name branch-logical-mixed.cpp %s | FileCheck %s
 
 
 // CHECK-LABEL: _Z5func1ii:

diff  --git a/clang/test/CoverageMapping/branch-templates.cpp 
b/clang/test/CoverageMapping/branch-templates.cpp
index 9e312df9b2de..1fd01218c903 100644
--- a/clang/test/CoverageMapping/branch-templates.cpp
+++ b/clang/test/CoverageMapping/branch-templates.cpp
@@ -1,7 +1,7 @@
 // Test that branch regions are generated for conditions in function template
 // instantiations.
 
-// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping 
-dump-coverage-mapping -emit-llvm-only -main-file-name branch-templates.cpp %s 
| FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 
-fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -main-file-name branch-templates.cpp %s | FileCheck %s
 
 template
 void unused(T x) {

diff  --git a/clang/test/Profile/branch-logical-mixed.cpp 
b/clang/test/Profile/branch-logical-mixed.cpp
index e6f546d3ac4b..cbfcf061f152 100644
--- a/clang/test/Profile/branch-logical-mixed.cpp
+++ b/clang/test/Profile/branch-logical-mixed.cpp
@@ -1,7 +1,7 @@
 // Test to ensure instrumentation of logical operator RHS True/False counters
 // are being instrumented for branch coverage
 
-// RUN: %clang_cc1 -main-file-name branch-logical-mixed.cpp %s -o - -emit-llvm 
-fprofile-instrument=clang | FileCheck -allow-deprecated-dag-overlap %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++11 -main-file-name 
branch-logical-mixed.cpp %s -o - -emit-llvm -fprofile-instrument=clang | 
FileCheck -allow-deprecated-dag-overlap %s
 
 
 // CHECK: @[[FUNC:__profc__Z4funcv]] = private global [61 x i64] 
zeroinitializer

diff  --git 

[PATCH] D93883: [WebAssembly] Prototype prefetch instructions

[Thomas Lively 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 rG497026c90233: [WebAssembly] Prototype prefetch instructions 
(authored by tlively).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93883

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-prefetch-offset.ll
  llvm/test/MC/WebAssembly/simd-encodings.s

Index: llvm/test/MC/WebAssembly/simd-encodings.s
===
--- llvm/test/MC/WebAssembly/simd-encodings.s
+++ llvm/test/MC/WebAssembly/simd-encodings.s
@@ -736,4 +736,10 @@
 # CHECK: i32x4.extadd_pairwise_i16x8_u # encoding: [0xfd,0xa6,0x01]
 i32x4.extadd_pairwise_i16x8_u
 
+# CHECK: prefetch.t 16 # encoding: [0xfd,0xc5,0x01,0x00,0x10]
+prefetch.t 16
+
+# CHECK: prefetch.nt 16 # encoding: [0xfd,0xc6,0x01,0x00,0x10]
+prefetch.nt 16
+
 end_function
Index: llvm/test/CodeGen/WebAssembly/simd-prefetch-offset.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/simd-prefetch-offset.ll
@@ -0,0 +1,235 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -verify-machineinstrs -mattr=+simd128 | FileCheck %s
+
+; Test experimental prefetch instructions
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+declare void @llvm.wasm.prefetch.t(i8*)
+declare void @llvm.wasm.prefetch.nt(i8*)
+@gv = global i8 0
+
+;===
+; prefetch.t
+;===
+
+define void @prefetch_t_no_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_no_offset:
+; CHECK: .functype prefetch_t_no_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  tail call void @llvm.wasm.prefetch.t(i8* %p)
+  ret void
+}
+
+define void @prefetch_t_with_folded_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_with_folded_offset:
+; CHECK: .functype prefetch_t_with_folded_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:i32.const 24
+; CHECK-NEXT:i32.add
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  %q = ptrtoint i8* %p to i32
+  %r = add nuw i32 %q, 24
+  %s = inttoptr i32 %r to i8*
+  tail call void @llvm.wasm.prefetch.t(i8* %s)
+  ret void
+}
+
+define void @prefetch_t_with_folded_gep_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_with_folded_gep_offset:
+; CHECK: .functype prefetch_t_with_folded_gep_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:i32.const 6
+; CHECK-NEXT:i32.add
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  %s = getelementptr inbounds i8, i8* %p, i32 6
+  tail call void @llvm.wasm.prefetch.t(i8* %s)
+  ret void
+}
+
+define void @prefetch_t_with_unfolded_gep_negative_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_with_unfolded_gep_negative_offset:
+; CHECK: .functype prefetch_t_with_unfolded_gep_negative_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:i32.const -6
+; CHECK-NEXT:i32.add
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  %s = getelementptr inbounds i8, i8* %p, i32 -6
+  tail call void @llvm.wasm.prefetch.t(i8* %s)
+  ret void
+}
+
+define void @prefetch_t_with_unfolded_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_with_unfolded_offset:
+; CHECK: .functype prefetch_t_with_unfolded_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:i32.const 24
+; CHECK-NEXT:i32.add
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  %q = ptrtoint i8* %p to i32
+  %r = add nsw i32 %q, 24
+  %s = inttoptr i32 %r to i8*
+  tail call void @llvm.wasm.prefetch.t(i8* %s)
+  ret void
+}
+
+define void @prefetch_t_with_unfolded_gep_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_with_unfolded_gep_offset:
+; CHECK: .functype prefetch_t_with_unfolded_gep_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:i32.const 6
+; CHECK-NEXT:i32.add
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  %s = getelementptr i8, i8* %p, i32 6
+  tail 

[clang] 497026c - [WebAssembly] Prototype prefetch instructions

[Thomas Lively via cfe-commits]

Author: Thomas Lively
Date: 2021-01-05T11:32:03-08:00
New Revision: 497026c90233e82ffd3ce2438c5f9567be6dabe7

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

LOG: [WebAssembly] Prototype prefetch instructions

As proposed in https://github.com/WebAssembly/simd/pull/352 and using the
opcodes used in the V8 prototype:
https://chromium-review.googlesource.com/c/v8/v8/+/2543167. These instructions
are only usable via intrinsics and clang builtins to make them opt-in while they
are being benchmarked.

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

Added: 
llvm/test/CodeGen/WebAssembly/simd-prefetch-offset.ll

Modified: 
clang/include/clang/Basic/BuiltinsWebAssembly.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/builtins-wasm.c
llvm/include/llvm/IR/IntrinsicsWebAssembly.td
llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
llvm/test/MC/WebAssembly/simd-encodings.s

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsWebAssembly.def 
b/clang/include/clang/Basic/BuiltinsWebAssembly.def
index d6860e0b13be..84482082095e 100644
--- a/clang/include/clang/Basic/BuiltinsWebAssembly.def
+++ b/clang/include/clang/Basic/BuiltinsWebAssembly.def
@@ -220,5 +220,8 @@ TARGET_BUILTIN(__builtin_wasm_store64_lane, "vLLi*V2LLiIi", 
"n", "simd128")
 
 TARGET_BUILTIN(__builtin_wasm_eq_i64x2, "V2LLiV2LLiV2LLi", "nc", "simd128")
 
+TARGET_BUILTIN(__builtin_wasm_prefetch_t, "vv*", "n", "simd128")
+TARGET_BUILTIN(__builtin_wasm_prefetch_nt, "vv*", "n", "simd128")
+
 #undef BUILTIN
 #undef TARGET_BUILTIN

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 6e98af407a9a..1e0337ca7ac3 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -17171,6 +17171,16 @@ Value 
*CodeGenFunction::EmitWebAssemblyBuiltinExpr(unsigned BuiltinID,
 Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_shuffle);
 return Builder.CreateCall(Callee, Ops);
   }
+  case WebAssembly::BI__builtin_wasm_prefetch_t: {
+Value *Ptr = EmitScalarExpr(E->getArg(0));
+Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_prefetch_t);
+return Builder.CreateCall(Callee, Ptr);
+  }
+  case WebAssembly::BI__builtin_wasm_prefetch_nt: {
+Value *Ptr = EmitScalarExpr(E->getArg(0));
+Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_prefetch_nt);
+return Builder.CreateCall(Callee, Ptr);
+  }
   default:
 return nullptr;
   }

diff  --git a/clang/test/CodeGen/builtins-wasm.c 
b/clang/test/CodeGen/builtins-wasm.c
index a07c278c33af..83924b48542e 100644
--- a/clang/test/CodeGen/builtins-wasm.c
+++ b/clang/test/CodeGen/builtins-wasm.c
@@ -1002,3 +1002,13 @@ i8x16 shuffle(i8x16 x, i8x16 y) {
   // WEBASSEMBLY-SAME: i32 15
   // WEBASSEMBLY-NEXT: ret
 }
+
+void prefetch_t(void *p) {
+  return __builtin_wasm_prefetch_t(p);
+  // WEBASSEMBLY: call void @llvm.wasm.prefetch.t(i8* %p)
+}
+
+void prefetch_nt(void *p) {
+  return __builtin_wasm_prefetch_nt(p);
+  // WEBASSEMBLY: call void @llvm.wasm.prefetch.nt(i8* %p)
+}

diff  --git a/llvm/include/llvm/IR/IntrinsicsWebAssembly.td 
b/llvm/include/llvm/IR/IntrinsicsWebAssembly.td
index d9a6aa78fdcd..e87700ab0fcb 100644
--- a/llvm/include/llvm/IR/IntrinsicsWebAssembly.td
+++ b/llvm/include/llvm/IR/IntrinsicsWebAssembly.td
@@ -311,6 +311,20 @@ def int_wasm_eq :
 [llvm_v2i64_ty, llvm_v2i64_ty],
 [IntrNoMem, IntrSpeculatable]>;
 
+// TODO: Remove this after experiments have been run. Use the target-agnostic
+// int_prefetch if this becomes specified at some point.
+def int_wasm_prefetch_t :
+  Intrinsic<[], [llvm_ptr_ty],
+[IntrInaccessibleMemOrArgMemOnly, IntrWillReturn,
+ ReadOnly>, NoCapture>],
+"", [SDNPMemOperand]>;
+
+def int_wasm_prefetch_nt :
+  Intrinsic<[], [llvm_ptr_ty],
+[IntrInaccessibleMemOrArgMemOnly, IntrWillReturn,
+ ReadOnly>, NoCapture>],
+"", [SDNPMemOperand]>;
+
 
//===--===//
 // Thread-local storage intrinsics
 
//===--===//

diff  --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp 
b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index 18d7b642e044..cd07a142147c 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -427,7 +427,8 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser 
{
   bool 

[PATCH] D93701: [clang][cli] NFC: Make Diags optional in normalizer

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

dexonsmith added a comment.

In D93701#2479574 , @jansvoboda11 
wrote:

> In D93701#2477239 , @jansvoboda11 
> wrote:
>
>> Another solution would be to create a "dummy" `DiagnosticsEngine` for the 
>> `ParseDiagnosticArgs` calls, but I didn't find a way to do that.
>
> Something like `CompilerInstance::createDiagnostics(new DiagnosticOptions(), 
> new IgnoreDiagnostics())` could work. I'll give that a try.

I think you can also construct a `DiagnosticsEngine` directly on the stack, 
without going through `CompilerInstance`, if that helps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93701

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

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

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

Now that the test is fixed, this could be re-landed, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86844

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

[PATCH] D94101: [clang][cli] Specify correct integer width for -fbuild-session-timestamp

[Jan Svoboda 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 rGf111cf992df4: [clang][cli] Specify correct integer width for 
-fbuild-session-timestamp (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94101

Files:
  clang/include/clang/Driver/Options.td
  clang/unittests/Frontend/CompilerInvocationTest.cpp


Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -368,6 +368,18 @@
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("pic";
 }
 
+// Wide integer option.
+
+TEST_F(CommandLineTest, WideIntegerHighValue) {
+  const char *Args[] = {"-fbuild-session-timestamp=1609827494445723662"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getHeaderSearchOpts().BuildSessionTimestamp,
+1609827494445723662ull);
+}
+
 // Tree of boolean options that can be (directly or transitively) implied by
 // their parent:
 //
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1767,7 +1767,7 @@
 def fbuild_session_timestamp : Joined<["-"], "fbuild-session-timestamp=">,
   Group, Flags<[CC1Option]>, MetaVarName<"">,
   HelpText<"Time when the current build session started">,
-  MarshallingInfoStringInt<"HeaderSearchOpts->BuildSessionTimestamp">;
+  MarshallingInfoStringInt<"HeaderSearchOpts->BuildSessionTimestamp", "0", 
"uint64_t">;
 def fbuild_session_file : Joined<["-"], "fbuild-session-file=">,
   Group, MetaVarName<"">,
   HelpText<"Use the last modification time of  as the build session 
timestamp">;


Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -368,6 +368,18 @@
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("pic";
 }
 
+// Wide integer option.
+
+TEST_F(CommandLineTest, WideIntegerHighValue) {
+  const char *Args[] = {"-fbuild-session-timestamp=1609827494445723662"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getHeaderSearchOpts().BuildSessionTimestamp,
+1609827494445723662ull);
+}
+
 // Tree of boolean options that can be (directly or transitively) implied by
 // their parent:
 //
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1767,7 +1767,7 @@
 def fbuild_session_timestamp : Joined<["-"], "fbuild-session-timestamp=">,
   Group, Flags<[CC1Option]>, MetaVarName<"">,
   HelpText<"Time when the current build session started">,
-  MarshallingInfoStringInt<"HeaderSearchOpts->BuildSessionTimestamp">;
+  MarshallingInfoStringInt<"HeaderSearchOpts->BuildSessionTimestamp", "0", "uint64_t">;
 def fbuild_session_file : Joined<["-"], "fbuild-session-file=">,
   Group, MetaVarName<"">,
   HelpText<"Use the last modification time of  as the build session timestamp">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f111cf9 - [clang][cli] Specify correct integer width for -fbuild-session-timestamp

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-01-05T20:10:07+01:00
New Revision: f111cf992df4ec00acfdd026eac12b36c3831999

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

LOG: [clang][cli] Specify correct integer width for -fbuild-session-timestamp

This fixes an issue where large integer values were rejected as invalid.

Reviewed By: arphaman

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/unittests/Frontend/CompilerInvocationTest.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 9a851f63a663..c7da888968a8 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1767,7 +1767,7 @@ def fmodules_search_all : Flag <["-"], 
"fmodules-search-all">, Group,
 def fbuild_session_timestamp : Joined<["-"], "fbuild-session-timestamp=">,
   Group, Flags<[CC1Option]>, MetaVarName<"">,
   HelpText<"Time when the current build session started">,
-  MarshallingInfoStringInt<"HeaderSearchOpts->BuildSessionTimestamp">;
+  MarshallingInfoStringInt<"HeaderSearchOpts->BuildSessionTimestamp", "0", 
"uint64_t">;
 def fbuild_session_file : Joined<["-"], "fbuild-session-file=">,
   Group, MetaVarName<"">,
   HelpText<"Use the last modification time of  as the build session 
timestamp">;

diff  --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp 
b/clang/unittests/Frontend/CompilerInvocationTest.cpp
index 51b7ba8c147f..83ae169f9929 100644
--- a/clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -368,6 +368,18 @@ TEST_F(CommandLineTest, 
CanGenerateCC1COmmandLineSeparateEnumDefault) {
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("pic";
 }
 
+// Wide integer option.
+
+TEST_F(CommandLineTest, WideIntegerHighValue) {
+  const char *Args[] = {"-fbuild-session-timestamp=1609827494445723662"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getHeaderSearchOpts().BuildSessionTimestamp,
+1609827494445723662ull);
+}
+
 // Tree of boolean options that can be (directly or transitively) implied by
 // their parent:
 //



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

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D93630#2479757 , @aaron.ballman 
wrote:

> In D93630#2479343 , @vsavchenko 
> wrote:
>
>> In D93630#2479260 , @aaron.ballman 
>> wrote:
>>
>>> In D93630#2478977 , @vsavchenko 
>>> wrote:
>>>
 I guess I want to clarify one point here, after this patch the parser 
 **will not assume** that statement always follows statement attributes.  
 We simply turn off the assumption that what follows is a declaration, 
 parser will simply determine whether it is a statement or a declaration, 
 it can do it on its own without attribute's "help".
>>>
>>> We used to say that if there's a GNU attribute (or we're parsing a 
>>> declaration statement), what follows must be a declaration. Now we're using 
>>> the attributes in the given attribute list to decide whether what follows 
>>> is a declaration or a statement (if all of the attributes are statement 
>>> attributes, we don't treat what follows as being a declaration unless we're 
>>> in a declaration statement). Or am I misreading the code?
>>
>> We do not decide whether it is declaration or statement.  I do get that it 
>> is a matter of perspective, but right now I prefer to read it like this:
>> **BEFORE**: if we've seen a GNU-style attribute parse whatever comes next as 
>> a declaration
>> **AFTER**: if we've seen a GNU-style attribute except for the case when all 
>> of the parsed attributes are known to be statement attributes, parse 
>> whatever comes next as a declaration
>>
>> So, essentially, when we see statement attributes only, 
>> attribute/declaration heuristic is canceled and we parse exactly the way we 
>> would've parsed as if no attributes present.
>
> I think this is a defensible design, but it still has the potential to change 
> the parsing behavior in some circumstances and I want to make sure those 
> circumstances are ones we know about. Note, I still think we should 
> coordinate this change with the GCC folks. GCC has a lot of attributes that 
> Clang does not have, so my concern with GCC is that Clang supports writing 
> attributes in this way and GCC can't support it for some reason and it causes 
> issues for an attribute we want to share between implementations. I don't 
> have a specific case in mind that may cause an issue but they have a lot of 
> attributes I'm unfamiliar with.

This is where we also pretty defensive and conservative, if we see any unknown 
attributes, we stick with the "attribute -> declaration" rule.

>>> e.g.,
>>>
>>>   void foo(void) {
>>> __attribute__((fallthrough)) i = 12;
>>>   }
>>>
>>> Before patch:
>>>
>>>   F:\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only 
>>> "F:\Aaron Ballman\Desktop\test.c"
>>>   F:\Aaron Ballman\Desktop\test.c:2:32: warning: type specifier missing, 
>>> defaults to 'int' [-Wimplicit-int]
>>> __attribute__((fallthrough)) i = 12;
>>>  ^
>>>   F:\Aaron Ballman\Desktop\test.c:2:18: error: 'fallthrough' attribute 
>>> cannot be applied to a declaration
>>> __attribute__((fallthrough)) i = 12;
>>>^ ~
>>>   1 warning and 1 error generated.
>>>
>>> After patch:
>>>
>>>   F:\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only 
>>> "F:\Aaron Ballman\Desktop\test.c"
>>>   F:\Aaron Ballman\Desktop\test.c:2:32: error: use of undeclared identifier 
>>> 'i'
>>> __attribute__((fallthrough)) i = 12;
>>>  ^
>>>   1 error generated.
>>
>> In both situations, the code contains errors.  I don't think that this is a 
>> valid argument, to be honest.  Actually, the nature of this change is that 
>> it starts to parse strictly MORE cases.
>
> My concerns are if there are cases where we would start to reject valid code 
> based on the presence of a GNU statement attribute or where we'd regress 
> diagnostics. This seemed like a case where we were regressing diagnostics and 
> could potentially reject otherwise valid code (using a different attribute 
> that wouldn't generate an error in the way `fallthrough` does). We previously 
> were saying "aha, this is a declaration and the attribute cannot apply to it" 
> which is strictly better than "aha, I have no idea what this identifier is 
> doing here". However, I'm changing my opinion here -- see below.
>
>>> So I think this will cause issues for the suppression attribute you're 
>>> working on if we allow it to have a GNU spelling with valid C89 code like; 
>>> `__attribute__((suppress)) i = 12;`
>>
>> This is **not** a valid code if you remove `__attribute__((suppress))`. `i = 
>> 12` without a prior declaration of `i` will cause the `use of undeclared 
>> identifier 'i'` error.  You can try it with any compiler: 
>> https://godbolt.org/z/P43nxn.
>
> Thank you for pointing out that my test case was bad 

[PATCH] D94101: [clang][cli] Specify correct integer width for -fbuild-session-timestamp

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added a reviewer: arphaman.
Herald added a subscriber: dang.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes an issue where large integer values were rejected as invalid.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94101

Files:
  clang/include/clang/Driver/Options.td
  clang/unittests/Frontend/CompilerInvocationTest.cpp


Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -368,6 +368,18 @@
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("pic";
 }
 
+// Wide integer option.
+
+TEST_F(CommandLineTest, WideIntegerHighValue) {
+  const char *Args[] = {"-fbuild-session-timestamp=1609827494445723662"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getHeaderSearchOpts().BuildSessionTimestamp,
+1609827494445723662ull);
+}
+
 // Tree of boolean options that can be (directly or transitively) implied by
 // their parent:
 //
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1767,7 +1767,7 @@
 def fbuild_session_timestamp : Joined<["-"], "fbuild-session-timestamp=">,
   Group, Flags<[CC1Option]>, MetaVarName<"">,
   HelpText<"Time when the current build session started">,
-  MarshallingInfoStringInt<"HeaderSearchOpts->BuildSessionTimestamp">;
+  MarshallingInfoStringInt<"HeaderSearchOpts->BuildSessionTimestamp", "0", 
"uint64_t">;
 def fbuild_session_file : Joined<["-"], "fbuild-session-file=">,
   Group, MetaVarName<"">,
   HelpText<"Use the last modification time of  as the build session 
timestamp">;


Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -368,6 +368,18 @@
   ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("pic";
 }
 
+// Wide integer option.
+
+TEST_F(CommandLineTest, WideIntegerHighValue) {
+  const char *Args[] = {"-fbuild-session-timestamp=1609827494445723662"};
+
+  CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+  ASSERT_FALSE(Diags->hasErrorOccurred());
+  ASSERT_EQ(Invocation.getHeaderSearchOpts().BuildSessionTimestamp,
+1609827494445723662ull);
+}
+
 // Tree of boolean options that can be (directly or transitively) implied by
 // their parent:
 //
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1767,7 +1767,7 @@
 def fbuild_session_timestamp : Joined<["-"], "fbuild-session-timestamp=">,
   Group, Flags<[CC1Option]>, MetaVarName<"">,
   HelpText<"Time when the current build session started">,
-  MarshallingInfoStringInt<"HeaderSearchOpts->BuildSessionTimestamp">;
+  MarshallingInfoStringInt<"HeaderSearchOpts->BuildSessionTimestamp", "0", "uint64_t">;
 def fbuild_session_file : Joined<["-"], "fbuild-session-file=">,
   Group, MetaVarName<"">,
   HelpText<"Use the last modification time of  as the build session timestamp">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94039: [WebAssembly] Update WasmEHPrepare for the new spec

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In the description I think "but LLVM does not have a way of that kind of 
behavior" is missing the word "modeling" => "but LLVM does not have a way of 
modeling that kind of behavior"




Comment at: llvm/lib/CodeGen/WasmEHPrepare.cpp:374-375
+  // be lowered to wasm 'catch' instruction. We do this mainly because
+  // instruction selection cannot handle wasm.get.exception intrinsic's token
+  // argument.
+  Instruction *CatchCI =

What is the token argument used for? Could clang generate `llvm.wasm.catch` 
directly?



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td:166
 let isCodeGenOnly = 1, hasSideEffects = 1 in
-defm EXTRACT_EXCEPTION_I32 : NRI<(outs I32:$dst), (ins),
- [(set I32:$dst, 
(int_wasm_extract_exception))],
+defm EXTRACT_EXCEPTION_I32 : NRI<(outs I32:$dst), (ins), [],
  "extract_exception\t$dst">;

Why do we need to keep this instruction definition? Is it removed in a later 
diff in this stack?



Comment at: llvm/test/CodeGen/WebAssembly/wasmehprepare.ll:611
 declare void @__clang_call_terminate(i8*)
+declare void @_ZSt9terminatev()
 

What is this addition for?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94039

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

[PATCH] D94031: [ASTMatchers] Fix child traversal over range-for loops

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:245
+ScopedIncrement ScopedDepth();
+if (!match(*Node->getLoopVariable()) || !match(*Node->getRangeInit()) ||
+!match(*Node->getBody()))

aaron.ballman wrote:
> steveire wrote:
> > aaron.ballman wrote:
> > > Should we be traversing the init statement before the loop variable so 
> > > that the traversals happen in lexical order?
> > Do you mean that in 
> > 
> > ```
> > for (auto i : arr)
> > {
> > }
> > ```
> > 
> > to visit the `arr` before the `auto i`? 
> > 
> > I think visiting the `auto i` before the `arr` makes sense.
> Nope, I mean that in:
> ```
> for (int i = 12; auto j : {1, 2, 3, 4}) {}
> ```
> we should visit the `int i = 12;` before the `auto j`
That was actually missing entirely. Added now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94031

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

[PATCH] D94031: [ASTMatchers] Fix child traversal over range-for loops

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 314654.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94031

Files:
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2553,7 +2553,9 @@
 int arr[2];
 for (auto i : arr)
 {
-
+  if (true)
+  {
+  }
 }
   }
   )cpp";
@@ -2596,6 +2598,33 @@
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
 EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
+  {
+auto M = cxxForRangeStmt(hasDescendant(ifStmt()));
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+EXPECT_TRUE(matches(
+Code, traverse(TK_AsIs, cxxForRangeStmt(has(declStmt(
+hasSingleDecl(varDecl(hasName("i");
+EXPECT_TRUE(
+matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+   cxxForRangeStmt(has(varDecl(hasName("i")));
+  }
+  {
+EXPECT_TRUE(matches(
+Code, traverse(TK_AsIs, cxxForRangeStmt(has(declStmt(hasSingleDecl(
+varDecl(hasInitializer(declRefExpr(
+to(varDecl(hasName("arr");
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
+   cxxForRangeStmt(has(declRefExpr(
+   to(varDecl(hasName("arr");
+  }
+  {
+auto M = cxxForRangeStmt(has(compoundStmt()));
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
   {
 auto M = binaryOperator(hasOperatorName("!="));
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
@@ -2659,7 +2688,8 @@
  true, {"-std=c++20"}));
   }
   {
-auto M = cxxForRangeStmt(has(declStmt()));
+auto M =
+cxxForRangeStmt(has(declStmt(hasSingleDecl(varDecl(hasName("i"));
 EXPECT_TRUE(
 matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
 EXPECT_FALSE(
@@ -2679,6 +2709,19 @@
 matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
  true, {"-std=c++20"}));
   }
+  {
+auto M = cxxForRangeStmt(
+has(declStmt(hasSingleDecl(varDecl(
+hasName("a"),
+hasInitializer(declRefExpr(to(varDecl(hasName("arr"),
+hasLoopVariable(varDecl(hasName("i"))),
+hasRangeInit(declRefExpr(to(varDecl(hasName("a"));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_AsIs, M), true, {"-std=c++20"}));
+EXPECT_TRUE(
+matchesConditionally(Code, traverse(TK_IgnoreUnlessSpelledInSource, M),
+ true, {"-std=c++20"}));
+  }
   Code = R"cpp(
 void hasDefaultArg(int i, int j = 0)
 {
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -236,6 +236,20 @@
 ScopedIncrement ScopedDepth();
 return traverse(TAL);
   }
+  bool TraverseCXXForRangeStmt(CXXForRangeStmt *Node) {
+if (!Finder->isTraversalIgnoringImplicitNodes())
+  return VisitorBase::TraverseCXXForRangeStmt(Node);
+if (!Node)
+  return true;
+ScopedIncrement ScopedDepth();
+if (auto *Init = Node->getInit())
+  if (!match(*Init))
+return false;
+if (!match(*Node->getLoopVariable()) || !match(*Node->getRangeInit()) ||
+!match(*Node->getBody()))
+  return false;
+return VisitorBase::TraverseStmt(Node->getBody());
+  }
   bool TraverseLambdaExpr(LambdaExpr *Node) {
 if (!Finder->isTraversalIgnoringImplicitNodes())
   return VisitorBase::TraverseLambdaExpr(Node);
@@ -575,8 +589,6 @@
 
 if (isTraversalIgnoringImplicitNodes()) {
   IgnoreImplicitChildren = true;
-  if (Node.get())
-ScopedTraversal = true;
 }
 
 ASTNodeNotSpelledInSourceScope RAII(this, ScopedTraversal);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94038: [WebAssembly] Rename wasm_rethrow_in_catch intrinsic/builtin

[Thomas Lively via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:53
 // throw / rethrow
+// The immediate argument is an index to for a tag, which is 0 for C++.
 def int_wasm_throw : Intrinsic<[], [llvm_i32_ty, llvm_ptr_ty],

Extra word


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94038

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

[PATCH] D94031: [ASTMatchers] Fix child traversal over range-for loops

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:245
+ScopedIncrement ScopedDepth();
+if (!match(*Node->getLoopVariable()) || !match(*Node->getRangeInit()) ||
+!match(*Node->getBody()))

steveire wrote:
> aaron.ballman wrote:
> > Should we be traversing the init statement before the loop variable so that 
> > the traversals happen in lexical order?
> Do you mean that in 
> 
> ```
> for (auto i : arr)
> {
> }
> ```
> 
> to visit the `arr` before the `auto i`? 
> 
> I think visiting the `auto i` before the `arr` makes sense.
Nope, I mean that in:
```
for (int i = 12; auto j : {1, 2, 3, 4}) {}
```
we should visit the `int i = 12;` before the `auto j`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94031

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

[PATCH] D93883: [WebAssembly] Prototype prefetch instructions

[Thomas Lively via Phabricator via cfe-commits]

tlively added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:318
+  Intrinsic<[], [llvm_ptr_ty],
+[IntrInaccessibleMemOrArgMemOnly, IntrWillReturn,
+ ReadOnly>, NoCapture>],

aheejin wrote:
> It [[ 
> https://github.com/llvm/llvm-project/blob/5abfeccf10bcbc0d673ece21ddd8d4ac4a0e7594/llvm/include/llvm/IR/Intrinsics.td#L130
>  | looks ]] `IntrWillReturn` is applied by default?
It looks like this is only true for Intrinsics that inherit 
`DefaultAttrsIntrinsic`. Otherwise the default-enabled attributes are 
confusingly disabled by default 
https://github.com/llvm/llvm-project/blob/5abfeccf10bcbc0d673ece21ddd8d4ac4a0e7594/llvm/include/llvm/IR/Intrinsics.td#L346.
 As a separate change it would be a good idea to change most of our target 
intrinsics to use `DefaultAttrsIntrinsics`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93883

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

[PATCH] D93883: [WebAssembly] Prototype prefetch instructions

[Thomas Lively via Phabricator via cfe-commits]

tlively updated this revision to Diff 314647.
tlively marked 3 inline comments as done.
tlively added a comment.

- Fix section headers in test file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93883

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-prefetch-offset.ll
  llvm/test/MC/WebAssembly/simd-encodings.s

Index: llvm/test/MC/WebAssembly/simd-encodings.s
===
--- llvm/test/MC/WebAssembly/simd-encodings.s
+++ llvm/test/MC/WebAssembly/simd-encodings.s
@@ -736,4 +736,10 @@
 # CHECK: i32x4.extadd_pairwise_i16x8_u # encoding: [0xfd,0xa6,0x01]
 i32x4.extadd_pairwise_i16x8_u
 
+# CHECK: prefetch.t 16 # encoding: [0xfd,0xc5,0x01,0x00,0x10]
+prefetch.t 16
+
+# CHECK: prefetch.nt 16 # encoding: [0xfd,0xc6,0x01,0x00,0x10]
+prefetch.nt 16
+
 end_function
Index: llvm/test/CodeGen/WebAssembly/simd-prefetch-offset.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/simd-prefetch-offset.ll
@@ -0,0 +1,235 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -verify-machineinstrs -mattr=+simd128 | FileCheck %s
+
+; Test experimental prefetch instructions
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+declare void @llvm.wasm.prefetch.t(i8*)
+declare void @llvm.wasm.prefetch.nt(i8*)
+@gv = global i8 0
+
+;===
+; prefetch.t
+;===
+
+define void @prefetch_t_no_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_no_offset:
+; CHECK: .functype prefetch_t_no_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  tail call void @llvm.wasm.prefetch.t(i8* %p)
+  ret void
+}
+
+define void @prefetch_t_with_folded_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_with_folded_offset:
+; CHECK: .functype prefetch_t_with_folded_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:i32.const 24
+; CHECK-NEXT:i32.add
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  %q = ptrtoint i8* %p to i32
+  %r = add nuw i32 %q, 24
+  %s = inttoptr i32 %r to i8*
+  tail call void @llvm.wasm.prefetch.t(i8* %s)
+  ret void
+}
+
+define void @prefetch_t_with_folded_gep_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_with_folded_gep_offset:
+; CHECK: .functype prefetch_t_with_folded_gep_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:i32.const 6
+; CHECK-NEXT:i32.add
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  %s = getelementptr inbounds i8, i8* %p, i32 6
+  tail call void @llvm.wasm.prefetch.t(i8* %s)
+  ret void
+}
+
+define void @prefetch_t_with_unfolded_gep_negative_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_with_unfolded_gep_negative_offset:
+; CHECK: .functype prefetch_t_with_unfolded_gep_negative_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:i32.const -6
+; CHECK-NEXT:i32.add
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  %s = getelementptr inbounds i8, i8* %p, i32 -6
+  tail call void @llvm.wasm.prefetch.t(i8* %s)
+  ret void
+}
+
+define void @prefetch_t_with_unfolded_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_with_unfolded_offset:
+; CHECK: .functype prefetch_t_with_unfolded_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:i32.const 24
+; CHECK-NEXT:i32.add
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  %q = ptrtoint i8* %p to i32
+  %r = add nsw i32 %q, 24
+  %s = inttoptr i32 %r to i8*
+  tail call void @llvm.wasm.prefetch.t(i8* %s)
+  ret void
+}
+
+define void @prefetch_t_with_unfolded_gep_offset(i8* %p) {
+; CHECK-LABEL: prefetch_t_with_unfolded_gep_offset:
+; CHECK: .functype prefetch_t_with_unfolded_gep_offset (i32) -> ()
+; CHECK-NEXT:  # %bb.0:
+; CHECK-NEXT:local.get 0
+; CHECK-NEXT:i32.const 6
+; CHECK-NEXT:i32.add
+; CHECK-NEXT:prefetch.t 0
+; CHECK-NEXT:# fallthrough-return
+  %s = getelementptr i8, i8* %p, i32 6
+  tail call void @llvm.wasm.prefetch.t(i8* %s)
+  ret void
+}
+
+define void 

[PATCH] D93637: [libTooling] Add support for smart pointers to relevant Transformer `Stencil`s.

[Yitzhak Mandelbaum 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 rG675a2973ee77: [libTooling] Add support for smart pointers to 
relevant Transformer `Stencil`s. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93637

Files:
  clang/include/clang/Tooling/Transformer/Stencil.h
  clang/lib/Tooling/Transformer/Stencil.cpp
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Expr.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/FixIt.h"
 #include "clang/Tooling/Tooling.h"
@@ -29,10 +30,18 @@
 using MatchResult = MatchFinder::MatchResult;
 
 // Create a valid translation-unit from a statement.
-static std::string wrapSnippet(StringRef StatementCode) {
-  return ("namespace N { class C {}; } "
-  "namespace { class AnonC {}; } "
-  "struct S { int field; }; auto stencil_test_snippet = []{" +
+static std::string wrapSnippet(StringRef ExtraPreface,
+   StringRef StatementCode) {
+  constexpr char Preface[] = R"cc(
+namespace N { class C {}; }
+namespace { class AnonC {}; }
+struct S { int Field; };
+struct Smart {
+  S* operator->() const;
+  S& operator*() const;
+};
+  )cc";
+  return (Preface + ExtraPreface + "auto stencil_test_snippet = []{" +
   StatementCode + "};")
   .str();
 }
@@ -55,10 +64,12 @@
 // matcher correspondingly. `Matcher` should match one of the statements in
 // `StatementCode` exactly -- that is, produce exactly one match. However,
 // `StatementCode` may contain other statements not described by `Matcher`.
+// `ExtraPreface` (optionally) adds extra decls to the TU, before the code.
 static llvm::Optional matchStmt(StringRef StatementCode,
-   StatementMatcher Matcher) {
-  auto AstUnit = tooling::buildASTFromCodeWithArgs(wrapSnippet(StatementCode),
-   {"-Wno-unused-value"});
+   StatementMatcher Matcher,
+   StringRef ExtraPreface = "") {
+  auto AstUnit = tooling::buildASTFromCodeWithArgs(
+  wrapSnippet(ExtraPreface, StatementCode), {"-Wno-unused-value"});
   if (AstUnit == nullptr) {
 ADD_FAILURE() << "AST construction failed";
 return llvm::None;
@@ -260,6 +271,42 @@
   testExpr(Id, "int x; ", maybeDeref(Id), "x");
 }
 
+TEST_F(StencilTest, MaybeDerefSmartPointer) {
+  StringRef Id = "id";
+  std::string Snippet = R"cc(
+Smart x;
+x;
+  )cc";
+  testExpr(Id, Snippet, maybeDeref(Id), "*x");
+}
+
+// Tests that unique_ptr specifically is handled.
+TEST_F(StencilTest, MaybeDerefSmartPointerUniquePtr) {
+  StringRef Id = "id";
+  // We deliberately specify `unique_ptr` as empty to verify that it matches
+  // because of its name, rather than its contents.
+  StringRef ExtraPreface =
+  "namespace std { template  class unique_ptr {}; }\n";
+  StringRef Snippet = R"cc(
+std::unique_ptr x;
+x;
+  )cc";
+  auto StmtMatch = matchStmt(Snippet, expr().bind(Id), ExtraPreface);
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(maybeDeref(Id)->eval(StmtMatch->Result),
+   HasValue(std::string("*x")));
+}
+
+TEST_F(StencilTest, MaybeDerefSmartPointerFromMemberExpr) {
+  StringRef Id = "id";
+  std::string Snippet = "Smart x; x->Field;";
+  auto StmtMatch =
+  matchStmt(Snippet, memberExpr(hasObjectExpression(expr().bind(Id;
+  ASSERT_TRUE(StmtMatch);
+  const Stencil Stencil = maybeDeref(Id);
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("*x"));
+}
+
 TEST_F(StencilTest, MaybeAddressOfPointer) {
   StringRef Id = "id";
   testExpr(Id, "int *x; x;", maybeAddressOf(Id), "x");
@@ -280,6 +327,26 @@
   testExpr(Id, "int *x; *x;", addressOf(Id), "x");
 }
 
+TEST_F(StencilTest, MaybeAddressOfSmartPointer) {
+  StringRef Id = "id";
+  testExpr(Id, "Smart x; x;", maybeAddressOf(Id), "x");
+}
+
+TEST_F(StencilTest, MaybeAddressOfSmartPointerFromMemberCall) {
+  StringRef Id = "id";
+  std::string Snippet = "Smart x; x->Field;";
+  auto StmtMatch =
+  matchStmt(Snippet, memberExpr(hasObjectExpression(expr().bind(Id;
+  ASSERT_TRUE(StmtMatch);
+  const Stencil Stencil = maybeAddressOf(Id);
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("x"));
+}
+
+TEST_F(StencilTest, MaybeAddressOfSmartPointerDerefNoCancel) {
+  StringRef Id = "id";
+  testExpr(Id, "Smart x; *x;", maybeAddressOf(Id), "&*x");
+}
+
 TEST_F(StencilTest, AccessOpValue) {
  

[clang] 675a297 - [libTooling] Add support for smart pointers to relevant Transformer `Stencil`s.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2021-01-05T17:57:41Z
New Revision: 675a2973ee7745d1859e3b72be40a803dd349e55

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

LOG: [libTooling] Add support for smart pointers to relevant Transformer 
`Stencil`s.

Stencils `maybeDeref` and `maybeAddressOf` are designed to handle nodes that may
be pointers. Currently, they only handle native pointers. This patch extends the
support to recognize smart pointers and handle them as well.

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

Added: 


Modified: 
clang/include/clang/Tooling/Transformer/Stencil.h
clang/lib/Tooling/Transformer/Stencil.cpp
clang/unittests/Tooling/StencilTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Transformer/Stencil.h 
b/clang/include/clang/Tooling/Transformer/Stencil.h
index b729c826c808..1b7495eb0262 100644
--- a/clang/include/clang/Tooling/Transformer/Stencil.h
+++ b/clang/include/clang/Tooling/Transformer/Stencil.h
@@ -82,7 +82,6 @@ Stencil deref(llvm::StringRef ExprId);
 /// If \p ExprId is of pointer type, constructs an idiomatic dereferencing of
 /// the expression bound to \p ExprId, including wrapping it in parentheses, if
 /// needed. Otherwise, generates the original expression source.
-/// FIXME: Identify smart-pointers as pointer types.
 Stencil maybeDeref(llvm::StringRef ExprId);
 
 /// Constructs an expression that idiomatically takes the address of the
@@ -94,7 +93,6 @@ Stencil addressOf(llvm::StringRef ExprId);
 /// idiomatically takes the address of the expression bound to \p ExprId,
 /// including wrapping \p ExprId in parentheses, if needed. Otherwise, 
generates
 /// the original expression source.
-/// FIXME: Identify smart-pointers as pointer types.
 Stencil maybeAddressOf(llvm::StringRef ExprId);
 
 /// Constructs a `MemberExpr` that accesses the named member (\p Member) of the

diff  --git a/clang/lib/Tooling/Transformer/Stencil.cpp 
b/clang/lib/Tooling/Transformer/Stencil.cpp
index 56f145393691..d46087e4b04b 100644
--- a/clang/lib/Tooling/Transformer/Stencil.cpp
+++ b/clang/lib/Tooling/Transformer/Stencil.cpp
@@ -195,6 +195,24 @@ Error evalData(const DebugPrintNodeData ,
   return printNode(Data.Id, Match, Result);
 }
 
+// FIXME: Consider memoizing this function using the `ASTContext`.
+static bool isSmartPointerType(QualType Ty, ASTContext ) {
+  using namespace ::clang::ast_matchers;
+
+  // Optimization: hard-code common smart-pointer types. This can/should be
+  // removed if we start caching the results of this function.
+  auto KnownSmartPointer =
+  cxxRecordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+  const auto QuacksLikeASmartPointer = cxxRecordDecl(
+  hasMethod(cxxMethodDecl(hasOverloadedOperatorName("->"),
+  returns(qualType(pointsTo(type()),
+  hasMethod(cxxMethodDecl(hasOverloadedOperatorName("*"),
+  returns(qualType(references(type()));
+  const auto SmartPointer = qualType(hasDeclaration(
+  cxxRecordDecl(anyOf(KnownSmartPointer, QuacksLikeASmartPointer;
+  return match(SmartPointer, Ty, Context).size() > 0;
+}
+
 Error evalData(const UnaryOperationData ,
const MatchFinder::MatchResult , std::string *Result) {
   // The `Describe` operation can be applied to any node, not just expressions,
@@ -215,17 +233,37 @@ Error evalData(const UnaryOperationData ,
 Source = tooling::buildDereference(*E, *Match.Context);
 break;
   case UnaryNodeOperator::MaybeDeref:
-if (!E->getType()->isAnyPointerType()) {
-  *Result += tooling::getText(*E, *Match.Context);
-  return Error::success();
+if (E->getType()->isAnyPointerType() ||
+isSmartPointerType(E->getType(), *Match.Context)) {
+  // Strip off any operator->. This can only occur inside an actual arrow
+  // member access, so we treat it as equivalent to an actual object
+  // expression.
+  if (const auto *OpCall = dyn_cast(E)) {
+if (OpCall->getOperator() == clang::OO_Arrow &&
+OpCall->getNumArgs() == 1) {
+  E = OpCall->getArg(0);
+}
+  }
+  Source = tooling::buildDereference(*E, *Match.Context);
+  break;
 }
-Source = tooling::buildDereference(*E, *Match.Context);
-break;
+*Result += tooling::getText(*E, *Match.Context);
+return Error::success();
   case UnaryNodeOperator::AddressOf:
 Source = tooling::buildAddressOf(*E, *Match.Context);
 break;
   case UnaryNodeOperator::MaybeAddressOf:
-if (E->getType()->isAnyPointerType()) {
+if (E->getType()->isAnyPointerType() ||
+isSmartPointerType(E->getType(), *Match.Context)) {
+  // Strip off any operator->. This can only occur inside an actual arrow
+  

[PATCH] D94031: [ASTMatchers] Fix child traversal over range-for loops

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:245
+ScopedIncrement ScopedDepth();
+if (!match(*Node->getLoopVariable()) || !match(*Node->getRangeInit()) ||
+!match(*Node->getBody()))

aaron.ballman wrote:
> Should we be traversing the init statement before the loop variable so that 
> the traversals happen in lexical order?
Do you mean that in 

```
for (auto i : arr)
{
}
```

to visit the `arr` before the `auto i`? 

I think visiting the `auto i` before the `arr` makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94031

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

[clang] f22c0f4 - [ASTMatchers] Omit methods from explicit template instantations

[Stephen Kelly via cfe-commits]

Author: Stephen Kelly
Date: 2021-01-05T17:42:33Z
New Revision: f22c0f40b5d657c0293fc9332274c18d3c4f836c

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

LOG: [ASTMatchers] Omit methods from explicit template instantations

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

Added: 


Modified: 
clang/lib/ASTMatchers/ASTMatchFinder.cpp
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Removed: 




diff  --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp 
b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
index 54dc874470dd..99d95838af61 100644
--- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -1158,6 +1158,8 @@ bool MatchASTVisitor::TraverseDecl(Decl *DeclNode) {
   } else if (const auto *FD = dyn_cast(DeclNode)) {
 if (FD->isDefaulted())
   ScopedChildren = true;
+if (FD->isTemplateInstantiation())
+  ScopedTraversal = true;
   }
 
   ASTNodeNotSpelledInSourceScope RAII1(this, ScopedTraversal);

diff  --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index cde3e460eeb0..e706ea4b2a54 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2201,10 +2201,18 @@ struct TemplStruct {
   TemplStruct() {}
   ~TemplStruct() {}
 
+  void outOfLine(T);
+
 private:
   T m_t;
 };
 
+template
+void TemplStruct::outOfLine(T)
+{
+
+}
+
 template
 T timesTwo(T input)
 {
@@ -2277,7 +2285,7 @@ template<> bool timesTwo(bool){
  hasTemplateArgument(0, refersToType(asString("float";
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MTempl)));
 // TODO: If we could match on explicit instantiations of function 
templates,
-// this would be EXPECT_TRUE.
+// this would be EXPECT_TRUE. See Sema::ActOnExplicitInstantiation.
 EXPECT_FALSE(
 matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MTempl)));
   }
@@ -2324,6 +2332,14 @@ template<> bool timesTwo(bool){
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
 EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
+  {
+// Instantiated, out-of-line methods are not matchable.
+auto M =
+cxxMethodDecl(hasName("outOfLine"), isDefinition(),
+  hasParameter(0, 
parmVarDecl(hasType(asString("float");
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
   {
 // Explicit specialization is written in source and it matches:
 auto M = classTemplateSpecializationDecl(



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

[PATCH] D94032: [ASTMatchers] Omit methods from explicit template instantations

[Stephen Kelly via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf22c0f40b5d6: [ASTMatchers] Omit methods from explicit 
template instantations (authored by stephenkelly).

Changed prior to commit:
  https://reviews.llvm.org/D94032?vs=314451=314643#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94032

Files:
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2201,10 +2201,18 @@
   TemplStruct() {}
   ~TemplStruct() {}
 
+  void outOfLine(T);
+
 private:
   T m_t;
 };
 
+template
+void TemplStruct::outOfLine(T)
+{
+
+}
+
 template
 T timesTwo(T input)
 {
@@ -2277,7 +2285,7 @@
  hasTemplateArgument(0, refersToType(asString("float";
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MTempl)));
 // TODO: If we could match on explicit instantiations of function 
templates,
-// this would be EXPECT_TRUE.
+// this would be EXPECT_TRUE. See Sema::ActOnExplicitInstantiation.
 EXPECT_FALSE(
 matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MTempl)));
   }
@@ -2324,6 +2332,14 @@
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
 EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
+  {
+// Instantiated, out-of-line methods are not matchable.
+auto M =
+cxxMethodDecl(hasName("outOfLine"), isDefinition(),
+  hasParameter(0, 
parmVarDecl(hasType(asString("float");
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
   {
 // Explicit specialization is written in source and it matches:
 auto M = classTemplateSpecializationDecl(
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -1158,6 +1158,8 @@
   } else if (const auto *FD = dyn_cast(DeclNode)) {
 if (FD->isDefaulted())
   ScopedChildren = true;
+if (FD->isTemplateInstantiation())
+  ScopedTraversal = true;
   }
 
   ASTNodeNotSpelledInSourceScope RAII1(this, ScopedTraversal);


Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -2201,10 +2201,18 @@
   TemplStruct() {}
   ~TemplStruct() {}
 
+  void outOfLine(T);
+
 private:
   T m_t;
 };
 
+template
+void TemplStruct::outOfLine(T)
+{
+
+}
+
 template
 T timesTwo(T input)
 {
@@ -2277,7 +2285,7 @@
  hasTemplateArgument(0, refersToType(asString("float";
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, MTempl)));
 // TODO: If we could match on explicit instantiations of function templates,
-// this would be EXPECT_TRUE.
+// this would be EXPECT_TRUE. See Sema::ActOnExplicitInstantiation.
 EXPECT_FALSE(
 matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, MTempl)));
   }
@@ -2324,6 +2332,14 @@
 EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
 EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
   }
+  {
+// Instantiated, out-of-line methods are not matchable.
+auto M =
+cxxMethodDecl(hasName("outOfLine"), isDefinition(),
+  hasParameter(0, parmVarDecl(hasType(asString("float");
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
   {
 // Explicit specialization is written in source and it matches:
 auto M = classTemplateSpecializationDecl(
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -1158,6 +1158,8 @@
   } else if (const auto *FD = dyn_cast(DeclNode)) {
 if (FD->isDefaulted())
   ScopedChildren = true;
+if (FD->isTemplateInstantiation())
+  ScopedTraversal = true;
   }
 
   ASTNodeNotSpelledInSourceScope RAII1(this, ScopedTraversal);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

I tried the updated patch against our build and it was successful. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92936

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

[PATCH] D94031: [ASTMatchers] Fix child traversal over range-for loops

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:245
+ScopedIncrement ScopedDepth();
+if (!match(*Node->getLoopVariable()) || !match(*Node->getRangeInit()) ||
+!match(*Node->getBody()))

Should we be traversing the init statement before the loop variable so that the 
traversals happen in lexical order?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94031

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

[PATCH] D93354: [clang-tidy] Make clang-format and include-order-check coherent again

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

i thought clang-format was also putting angled includes and "OtherHeaders" into 
same category, by looking at:

  LLVMStyle.IncludeStyle.IncludeCategories = {
{"^\"(llvm|llvm-c|clang|clang-c)/", 2, 0, false},
{"^(<|\"(gtest|gmock|isl|json)/)", 3, 0, false},  // Starts with 
`<` or gtest/gmock...
{".*", 1, 0, false}};

not sure why it is done so (stylistic preferences i suppose ...). it definitely 
makes sense for me to separate them into 2 different categories, but i wouldn't 
dare make that decision :D maybe ask on cfe-dev first to get some consensus.
but it would make sense to include isl and json in this list for now, i forgot 
to do that in the initial patch :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93354

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

[PATCH] D93637: [libTooling] Add support for smart pointers to relevant Transformer `Stencil`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D93637#2479576 , @tdl-g wrote:

> Ah, if it's just an optimization that makes sense.  I still think it's worth 
> having a test case to confirm that one of the specially-handled cases works.

Sure. I added one for unique pointer. but, just one -- i didn't add one per 
case. My logic is that since it's not part of the contract/API, we're really 
just using the (single) test to test the internal function 
`isSmartPointerType`, so one test is enough. That said, if you disagree, I'm 
fine with adding more tests. Its easy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93637

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

[PATCH] D93637: [libTooling] Add support for smart pointers to relevant Transformer `Stencil`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 314642.
ymandel marked an inline comment as done.
ymandel added a comment.

Added test for unique_ptr specifically.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93637

Files:
  clang/include/clang/Tooling/Transformer/Stencil.h
  clang/lib/Tooling/Transformer/Stencil.cpp
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Expr.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/FixIt.h"
 #include "clang/Tooling/Tooling.h"
@@ -29,10 +30,18 @@
 using MatchResult = MatchFinder::MatchResult;
 
 // Create a valid translation-unit from a statement.
-static std::string wrapSnippet(StringRef StatementCode) {
-  return ("namespace N { class C {}; } "
-  "namespace { class AnonC {}; } "
-  "struct S { int field; }; auto stencil_test_snippet = []{" +
+static std::string wrapSnippet(StringRef ExtraPreface,
+   StringRef StatementCode) {
+  constexpr char Preface[] = R"cc(
+namespace N { class C {}; }
+namespace { class AnonC {}; }
+struct S { int Field; };
+struct Smart {
+  S* operator->() const;
+  S& operator*() const;
+};
+  )cc";
+  return (Preface + ExtraPreface + "auto stencil_test_snippet = []{" +
   StatementCode + "};")
   .str();
 }
@@ -55,10 +64,12 @@
 // matcher correspondingly. `Matcher` should match one of the statements in
 // `StatementCode` exactly -- that is, produce exactly one match. However,
 // `StatementCode` may contain other statements not described by `Matcher`.
+// `ExtraPreface` (optionally) adds extra decls to the TU, before the code.
 static llvm::Optional matchStmt(StringRef StatementCode,
-   StatementMatcher Matcher) {
-  auto AstUnit = tooling::buildASTFromCodeWithArgs(wrapSnippet(StatementCode),
-   {"-Wno-unused-value"});
+   StatementMatcher Matcher,
+   StringRef ExtraPreface = "") {
+  auto AstUnit = tooling::buildASTFromCodeWithArgs(
+  wrapSnippet(ExtraPreface, StatementCode), {"-Wno-unused-value"});
   if (AstUnit == nullptr) {
 ADD_FAILURE() << "AST construction failed";
 return llvm::None;
@@ -260,6 +271,42 @@
   testExpr(Id, "int x; ", maybeDeref(Id), "x");
 }
 
+TEST_F(StencilTest, MaybeDerefSmartPointer) {
+  StringRef Id = "id";
+  std::string Snippet = R"cc(
+Smart x;
+x;
+  )cc";
+  testExpr(Id, Snippet, maybeDeref(Id), "*x");
+}
+
+// Tests that unique_ptr specifically is handled.
+TEST_F(StencilTest, MaybeDerefSmartPointerUniquePtr) {
+  StringRef Id = "id";
+  // We deliberately specify `unique_ptr` as empty to verify that it matches
+  // because of its name, rather than its contents.
+  StringRef ExtraPreface =
+  "namespace std { template  class unique_ptr {}; }\n";
+  StringRef Snippet = R"cc(
+std::unique_ptr x;
+x;
+  )cc";
+  auto StmtMatch = matchStmt(Snippet, expr().bind(Id), ExtraPreface);
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(maybeDeref(Id)->eval(StmtMatch->Result),
+   HasValue(std::string("*x")));
+}
+
+TEST_F(StencilTest, MaybeDerefSmartPointerFromMemberExpr) {
+  StringRef Id = "id";
+  std::string Snippet = "Smart x; x->Field;";
+  auto StmtMatch =
+  matchStmt(Snippet, memberExpr(hasObjectExpression(expr().bind(Id;
+  ASSERT_TRUE(StmtMatch);
+  const Stencil Stencil = maybeDeref(Id);
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("*x"));
+}
+
 TEST_F(StencilTest, MaybeAddressOfPointer) {
   StringRef Id = "id";
   testExpr(Id, "int *x; x;", maybeAddressOf(Id), "x");
@@ -280,6 +327,26 @@
   testExpr(Id, "int *x; *x;", addressOf(Id), "x");
 }
 
+TEST_F(StencilTest, MaybeAddressOfSmartPointer) {
+  StringRef Id = "id";
+  testExpr(Id, "Smart x; x;", maybeAddressOf(Id), "x");
+}
+
+TEST_F(StencilTest, MaybeAddressOfSmartPointerFromMemberCall) {
+  StringRef Id = "id";
+  std::string Snippet = "Smart x; x->Field;";
+  auto StmtMatch =
+  matchStmt(Snippet, memberExpr(hasObjectExpression(expr().bind(Id;
+  ASSERT_TRUE(StmtMatch);
+  const Stencil Stencil = maybeAddressOf(Id);
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("x"));
+}
+
+TEST_F(StencilTest, MaybeAddressOfSmartPointerDerefNoCancel) {
+  StringRef Id = "id";
+  testExpr(Id, "Smart x; *x;", maybeAddressOf(Id), "&*x");
+}
+
 TEST_F(StencilTest, AccessOpValue) {
   StringRef Snippet = R"cc(
 S x;
Index: clang/lib/Tooling/Transformer/Stencil.cpp

[PATCH] D93385: [Driver][MachineOutliner] Support outlining option with LTO

[Jessica Paquette via Phabricator via cfe-commits]

paquette accepted this revision.
paquette 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/D93385/new/

https://reviews.llvm.org/D93385

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

[PATCH] D93785: [OpenMP][FIX] Ensure the isa trait is evaluated last

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 accepted this revision.
jhuber6 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/D93785/new/

https://reviews.llvm.org/D93785

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

[PATCH] D94032: [ASTMatchers] Omit methods from explicit template instantations

[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 with a commenting nit.




Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:2288-2289
 // TODO: If we could match on explicit instantiations of function 
templates,
 // this would be EXPECT_TRUE.
+// See Sema::ActOnExplicitInstantiation,
 EXPECT_FALSE(





Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:2337
+  {
+// Instantiated, out-of-line methods are not matchable
+auto M =




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94032

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

[PATCH] D93483: Add element-type to the Vector TypeLoc types.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/AST/TypeLoc.h:1756
+
+class VectorTypeLoc : public ConcreteTypeLoc {

fhahn wrote:
> Can we reuse/unify this with `MatrixTypeLoc`? And then have `MatrixTypeLoc` 
> just deal with the row/column operands.
Since these have to be ConcreteTypeLocs, I don't think the matrix type can 
directly inherit from it, but it can do something very similar.


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

https://reviews.llvm.org/D93483

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

[PATCH] D94098: [Clang] Inline assembly support for the ACLE type 'data512_t'.

[Alexandros Lamprineas via Phabricator via cfe-commits]

labrinea created this revision.
labrinea added reviewers: cfe-commits, t.p.northover, ab, kristof.beyls, 
simon_tatham.
labrinea requested review of this revision.
Herald added a project: clang.

This patch emits the new LLVM IR type introduced in 
https://reviews.llvm.org/D94091 when generating IR for inline assembly source 
code that operates on `data512_t`, as long as the target hooks indicate the 
presence of the LS64 extension.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94098

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aarch64-ls64-inline-asm.c

Index: clang/test/CodeGen/aarch64-ls64-inline-asm.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-ls64-inline-asm.c
@@ -0,0 +1,41 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple aarch64-eabi -target-feature +ls64 -S -emit-llvm -x c %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple aarch64_be-eabi -target-feature +ls64 -S -emit-llvm -x c %s -o - | FileCheck %s
+
+struct foo { unsigned long long x[8]; };
+
+// CHECK-LABEL: @load(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[OUTPUT_ADDR:%.*]] = alloca %struct.foo*, align 8
+// CHECK-NEXT:[[ADDR_ADDR:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:store %struct.foo* [[OUTPUT:%.*]], %struct.foo** [[OUTPUT_ADDR]], align 8
+// CHECK-NEXT:store i8* [[ADDR:%.*]], i8** [[ADDR_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load %struct.foo*, %struct.foo** [[OUTPUT_ADDR]], align 8
+// CHECK-NEXT:[[TMP1:%.*]] = load i8*, i8** [[ADDR_ADDR]], align 8
+// CHECK-NEXT:[[TMP2:%.*]] = call aarch64_ls64 asm sideeffect "ld64b $0,[$1]", "=r,r"(i8* [[TMP1]]) [[ATTR1:#.*]], !srcloc !6
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast %struct.foo* [[TMP0]] to aarch64_ls64*
+// CHECK-NEXT:store aarch64_ls64 [[TMP2]], aarch64_ls64* [[TMP3]], align 8
+// CHECK-NEXT:ret void
+//
+void load(struct foo *output, void *addr)
+{
+__asm__ volatile ("ld64b %0,[%1]" : "=r" (*output) : "r" (addr));
+}
+
+// CHECK-LABEL: @store(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[INPUT_ADDR:%.*]] = alloca %struct.foo*, align 8
+// CHECK-NEXT:[[ADDR_ADDR:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:store %struct.foo* [[INPUT:%.*]], %struct.foo** [[INPUT_ADDR]], align 8
+// CHECK-NEXT:store i8* [[ADDR:%.*]], i8** [[ADDR_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load %struct.foo*, %struct.foo** [[INPUT_ADDR]], align 8
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast %struct.foo* [[TMP0]] to aarch64_ls64*
+// CHECK-NEXT:[[TMP2:%.*]] = load aarch64_ls64, aarch64_ls64* [[TMP1]], align 8
+// CHECK-NEXT:[[TMP3:%.*]] = load i8*, i8** [[ADDR_ADDR]], align 8
+// CHECK-NEXT:call void asm sideeffect "st64b $0,[$1]", "r,r"(aarch64_ls64 [[TMP2]], i8* [[TMP3]]) [[ATTR1]], !srcloc !7
+// CHECK-NEXT:ret void
+//
+void store(const struct foo *input, void *addr)
+{
+__asm__ volatile ("st64b %0,[%1]" : : "r" (*input), "r" (addr));
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5533,6 +5533,23 @@
 Fn->addFnAttr("branch-target-enforcement",
   BPI.BranchTargetEnforcement ? "true" : "false");
   }
+
+  llvm::Type* adjustInlineAsmType(CodeGen::CodeGenFunction ,
+  StringRef Constraint,
+  llvm::Type* Ty) const override {
+if (getABIInfo().getContext().getTargetInfo().hasAArch64_LS64Type()) {
+  if (CGF.CGM.getDataLayout().getTypeSizeInBits(Ty) == 512) {
+auto *ST = dyn_cast(Ty);
+if (ST && ST->getNumElements() == 1) {
+  auto *AT = dyn_cast(ST->getElementType(0));
+  if (AT && AT->getNumElements() == 8 &&
+  AT->getElementType()->isIntegerTy(64))
+return llvm::Type::getAArch64_LS64Ty(CGF.getLLVMContext());
+}
+  }
+}
+return Ty;
+  }
 };
 
 class WindowsAArch64TargetCodeGenInfo : public AArch64TargetCodeGenInfo {
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2030,6 +2030,7 @@
   Arg = EmitLoadOfLValue(InputValue, Loc).getScalarVal();
 } else {
   llvm::Type *Ty = ConvertType(InputType);
+  Ty = getTargetHooks().adjustInlineAsmType(*this, ConstraintStr, Ty);
   uint64_t Size = CGM.getDataLayout().getTypeSizeInBits(Ty);
   if (Size <= 64 && llvm::isPowerOf2_64(Size)) {
 Ty = llvm::IntegerType::get(getLLVMContext(), Size);
@@ -2037,6 +2038,11 @@
 
 Arg = Builder.CreateLoad(
 Builder.CreateBitCast(InputValue.getAddress(*this), Ty));
+