[clang] 984554f - [clang][DR] Test and mark DR1479 as complete

2022-03-29 Thread Markus Böck via cfe-commits

Author: Markus Böck
Date: 2022-03-29T09:28:32+02:00
New Revision: 984554f846c44d4017be5b6e3bd694cc233ce7bc

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

LOG: [clang][DR] Test and mark DR1479 as complete

DR: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1479

Clang has implemented this DR as far back as I could go on compiler explorer 
(3.0). This patch simply adds a test case and needed comments for the update 
script to mark it as complete.

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

Added: 


Modified: 
clang/test/CXX/drs/dr14xx.cpp
clang/www/cxx_dr_status.html

Removed: 




diff  --git a/clang/test/CXX/drs/dr14xx.cpp b/clang/test/CXX/drs/dr14xx.cpp
index 4d64943e9eea0..06fcc190784d9 100644
--- a/clang/test/CXX/drs/dr14xx.cpp
+++ b/clang/test/CXX/drs/dr14xx.cpp
@@ -463,6 +463,10 @@ namespace dr1467 {  // dr1467: 3.7 c++11
 #endif
 } // dr1467
 
+namespace dr1479 { // dr1479: yes
+  int operator"" _a(const char*, std::size_t = 0); // expected-error {{literal 
operator cannot have a default argument}}
+}
+
 namespace dr1490 {  // dr1490: 3.7 c++11
   // List-initialization from a string literal
 

diff  --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 4734e9fd89253..6f66e2262a565 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -8688,7 +8688,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg1479";>1479
 CD3
 Literal operators and default arguments
-Unknown
+Yes
   
   
 https://wg21.link/cwg1480";>1480



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


[PATCH] D122620: [clang][DR] Test and mark DR1479 as complete

2022-03-29 Thread Markus Böck 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 rG984554f846c4: [clang][DR] Test and mark DR1479 as complete 
(authored by zero9178).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122620

Files:
  clang/test/CXX/drs/dr14xx.cpp
  clang/www/cxx_dr_status.html


Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -8688,7 +8688,7 @@
 https://wg21.link/cwg1479";>1479
 CD3
 Literal operators and default arguments
-Unknown
+Yes
   
   
 https://wg21.link/cwg1480";>1480
Index: clang/test/CXX/drs/dr14xx.cpp
===
--- clang/test/CXX/drs/dr14xx.cpp
+++ clang/test/CXX/drs/dr14xx.cpp
@@ -463,6 +463,10 @@
 #endif
 } // dr1467
 
+namespace dr1479 { // dr1479: yes
+  int operator"" _a(const char*, std::size_t = 0); // expected-error {{literal 
operator cannot have a default argument}}
+}
+
 namespace dr1490 {  // dr1490: 3.7 c++11
   // List-initialization from a string literal
 


Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -8688,7 +8688,7 @@
 https://wg21.link/cwg1479";>1479
 CD3
 Literal operators and default arguments
-Unknown
+Yes
   
   
 https://wg21.link/cwg1480";>1480
Index: clang/test/CXX/drs/dr14xx.cpp
===
--- clang/test/CXX/drs/dr14xx.cpp
+++ clang/test/CXX/drs/dr14xx.cpp
@@ -463,6 +463,10 @@
 #endif
 } // dr1467
 
+namespace dr1479 { // dr1479: yes
+  int operator"" _a(const char*, std::size_t = 0); // expected-error {{literal operator cannot have a default argument}}
+}
+
 namespace dr1490 {  // dr1490: 3.7 c++11
   // List-initialization from a string literal
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122567: [X86][AMX] enable amx cast intrinsics in FE.

2022-03-29 Thread Xiang Zhang via Phabricator via cfe-commits
xiangzhangllvm added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5413-5415
+if (PTy->isX86_AMXTy())
+  ArgValue = 
Builder.CreateIntrinsic(Intrinsic::x86_cast_vector_to_tile,
+ {ArgValue->getType()}, 
{ArgValue});

Can we fold it in CreateBitCast(ArgValue, PTy) function ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122567

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


[PATCH] D122567: [X86][AMX] enable amx cast intrinsics in FE.

2022-03-29 Thread LuoYuanke via Phabricator via cfe-commits
LuoYuanke added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5413-5415
+if (PTy->isX86_AMXTy())
+  ArgValue = 
Builder.CreateIntrinsic(Intrinsic::x86_cast_vector_to_tile,
+ {ArgValue->getType()}, 
{ArgValue});

xiangzhangllvm wrote:
> Can we fold it in CreateBitCast(ArgValue, PTy) function ?
I don't think so. We have amx specific cast to avoid some unexpected 
optimization for bitcast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122567

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


[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added a comment.

rebase?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

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


[PATCH] D122567: [X86][AMX] enable amx cast intrinsics in FE.

2022-03-29 Thread Xiang Zhang via Phabricator via cfe-commits
xiangzhangllvm added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:5413-5415
+if (PTy->isX86_AMXTy())
+  ArgValue = 
Builder.CreateIntrinsic(Intrinsic::x86_cast_vector_to_tile,
+ {ArgValue->getType()}, 
{ArgValue});

LuoYuanke wrote:
> xiangzhangllvm wrote:
> > Can we fold it in CreateBitCast(ArgValue, PTy) function ?
> I don't think so. We have amx specific cast to avoid some unexpected 
> optimization for bitcast.
That is not big issue. 
If nobody objected, I'll accept it soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122567

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


[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

2022-03-29 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:270
+  // needed.
+  BoolValue &ComparisonValue = MakeValue(Env, *HasValueVal);
+  auto *ComparisonExprLoc =

ymandel wrote:
> sgatev wrote:
> > ymandel wrote:
> > > ymandel wrote:
> > > > xazax.hun wrote:
> > > > > xazax.hun wrote:
> > > > > > ymandel wrote:
> > > > > > > xazax.hun wrote:
> > > > > > > > Is this the right way to initialize `ComparisonValue`?
> > > > > > > > 
> > > > > > > > Considering the expression: `opt.value_or(nullptr) != nullptr`
> > > > > > > > * When `has_value == false`, `opt.value_or(nullptr)` will 
> > > > > > > > return `nullptr`, so `!=` evaluates to false. This case seems 
> > > > > > > > to check out.
> > > > > > > > * However, when `has_value == true`, `opt` might still hold an 
> > > > > > > > `nullptr` and `!=` could still evaluate to false. 
> > > > > > > Thanks for digging into this. I think it's correct, but helpful 
> > > > > > > to step through:
> > > > > > > 
> > > > > > > Its correctness depends on `MakeValue`, so I'll focus on that in 
> > > > > > > particular. For the `nullptr` case, we'll get:
> > > > > > > ```
> > > > > > > HasValueVal && ContentsNotEqX
> > > > > > > ```
> > > > > > > So, when `has_value == true`, this basically reduces to 
> > > > > > > `ContentsNotEqX`. Since that's an atom, the result is 
> > > > > > > indeterminate, which I believe is the desired outcome.
> > > > > > > 
> > > > > > > WDYT?  Also, even if I've convinced you, please let me know how i 
> > > > > > > can improve the comments. For that matter, would `MakeValue` be 
> > > > > > > better with a more specific name, like "MakePredicate" or 
> > > > > > > somesuch?
> > > > > > I think what confuses me is that we do something different for the 
> > > > > > 3 cases. You convinced me that `HasValueVal && ContentsNotEqX` is 
> > > > > > correct. But we only do this for one branch out of the 3.  What is 
> > > > > > the reason for that?
> > > > > Oh, never mind. Yeah, I think changing `MakeValue` to `MakePredicate` 
> > > > > would make this a bit clearer. After a second read now I understand 
> > > > > better what is going on.
> > > > Just to be clear: the three cases you mean are lines 273-283, or 
> > > > something else?
> > > and never mind my question, then (I rpelied before I saw your updated). 
> > > I'll change the name and add comments.
> > Can you elaborate on the three cases on lines 273-283? Why not simply do
> > 
> > ```
> > auto &ComparisonExprLoc = Env.createStorageLocation(*ComparisonExpr);
> > Env.setStorageLocation(ComparisonExpr, ComparisonExprLoc);
> > Env.setValue(ComparisonExprLoc, ComparisonValue);
> > ```
> > Can you elaborate on the three cases on lines 273-283? Why not simply do
> > 
> > ```
> > auto &ComparisonExprLoc = Env.createStorageLocation(*ComparisonExpr);
> > Env.setStorageLocation(ComparisonExpr, ComparisonExprLoc);
> > Env.setValue(ComparisonExprLoc, ComparisonValue);
> > ```
> 
> for the second case: I think we should drop it -- I don't see a reason to 
> maintain the previous value (if there is any). It might be a good idea for 
> compositionality, but we're not doing that anywhere else, so it doesn't make 
> sense here.
> 
> for the first and third case: I assumed that if the expression already has a 
> location, we'd want to reuse it. But, based on your question, I take it 
> that's incorrect?
> 
Dropping the second case makes sense to me.

For the rest, `createStorageLocation` returns a stable storage location so the 
snippet above should be sufficient. However, `setStorageLocation` will fail if 
we try calling it again with the same expression, even if it's called with the 
same storage location. What do you think about making `setStorageLocation` not 
fail if it's called with the same arguments?



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:279
+ cast_or_null(Env.getValue(*ComparisonExprLoc))) {
+Env.setValue(*ComparisonExprLoc,
+ Env.makeAnd(*CurrentValue, ComparisonValue));

ymandel wrote:
> ymandel wrote:
> > xazax.hun wrote:
> > > I am still wondering a bit about this case. 
> > > 
> > > We generate: `HasValueVal and ContentsNotEqX and CurrentValue`.'
> > > I wonder if we want: `HasValueVal  and (ContentsNotEqX  <=> 
> > > CurrentValue)` instead?  Or even `HasValueVal  and CurrentValue`?
> > I don't think that the iff version is right, but `HasValueVal  and 
> > CurrentValue` could be. My concern is that we're not guaranteed that 
> > `CurrentValue` is populated. And, even if we were, it doesn't feel quite 
> > right. Assuming its a high fidelity model, we get (logically): 
> > `HasValue(opt) and Ne(ValueOr(opt,X),X)`. Then, when negated (say, on an 
> > else branch) we get `not(HasValue(opt)) or not(Ne(ValueOr(opt,X),X))` which 
> > is equivalent to `not(HasValue(opt)) or Eq(ValueOr

[PATCH] D122573: [TBAA] Emit distinct TBAA tags for pointers with different depths,types.

2022-03-29 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Can you put provide some flag?

Or -fstrict-aliasing controls this generation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122573

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


[PATCH] D122629: [RISCV] Add index check for vset/vget

2022-03-29 Thread Kito Cheng via Phabricator via cfe-commits
kito-cheng accepted this revision.
kito-cheng added a comment.
This revision is now accepted and ready to land.

LGTM, thanks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122629

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


[clang] de30408 - [OpenCL] opencl-c.h: remove a/b/c/i/p/n/v arg names

2022-03-29 Thread Sven van Haastregt via cfe-commits

Author: Sven van Haastregt
Date: 2022-03-29T10:16:27+01:00
New Revision: de30408b3b0a012f902b8565fa0b7625c1d5fec6

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

LOG: [OpenCL] opencl-c.h: remove a/b/c/i/p/n/v arg names

This simplifies completeness comparisons against OpenCLBuiltins.td and
also makes the header no longer "claim" any single-letter identifiers.

Continues the direction set out in D119560.

Added: 


Modified: 
clang/lib/Headers/opencl-c.h

Removed: 




diff  --git a/clang/lib/Headers/opencl-c.h b/clang/lib/Headers/opencl-c.h
index 1a5f7183a7f0a..71b0fbb3a691e 100644
--- a/clang/lib/Headers/opencl-c.h
+++ b/clang/lib/Headers/opencl-c.h
@@ -7121,27 +7121,27 @@ half16 __ovld __cnfn floor(half16);
  * intermediate products shall not occur. Edge case
  * behavior is per the IEEE 754-2008 standard.
  */
-float __ovld __cnfn fma(float a, float b, float c);
-float2 __ovld __cnfn fma(float2 a, float2 b, float2 c);
-float3 __ovld __cnfn fma(float3 a, float3 b, float3 c);
-float4 __ovld __cnfn fma(float4 a, float4 b, float4 c);
-float8 __ovld __cnfn fma(float8 a, float8 b, float8 c);
-float16 __ovld __cnfn fma(float16 a, float16 b, float16 c);
+float __ovld __cnfn fma(float, float, float);
+float2 __ovld __cnfn fma(float2, float2, float2);
+float3 __ovld __cnfn fma(float3, float3, float3);
+float4 __ovld __cnfn fma(float4, float4, float4);
+float8 __ovld __cnfn fma(float8, float8, float8);
+float16 __ovld __cnfn fma(float16, float16, float16);
 #ifdef cl_khr_fp64
-double __ovld __cnfn fma(double a, double b, double c);
-double2 __ovld __cnfn fma(double2 a, double2 b, double2 c);
-double3 __ovld __cnfn fma(double3 a, double3 b, double3 c);
-double4 __ovld __cnfn fma(double4 a, double4 b, double4 c);
-double8 __ovld __cnfn fma(double8 a, double8 b, double8 c);
-double16 __ovld __cnfn fma(double16 a, double16 b, double16 c);
+double __ovld __cnfn fma(double, double, double);
+double2 __ovld __cnfn fma(double2, double2, double2);
+double3 __ovld __cnfn fma(double3, double3, double3);
+double4 __ovld __cnfn fma(double4, double4, double4);
+double8 __ovld __cnfn fma(double8, double8, double8);
+double16 __ovld __cnfn fma(double16, double16, double16);
 #endif //cl_khr_fp64
 #ifdef cl_khr_fp16
-half __ovld __cnfn fma(half a, half b, half c);
-half2 __ovld __cnfn fma(half2 a, half2 b, half2 c);
-half3 __ovld __cnfn fma(half3 a, half3 b, half3 c);
-half4 __ovld __cnfn fma(half4 a, half4 b, half4 c);
-half8 __ovld __cnfn fma(half8 a, half8 b, half8 c);
-half16 __ovld __cnfn fma(half16 a, half16 b, half16 c);
+half __ovld __cnfn fma(half, half, half);
+half2 __ovld __cnfn fma(half2, half2, half2);
+half3 __ovld __cnfn fma(half3, half3, half3);
+half4 __ovld __cnfn fma(half4, half4, half4);
+half8 __ovld __cnfn fma(half8, half8, half8);
+half16 __ovld __cnfn fma(half16, half16, half16);
 #endif //cl_khr_fp16
 
 /**
@@ -7496,42 +7496,42 @@ int16 __ovld __cnfn ilogb(half16);
 /**
  * Multiply x by 2 to the power n.
  */
-float __ovld __cnfn ldexp(float, int n);
-float2 __ovld __cnfn ldexp(float2, int2 n);
-float3 __ovld __cnfn ldexp(float3, int3 n);
-float4 __ovld __cnfn ldexp(float4, int4 n);
-float8 __ovld __cnfn ldexp(float8, int8 n);
-float16 __ovld __cnfn ldexp(float16, int16 n);
-float2 __ovld __cnfn ldexp(float2, int n);
-float3 __ovld __cnfn ldexp(float3, int n);
-float4 __ovld __cnfn ldexp(float4, int n);
-float8 __ovld __cnfn ldexp(float8, int n);
-float16 __ovld __cnfn ldexp(float16, int n);
+float __ovld __cnfn ldexp(float, int);
+float2 __ovld __cnfn ldexp(float2, int2);
+float3 __ovld __cnfn ldexp(float3, int3);
+float4 __ovld __cnfn ldexp(float4, int4);
+float8 __ovld __cnfn ldexp(float8, int8);
+float16 __ovld __cnfn ldexp(float16, int16);
+float2 __ovld __cnfn ldexp(float2, int);
+float3 __ovld __cnfn ldexp(float3, int);
+float4 __ovld __cnfn ldexp(float4, int);
+float8 __ovld __cnfn ldexp(float8, int);
+float16 __ovld __cnfn ldexp(float16, int);
 #ifdef cl_khr_fp64
-double __ovld __cnfn ldexp(double, int n);
-double2 __ovld __cnfn ldexp(double2, int2 n);
-double3 __ovld __cnfn ldexp(double3, int3 n);
-double4 __ovld __cnfn ldexp(double4, int4 n);
-double8 __ovld __cnfn ldexp(double8, int8 n);
-double16 __ovld __cnfn ldexp(double16, int16 n);
-double2 __ovld __cnfn ldexp(double2, int n);
-double3 __ovld __cnfn ldexp(double3, int n);
-double4 __ovld __cnfn ldexp(double4, int n);
-double8 __ovld __cnfn ldexp(double8, int n);
-double16 __ovld __cnfn ldexp(double16, int n);
+double __ovld __cnfn ldexp(double, int);
+double2 __ovld __cnfn ldexp(double2, int2);
+double3 __ovld __cnfn ldexp(double3, int3);
+double4 __ovld __cnfn ldexp(double4, int4);
+double8 __ovld __cnfn ldexp(double8, int8);
+double16 __ovld __cnfn ldexp(double16, int16);
+double2 __ovld __cnfn ldexp(doub

[PATCH] D118370: [clang-tidy] bugprone-signal-handler: Message improvement and code refactoring.

2022-03-29 Thread Whisperity via Phabricator via cfe-commits
whisperity accepted this revision.
whisperity added a comment.
This revision is now accepted and ready to land.

Alright, I think this is good to go. I like that it makes it clear that the 
called function is coming from some external source (system header or 
otherwise).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118370

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


[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

2022-03-29 Thread Whisperity via Phabricator via cfe-commits
whisperity added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:153
+diag(HandlerLambda->getBeginLoc(),
+ "lambda function is not allowed as signal handler (until C++17)")
+<< HandlerLambda->getSourceRange();

balazske wrote:
> whisperity wrote:
> > I am trying to find some source for this claim but so far hit a blank. 😦 
> > Could you please elaborate where this information comes from? Most notably, 
> > [[ https://en.cppreference.com/w/cpp/utility/program/signal | std::signal 
> > on CppReference ]] makes no mention of this, which would be the first place 
> > I would expect most people to look at.
> > 
> > Maybe this claim is derived from the rule that signal handlers **MUST** 
> > have C linkage? (If so, is there a warning against people setting 
> > C++-linkage functions as signal handlers in this check in general?)
> This check is made to comply with a CERT rule [[ 
> https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC54-CPP.+A+signal+handler+must+be+a+plain+old+function
>  | MSC54-CPP ]]. According to this only the subset of C and C++ should be 
> used in a signal handler, and it should have extern "C" linkage. This does 
> not allow lambda functions because the linkage restriction (if not others). 
> (From C++17 on other rules apply, this check is not applicable for such code.)
Ah, right, I found it a bit further down the page:

> //Signal handlers are expected to have C linkage// and, in general, only use 
> the features from the common subset of C and C++. It is 
> implementation-defined if a function with C++ linkage can be used as a signal 
> handler. (until C++17)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118996

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


[PATCH] D122104: [X86][regcall] Support passing / returning structures

2022-03-29 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:5238
+  for (unsigned i = 0; i < IRCallArgs.size(); ++i)
+LargestVectorWidth = std::max(LargestVectorWidth,
+  getMaxVectorWidth(IRCallArgs[i]->getType()));

pengfei wrote:
> LuoYuanke wrote:
> > Does this also affect other calling convention besides fastcall?
> I don't think so. The change here adds for the missing cases like `[2 x <4 x 
> double>]` or `{ <2 x double>, <4 x double> }` which should also set 
> `min-legal-width-width` to the maximum of the vector length.
> There're several reasons why other calling convention won't be affected.
> 1. If a target has ability to pass arguments like `[2 x <4 x double>]`, it 
> must have the ability for `<4 x double>` and have set `min-legal-width-width` 
> to 256 when passing it. So it makes more sense to set `min-legal-width-width` 
> to 256 for `[2 x <4 x double>]` rather than keeping it as 0;
> 2. AFAIK, targets other than X86 simply ignore `min-legal-width-width`. So 
> the change won't affect them;
> 3. On x86, calling conventions other than regcall don't allow arguments size 
> larger than 512, see `if (!IsRegCall && Size > 512)`. They will be turned 
> into pointers, so they won't be affected by this change;
> 4. For arguments size no larger than 512 and only contain single vector 
> element, we have already turned them into pure vectors. So they have already 
> set `min-legal-width-width` to the correct value;
> 5. For arguments have more then one vector elements. Clang has bug which 
> doesn't match with GCC and ICC. I have filed a bug here 
> https://github.com/llvm/llvm-project/issues/54582
> 6. Thus, only regcall can generate arguments type like `[2 x <4 x double>]` 
> on X86. So only it will be affected by this.
Update for bullet 5. The handling for multi vector elements is correct. It's 
also passed / returned by memory. So it's still not affected by this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122104

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


[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment.

Why is this fold preferable to `(X & (X-1)) == 0`? At least on all 
architectures without native population count, the binary-and based test is 
preferable and it might even be better with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

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


[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

In D122077#3413550 , @joerg wrote:

> Why is this fold preferable to `(X & (X-1)) == 0`? At least on all 
> architectures without native population count, the binary-and based test is 
> preferable and it might even be better with it.

Less IR instructions. I think SDAG already expands some ctpop patterns to logic 
(backends should decide about optimal form)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

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


[PATCH] D122378: [doc] Rely on tblgen to dump supported options value when generating doc

2022-03-29 Thread serge 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 rGf5c666742f7b: [doc] Rely on tblgen to dump supported options 
value when generating doc (authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122378

Files:
  clang/include/clang/Driver/Options.td
  llvm/utils/TableGen/OptRSTEmitter.cpp

Index: llvm/utils/TableGen/OptRSTEmitter.cpp
===
--- llvm/utils/TableGen/OptRSTEmitter.cpp
+++ llvm/utils/TableGen/OptRSTEmitter.cpp
@@ -60,18 +60,45 @@
   // Print the option name.
   OS << R->getValueAsString("Name");
 
+  StringRef MetaVarName;
   // Print the meta-variable.
   if (!isa(R->getValueInit("MetaVarName"))) {
+MetaVarName = R->getValueAsString("MetaVarName");
+  } else if (!isa(R->getValueInit("Values")))
+MetaVarName = "";
+
+  if (!MetaVarName.empty()) {
 OS << '=';
-OS.write_escaped(R->getValueAsString("MetaVarName"));
+OS.write_escaped(MetaVarName);
   }
 
   OS << "\n\n";
 
+  std::string HelpText;
   // The option help text.
   if (!isa(R->getValueInit("HelpText"))) {
+HelpText = R->getValueAsString("HelpText").trim().str();
+if (!HelpText.empty() && HelpText.back() != '.')
+  HelpText.push_back('.');
+  }
+
+  if (!isa(R->getValueInit("Values"))) {
+SmallVector Values;
+SplitString(R->getValueAsString("Values"), Values, ",");
+HelpText += (" " + MetaVarName + " can be ").str();
+
+if (Values.size() == 1) {
+  HelpText += ("'" + Values.front() + "'.").str();
+} else {
+  HelpText += "one of '";
+  HelpText += join(Values.begin(), Values.end() - 1, "', '");
+  HelpText += ("' or '" + Values.back() + "'.").str();
+}
+  }
+
+  if (!HelpText.empty()) {
 OS << ' ';
-OS.write_escaped(R->getValueAsString("HelpText"));
+OS.write_escaped(HelpText);
 OS << "\n\n";
   }
 }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -960,7 +960,7 @@
   PosFlag,
   NegFlag>;
 def fgpu_default_stream_EQ : Joined<["-"], "fgpu-default-stream=">,
-  HelpText<"Specify default stream. Valid values are 'legacy' and 'per-thread'. The default value is 'legacy'. (HIP only)">,
+  HelpText<"Specify default stream. The default value is 'legacy'. (HIP only)">,
   Flags<[CC1Option]>,
   Values<"legacy,per-thread">,
   NormalizedValuesScope<"LangOptions::GPUDefaultStreamKind">,
@@ -1173,7 +1173,7 @@
   MarshallingInfoStringVector>;
 def fembed_bitcode_EQ : Joined<["-"], "fembed-bitcode=">,
 Group, Flags<[NoXarchOption, CC1Option, CC1AsOption]>, MetaVarName<"">,
-HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">,
+HelpText<"Embed LLVM bitcode">,
 Values<"off,all,bitcode,marker">, NormalizedValuesScope<"CodeGenOptions">,
 NormalizedValues<["Embed_Off", "Embed_All", "Embed_Bitcode", "Embed_Marker"]>,
 MarshallingInfoEnum, "Embed_Off">;
@@ -1299,7 +1299,7 @@
 ShouldParseIf;
 def fprofile_update_EQ : Joined<["-"], "fprofile-update=">,
 Group, Flags<[CC1Option, CoreOption]>, Values<"atomic,prefer-atomic,single">,
-MetaVarName<"">, HelpText<"Set update method of profile counters (atomic,prefer-atomic,single)">,
+MetaVarName<"">, HelpText<"Set update method of profile counters">,
 MarshallingInfoFlag>;
 defm pseudo_probe_for_profiling : BoolFOption<"pseudo-probe-for-profiling",
   CodeGenOpts<"PseudoProbeForProfiling">, DefaultFalse,
@@ -1314,7 +1314,7 @@
 MarshallingInfoStringVector>;
 def fswift_async_fp_EQ : Joined<["-"], "fswift-async-fp=">,
 Group, Flags<[CC1Option, CC1AsOption, CoreOption]>, MetaVarName<"">,
-HelpText<"Control emission of Swift async extended frame info (option: auto, always, never)">,
+HelpText<"Control emission of Swift async extended frame info">,
 Values<"auto,always,never">,
 NormalizedValuesScope<"CodeGenOptions::SwiftAsyncFramePointerKind">,
 NormalizedValues<["Auto", "Always", "Never"]>,
@@ -1485,7 +1485,7 @@
 def fwasm_exceptions : Flag<["-"], "fwasm-exceptions">, Group,
   HelpText<"Use WebAssembly style exceptions">;
 def exception_model : Separate<["-"], "exception-model">,
-  Flags<[CC1Option, NoDriverOption]>, HelpText<"The exception model: dwarf|sjlj|seh|wasm">,
+  Flags<[CC1Option, NoDriverOption]>, HelpText<"The exception model">,
   Values<"dwarf,sjlj,seh,wasm">,
   NormalizedValuesScope<"LangOptions::ExceptionHandlingKind">,
   NormalizedValues<["DwarfCFI", "SjLj", "WinEH", "Wasm"]>,
@@ -1667,7 +1667,7 @@
   : Joined<["-"], "fsanitize-address-use-after-return="

[clang] f5c6667 - [doc] Rely on tblgen to dump supported options value when generating doc

2022-03-29 Thread via cfe-commits

Author: serge-sans-paille
Date: 2022-03-29T12:25:33+02:00
New Revision: f5c666742f7bb4ae79ec79c8acf61dced4d37cc9

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

LOG: [doc] Rely on tblgen to dump supported options value when generating doc

It was already the case for CLI help, also support it for rst output. As a side
effect remove redundant (and sometime inconsistent!) value help from HelpText in
clang/Driver/Options.td.

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
llvm/utils/TableGen/OptRSTEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 8c488f254b1ec..153badd971349 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -960,7 +960,7 @@ defm cuda_short_ptr : BoolFOption<"cuda-short-ptr",
   PosFlag,
   NegFlag>;
 def fgpu_default_stream_EQ : Joined<["-"], "fgpu-default-stream=">,
-  HelpText<"Specify default stream. Valid values are 'legacy' and 
'per-thread'. The default value is 'legacy'. (HIP only)">,
+  HelpText<"Specify default stream. The default value is 'legacy'. (HIP 
only)">,
   Flags<[CC1Option]>,
   Values<"legacy,per-thread">,
   NormalizedValuesScope<"LangOptions::GPUDefaultStreamKind">,
@@ -1173,7 +1173,7 @@ def fembed_offload_object_EQ : Joined<["-"], 
"fembed-offload-object=">,
   MarshallingInfoStringVector>;
 def fembed_bitcode_EQ : Joined<["-"], "fembed-bitcode=">,
 Group, Flags<[NoXarchOption, CC1Option, CC1AsOption]>, 
MetaVarName<"">,
-HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">,
+HelpText<"Embed LLVM bitcode">,
 Values<"off,all,bitcode,marker">, NormalizedValuesScope<"CodeGenOptions">,
 NormalizedValues<["Embed_Off", "Embed_All", "Embed_Bitcode", 
"Embed_Marker"]>,
 MarshallingInfoEnum, "Embed_Off">;
@@ -1299,7 +1299,7 @@ def fprofile_exclude_files_EQ : Joined<["-"], 
"fprofile-exclude-files=">,
 ShouldParseIf;
 def fprofile_update_EQ : Joined<["-"], "fprofile-update=">,
 Group, Flags<[CC1Option, CoreOption]>, 
Values<"atomic,prefer-atomic,single">,
-MetaVarName<"">, HelpText<"Set update method of profile counters 
(atomic,prefer-atomic,single)">,
+MetaVarName<"">, HelpText<"Set update method of profile counters">,
 MarshallingInfoFlag>;
 defm pseudo_probe_for_profiling : BoolFOption<"pseudo-probe-for-profiling",
   CodeGenOpts<"PseudoProbeForProfiling">, DefaultFalse,
@@ -1314,7 +1314,7 @@ def fprofile_list_EQ : Joined<["-"], "fprofile-list=">,
 MarshallingInfoStringVector>;
 def fswift_async_fp_EQ : Joined<["-"], "fswift-async-fp=">,
 Group, Flags<[CC1Option, CC1AsOption, CoreOption]>, 
MetaVarName<"">,
-HelpText<"Control emission of Swift async extended frame info (option: 
auto, always, never)">,
+HelpText<"Control emission of Swift async extended frame info">,
 Values<"auto,always,never">,
 NormalizedValuesScope<"CodeGenOptions::SwiftAsyncFramePointerKind">,
 NormalizedValues<["Auto", "Always", "Never"]>,
@@ -1485,7 +1485,7 @@ def fseh_exceptions : Flag<["-"], "fseh-exceptions">, 
Group,
 def fwasm_exceptions : Flag<["-"], "fwasm-exceptions">, Group,
   HelpText<"Use WebAssembly style exceptions">;
 def exception_model : Separate<["-"], "exception-model">,
-  Flags<[CC1Option, NoDriverOption]>, HelpText<"The exception model: 
dwarf|sjlj|seh|wasm">,
+  Flags<[CC1Option, NoDriverOption]>, HelpText<"The exception model">,
   Values<"dwarf,sjlj,seh,wasm">,
   NormalizedValuesScope<"LangOptions::ExceptionHandlingKind">,
   NormalizedValues<["DwarfCFI", "SjLj", "WinEH", "Wasm"]>,
@@ -1667,7 +1667,7 @@ def sanitize_address_use_after_return_EQ
   : Joined<["-"], "fsanitize-address-use-after-return=">,
 MetaVarName<"">,
 Flags<[CC1Option]>,
-HelpText<"Select the mode of detecting stack use-after-return in 
AddressSanitizer: never | runtime (default) | always">,
+HelpText<"Select the mode of detecting stack use-after-return in 
AddressSanitizer">,
 Group,
 Values<"never,runtime,always">,
 NormalizedValuesScope<"llvm::AsanDetectStackUseAfterReturnMode">,
@@ -1958,7 +1958,7 @@ def finstrument_function_entry_bare : Flag<["-"], 
"finstrument-function-entry-ba
   HelpText<"Instrument function entry only, after inlining, without arguments 
to the instrumentation call">,
   MarshallingInfoFlag>;
 def fcf_protection_EQ : Joined<["-"], "fcf-protection=">, Flags<[CoreOption, 
CC1Option]>, Group,
-  HelpText<"Instrument control-flow architecture protection. Options: return, 
branch, full, none.">, Values<"return,branch,full,none">;
+  HelpText<"Instrument control-flow architecture protection">, 
Values<"return,branch,full,none">;
 def fcf_protection : F

[PATCH] D122446: [clang][extract-api] Add Objective-C interface support

2022-03-29 Thread Daniel Grumberg via Phabricator via cfe-commits
dang added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:245
+if (const auto *SuperClassDecl = Decl->getSuperClass()) {
+  SuperClass.Name = SuperClassDecl->getObjCRuntimeNameAsString();
+  SuperClass.USR = API.recordUSR(SuperClassDecl);

zixuw wrote:
> dang wrote:
> > Shouldn't we be recording this string in the StringAllocator for reuse 
> > later. I have a patch in progress for macro support that will do the 
> > serialization once the AST isn't available anymore so we really shouldn't 
> > be taking in StringRefs for stuff in the AST.
> Right but that only makes sense once we kill the AST before serialization 
> right? I mean I'm happy to wrap these in `copyString` now if we know for sure 
> that we're doing that in a later patch. But it feels beyond the scope of this 
> particular patch. Especially that we'd also need to go through the previous 
> code to copy the names of global records, enums, etc. anyway.
> 
> I feel like that the change to surface `APISet` and punt serialization should 
> be separated out from the macro change. And we can wrap all the name strings 
> in that patch so that it's a meaningful atomic commit.
Yeah sounds good!



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:375
+  case APIRecord::RK_ObjCIvar:
+Kind["identifier"] = AddLangPrefix("ivar");
+Kind["displayName"] = "Instance Variable";

zixuw wrote:
> dang wrote:
> > this should probably be more explicit to keep in line with how things are 
> > currently done. Maybe something like "instance.variable"
> Right, naming completely new things here so worth the discussion.
> To keep in line with the current convention, I don't see instance methods 
> having an `instance.` prefix. It's just `method` as opposed to `type.method`.
> Also global variable is just `var`, if we really need to add the `instance.` 
> prefix (which I still don't think makes much sense for the above reason), 
> wouldn't it be `instance.var`?
Actually, now that I think about it, we should just make them properties. There 
is already a precedent of doing that for struct fields, and ivars are pretty 
much the equivalent of a struct field for an interface.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122446

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


[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Hirochika Matsumoto via Phabricator via cfe-commits
hkmatsumoto updated this revision to Diff 418832.
hkmatsumoto added a comment.

Sync with main branch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

Files:
  llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
  llvm/test/Transforms/InstCombine/ispow2.ll

Index: llvm/test/Transforms/InstCombine/ispow2.ll
===
--- llvm/test/Transforms/InstCombine/ispow2.ll
+++ llvm/test/Transforms/InstCombine/ispow2.ll
@@ -740,10 +740,8 @@
 define i1 @is_pow2or0_ctpop(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 1
@@ -755,10 +753,8 @@
 define i1 @is_pow2or0_ctpop_swap_cmp(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_swap_cmp(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[CMP]], [[ISZERO]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 1
@@ -770,10 +766,8 @@
 define i1 @is_pow2or0_ctpop_logical(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_logical(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 1
@@ -785,10 +779,8 @@
 define <2 x i1> @is_pow2or0_ctpop_commute_vec(<2 x i8> %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_commute_vec(
 ; CHECK-NEXT:[[T0:%.*]] = tail call <2 x i8> @llvm.ctpop.v2i8(<2 x i8> [[X:%.*]])
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq <2 x i8> [[T0]], 
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq <2 x i8> [[X]], zeroinitializer
-; CHECK-NEXT:[[R:%.*]] = or <2 x i1> [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret <2 x i1> [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult <2 x i8> [[T0]], 
+; CHECK-NEXT:ret <2 x i1> [[TMP1]]
 ;
   %t0 = tail call <2 x i8> @llvm.ctpop.v2i8(<2 x i8> %x)
   %cmp = icmp eq <2 x i8> %t0, 
@@ -807,8 +799,8 @@
 ; CHECK-NEXT:call void @use_i1(i1 [[CMP]])
 ; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
 ; CHECK-NEXT:call void @use_i1(i1 [[ISZERO]])
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   call void @use(i32 %t0)
@@ -828,8 +820,8 @@
 ; CHECK-NEXT:call void @use_i1(i1 [[CMP]])
 ; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
 ; CHECK-NEXT:call void @use_i1(i1 [[ISZERO]])
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   call void @use(i32 %t0)
@@ -940,10 +932,8 @@
 define i1 @isnot_pow2nor0_ctpop(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2nor0_ctpop(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp ne i32 [[T0]], 1
-; CHECK-NEXT:[[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ugt i32 [[T0]], 1
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ne i32 %t0, 1
@@ -955,10 +945,8 @@
 define i1 @isnot_pow2nor0_ctpop_swap_cmp(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2nor0_ctpop_swap_cmp(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp ne i32 [[T0]], 1
-; CHECK-NEXT:[[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = and i1 [[CMP]], [[NOTZERO]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ugt i32 [[T0]], 1
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ne i32 %t0, 1
@@ -970,10 +958,8 @@
 define i1 @isnot_pow2nor0_ctpop_logical(i32 %x) {
 ; CHECK-LABEL:

[PATCH] D122544: Utilize comparison operation implemented in APInt

2022-03-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp:175
 // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: suspicious usage of sizeof 
pointer 'sizeof(T)/sizeof(T)'
-int Test3() { return Foo<42>() + Bar(); }
+template <__int128_t N> 
+bool Baz() { return sizeof(A) < N; }

aaronpuchert wrote:
> This causes test failures on 32-bit architectures:
> ```
> /home/tcwg-buildbot/worker/clang-armv7-2stage/stage2/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-sizeof-expression.cpp.tmp.cpp:175:11:
>  error: unknown type name '__int128_t' [clang-diagnostic-error]
> template <__int128_t N> 
>   ^
> ```
> 
> It's probably best to wrap this in `#ifdef __SIZEOF_INT128__` lest we disable 
> the entire test on the affected platforms.
Good catch! @SimplyDanny, can you fix up the test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122544

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


[PATCH] D122064: [clang-format][docs] Fix incorrect 'clang-format 11' option markers

2022-03-29 Thread Krystian Kuzniarek via Phabricator via cfe-commits
kuzkry added a comment.

Hi. Could you deliver this, please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122064

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


[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment.

In D122077#3413565 , @xbolva00 wrote:

> In D122077#3413550 , @joerg wrote:
>
>> Why is this fold preferable to `(X & (X-1)) == 0`? At least on all 
>> architectures without native population count, the binary-and based test is 
>> preferable and it might even be better with it.
>
> Less IR instructions. I think SDAG already expands some ctpop patterns to 
> logic (backends should decide about optimal form)

Correct - we try to convert the setcc(ctpop) pattern here:
https://github.com/llvm/llvm-project/blob/f1d8e46258c6a08ca1a375dc9670dd5581d6cf65/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp#L3772

That doesn't check whether the target has a popcount instruction for scalar 
types, so it is potentially too aggressive. For x86, we have a bonus 
optimization after that, so it should not show up there:
https://godbolt.org/z/oc16Kdhcf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

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


[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added inline comments.



Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:915
+/// Fold (icmp eq ctpop(X) 1) | (icmp eq X 0) into (icmp ult ctpop(X) 2) and
+/// fold (icmp ne ctpop(X) 1) & (icmp ne X 0) into (icmp uge ctpop(X) 2).
+static Value *foldIsPowerOf2OrZero(ICmpInst *Cmp0, ICmpInst *Cmp1, bool IsAnd,

Why create ">= 2" instead of "> 1" directly? 

I don't think it makes the transform or code any clearer with ">= 2", and we 
will always canonicalize to the other form, so I would prefer to go directly to 
the final result for efficiency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

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


[PATCH] D122446: [clang][extract-api] Add Objective-C interface support

2022-03-29 Thread Daniel Grumberg via Phabricator via cfe-commits
dang added inline comments.



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:642-648
+// Instantiate template for FunctionDecl.
+template FunctionSignature
+DeclarationFragmentsBuilder::getFunctionSignature(const FunctionDecl *);
+
+// Instantiate template for ObjCMethodDecl.
+template FunctionSignature
+DeclarationFragmentsBuilder::getFunctionSignature(const ObjCMethodDecl *);

Do we need to explicitly instantiate them? Wouldn't they get instantiated as 
needed?



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:245
+if (const auto *SuperClassDecl = Decl->getSuperClass()) {
+  SuperClass.Name = SuperClassDecl->getObjCRuntimeNameAsString();
+  SuperClass.USR = API.recordUSR(SuperClassDecl);

dang wrote:
> zixuw wrote:
> > dang wrote:
> > > Shouldn't we be recording this string in the StringAllocator for reuse 
> > > later. I have a patch in progress for macro support that will do the 
> > > serialization once the AST isn't available anymore so we really shouldn't 
> > > be taking in StringRefs for stuff in the AST.
> > Right but that only makes sense once we kill the AST before serialization 
> > right? I mean I'm happy to wrap these in `copyString` now if we know for 
> > sure that we're doing that in a later patch. But it feels beyond the scope 
> > of this particular patch. Especially that we'd also need to go through the 
> > previous code to copy the names of global records, enums, etc. anyway.
> > 
> > I feel like that the change to surface `APISet` and punt serialization 
> > should be separated out from the macro change. And we can wrap all the name 
> > strings in that patch so that it's a meaningful atomic commit.
> Yeah sounds good!
I have looked into it a bit more and the ASTContext will still be live at that 
point (it gets deallocated right after we will be doing the serialization) so 
this is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122446

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


[PATCH] D122648: [clang][extractapi] Tie API and serialization to the FrontendAction

2022-03-29 Thread Daniel Grumberg via Phabricator via cfe-commits
dang created this revision.
dang added reviewers: zixuw, ributzka, QuietMisdreavus.
Herald added a project: All.
dang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Make the API records a property of the action instead of the ASTVisitor
so that it can be accessed outside the AST visitation and push back
serialization to the end of the frontend action.

This will allow accessing and modifying the API records outside of the
ASTVisitor, which is a prerequisite for supporting macros.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122648

Files:
  clang/include/clang/ExtractAPI/FrontendActions.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -41,9 +41,8 @@
 /// information.
 class ExtractAPIVisitor : public RecursiveASTVisitor {
 public:
-  explicit ExtractAPIVisitor(ASTContext &Context)
-  : Context(Context),
-API(Context.getTargetInfo().getTriple(), Context.getLangOpts()) {}
+  ExtractAPIVisitor(ASTContext &Context, APISet &API)
+  : Context(Context), API(API) {}
 
   const APISet &getAPI() const { return API; }
 
@@ -304,42 +303,39 @@
   }
 
   ASTContext &Context;
-  APISet API;
+  APISet &API;
 };
 
 class ExtractAPIConsumer : public ASTConsumer {
 public:
-  ExtractAPIConsumer(ASTContext &Context, StringRef ProductName,
- std::unique_ptr OS)
-  : Visitor(Context), ProductName(ProductName), OS(std::move(OS)) {}
+  ExtractAPIConsumer(ASTContext &Context, APISet &API)
+  : Visitor(Context, API) {}
 
   void HandleTranslationUnit(ASTContext &Context) override {
 // Use ExtractAPIVisitor to traverse symbol declarations in the context.
 Visitor.TraverseDecl(Context.getTranslationUnitDecl());
-
-// Setup a SymbolGraphSerializer to write out collected API information in
-// the Symbol Graph format.
-// FIXME: Make the kind of APISerializer configurable.
-SymbolGraphSerializer SGSerializer(Visitor.getAPI(), ProductName);
-SGSerializer.serialize(*OS);
   }
 
 private:
   ExtractAPIVisitor Visitor;
-  std::string ProductName;
-  std::unique_ptr OS;
+};
 };
 
 } // namespace
 
 std::unique_ptr
 ExtractAPIAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
-  std::unique_ptr OS = CreateOutputFile(CI, InFile);
+  OS = CreateOutputFile(CI, InFile);
   if (!OS)
 return nullptr;
-  return std::make_unique(
-  CI.getASTContext(), CI.getInvocation().getFrontendOpts().ProductName,
-  std::move(OS));
+
+  ProductName = CI.getFrontendOpts().ProductName;
+
+  // Now that we have enough information about the language options and the
+  // target triple, let's create the APISet before anyone uses it.
+  API = std::make_unique(CI.getTarget().getTriple(), CI.getLangOpts());
+
+  return std::make_unique(CI.getASTContext(), *API);
 }
 
 bool ExtractAPIAction::PrepareToExecuteAction(CompilerInstance &CI) {
@@ -371,6 +367,18 @@
   return true;
 }
 
+void ExtractAPIAction::EndSourceFileAction() {
+  if (!OS)
+return;
+
+  // Setup a SymbolGraphSerializer to write out collected API information in
+  // the Symbol Graph format.
+  // FIXME: Make the kind of APISerializer configurable.
+  SymbolGraphSerializer SGSerializer(*API, ProductName);
+  SGSerializer.serialize(*OS);
+  OS->flush();
+}
+
 std::unique_ptr
 ExtractAPIAction::CreateOutputFile(CompilerInstance &CI, StringRef InFile) {
   std::unique_ptr OS =
Index: clang/include/clang/ExtractAPI/FrontendActions.h
===
--- clang/include/clang/ExtractAPI/FrontendActions.h
+++ clang/include/clang/ExtractAPI/FrontendActions.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_EXTRACTAPI_FRONTEND_ACTIONS_H
 #define LLVM_CLANG_EXTRACTAPI_FRONTEND_ACTIONS_H
 
+#include "clang/ExtractAPI/API.h"
 #include "clang/Frontend/FrontendAction.h"
 
 namespace clang {
@@ -25,11 +26,15 @@
  StringRef InFile) override;
 
 private:
-  /// The synthesized input buffer that contains all the provided input header
-  /// files.
-  std::unique_ptr Buffer;
+  /// A representation of the APIs this action extracts.
+  std::unique_ptr API;
+
+  /// A stream to the output file of this action.
+  std::unique_ptr OS;
+
+  /// The product this action is extracting API information for.
+  std::string ProductName;
 
-public:
   /// Prepare to execute the action on the given CompilerInstance.
   ///
   /// This is called before executing the action on any inputs. This generates a
@@ -37,8 +42,15 @@
   /// list with it before actually executing the action.
   bool PrepareToExecuteAction(CompilerInstance &CI) override;
 
+  /// Called after executing the action on the synthesized input buffer.
+  ///
+  /// Note: Now that we have gath

[PATCH] D122556: [RISCV] Add definitions for Xiangshan processors.

2022-03-29 Thread luxufan via Phabricator via cfe-commits
StephenFan added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCV.td:547
+def : ProcessorModel<"xiangshan-nanhu", NoSchedModel, [Feature64Bit,
+   FeatureStdExtM,
+   FeatureStdExtA,

The document says that `xiangshan-nanhu` cpu support 
`RV64IMAFDC_zba_zbb_zbc_zbs_zbkb_zbkc_zbkx_zknd_zkne_zknh_zksed_zksh_svinval` . 
And it seems that `svinval` extension is not supported by llvm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122556

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


[PATCH] D122446: [clang][extract-api] Add Objective-C interface support

2022-03-29 Thread Daniel Grumberg via Phabricator via cfe-commits
dang added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:245
+if (const auto *SuperClassDecl = Decl->getSuperClass()) {
+  SuperClass.Name = SuperClassDecl->getObjCRuntimeNameAsString();
+  SuperClass.USR = API.recordUSR(SuperClassDecl);

dang wrote:
> dang wrote:
> > zixuw wrote:
> > > dang wrote:
> > > > Shouldn't we be recording this string in the StringAllocator for reuse 
> > > > later. I have a patch in progress for macro support that will do the 
> > > > serialization once the AST isn't available anymore so we really 
> > > > shouldn't be taking in StringRefs for stuff in the AST.
> > > Right but that only makes sense once we kill the AST before serialization 
> > > right? I mean I'm happy to wrap these in `copyString` now if we know for 
> > > sure that we're doing that in a later patch. But it feels beyond the 
> > > scope of this particular patch. Especially that we'd also need to go 
> > > through the previous code to copy the names of global records, enums, 
> > > etc. anyway.
> > > 
> > > I feel like that the change to surface `APISet` and punt serialization 
> > > should be separated out from the macro change. And we can wrap all the 
> > > name strings in that patch so that it's a meaningful atomic commit.
> > Yeah sounds good!
> I have looked into it a bit more and the ASTContext will still be live at 
> that point (it gets deallocated right after we will be doing the 
> serialization) so this is fine.
I thought it would still make sense to split the macro patch in two so here we 
go: https://reviews.llvm.org/D122648


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122446

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


[PATCH] D122446: [clang][extract-api] Add Objective-C interface support

2022-03-29 Thread Daniel Grumberg via Phabricator via cfe-commits
dang requested changes to this revision.
dang added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:660
+
+// Subheading of an Objective-C method is a `+` or `-` sign indicating whether
+// it's a class method or an instance method, followed by the selector name.

We shouldn't be appending the +/- here. We can discuss this more offline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122446

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


[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

2022-03-29 Thread Matthias Gehre via Phabricator via cfe-commits
mgehre-amd updated this revision to Diff 418861.
mgehre-amd added a comment.
Herald added a subscriber: dexonsmith.

- Remove diagnostics for float-to-bitint
- Remove lexer changes (will be another PR)
- Add -fexperimental-max-bitint-width=N


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122234

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/Driver/linux-ld.c
  clang/test/Sema/large-bit-int.c

Index: clang/test/Sema/large-bit-int.c
===
--- /dev/null
+++ clang/test/Sema/large-bit-int.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fexperimental-max-bitint-width=1024 -fsyntax-only -verify %s -Wno-unused -triple x86_64-gnu-linux
+
+void f() {
+  _Static_assert(__BITINT_MAXWIDTH__ == 1024, "Macro value is unexpected.");
+
+  _BitInt(1024) a;
+  unsigned _BitInt(1024) b;
+  
+  _BitInt(8388609) c;// expected-error {{signed _BitInt of bit sizes greater than 1024 not supported}}
+  unsigned _BitInt(0xFF) d; // expected-error {{unsigned _BitInt of bit sizes greater than 1024 not supported}}
+}
Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -174,7 +174,7 @@
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANG-NO-LIBGCC-STATIC %s
 // CHECK-CLANG-NO-LIBGCC-STATIC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
-// CHECK-CLANG-NO-LIBGCC-STATIC: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+// CHECK-CLANG-NO-LIBGCC-STATIC: "--start-group" "{{[^"]*}}/libclang_rt.bitint{{[^"]*}}" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
 // RUN: %clang -static-pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform --unwindlib=platform \
@@ -189,7 +189,7 @@
 // CHECK-CLANG-LD-STATIC-PIE: "text"
 // CHECK-CLANG-LD-STATIC-PIE: "-m" "elf_x86_64"
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
-// CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+// CHECK-CLANG-LD-STATIC-PIE: "--start-group" "{{[^"]*}}/libclang_rt.bitint{{[^"]*}}" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
 // RUN: %clang -static-pie -pie -no-canonical-prefixes %s -no-pie -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform --unwindlib=platform \
@@ -204,7 +204,7 @@
 // CHECK-CLANG-LD-STATIC-PIE-PIE: "text"
 // CHECK-CLANG-LD-STATIC-PIE-PIE: "-m" "elf_x86_64"
 // CHECK-CLANG-LD-STATIC-PIE-PIE: "{{.*}}rcrt1.o"
-// CHECK-CLANG-LD-STATIC-PIE-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+// CHECK-CLANG-LD-STATIC-PIE-PIE: "--start-group" "{{[^"]*}}/libclang_rt.bitint{{[^"]*}}" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
 // RUN: %clang -static-pie -static -no-canonical-prefixes %s -no-pie -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform --unwindlib=platform \
@@ -219,7 +219,7 @@
 // CHECK-CLANG-LD-STATIC-PIE-STATIC: "text"
 // CHECK-CLANG-LD-STATIC-PIE-STATIC: "-m" "elf_x86_64"
 // CHECK-CLANG-LD-STATIC-PIE-STATIC: "{{.*}}rcrt1.o"
-// CHECK-CLANG-LD-STATIC-PIE-STATIC: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+// CHECK-CLANG-LD-STATIC-PIE-STATIC: "--start-group" "{{[^"]*}}/libclang_rt.bitint{{[^"]*}}" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
 // RUN: %clang -static-pie -nopie -no-canonical-prefixes %s -no-pie -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
@@ -318,7 +318,7 @@
 // CHECK-LD-64-STATIC: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-unknown-linux/10.2.0/../../../../x86_64-unknown-linux/lib"
 // CHECK-LD-64-STATIC: "-L[[SYSROOT]]/lib"
 // CHECK-LD-64-STATIC: "-L[[SYSROOT]]/usr/lib"
-// CHECK-LD-64-STATIC: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+// CHECK-LD-64-STATIC: "--start-group" "{{[^"]*}}/libclang_rt.bitint{{[^"]*}}" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 
 // RUN: %clang -no-pie -### %s --target=x86_64-unknown-linux -rtlib=platform --unwindlib=platform -shared -static \
 // RUN:   --gcc-toolchain= --sysroot=%S/Inputs/basic_linux_tree 2>&1 | FileCheck --check-prefix=CHECK-LD-SHARED-STATIC %s
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -312,7 +312,7 @@
 
 #define BENIGN_LANGOPT(Name, Bits, Default, Description)
 #define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description)
-#def

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

2022-03-29 Thread Matthias Gehre via Phabricator via cfe-commits
mgehre-amd added inline comments.



Comment at: clang/lib/Serialization/ASTReader.cpp:315
 #define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description)
-#define BENIGN_VALUE_LANGOPT(Name, Type, Bits, Default, Description)
+#define BENIGN_VALUE_LANGOPT(Name, Bits, Default, Description)
 #include "clang/Basic/LangOptions.def"

This macro was taking the wrong number of arguments, but nobody noticed because 
there was no `BENIGN_VALUE_LANGOPT` before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122234

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


[PATCH] D122589: Additionally set f32 mode with denormal-fp-math

2022-03-29 Thread David Candler via Phabricator via cfe-commits
dcandler updated this revision to Diff 418865.
dcandler added a comment.

Added a test that checks attributes based on the -fdenormal-fp-math and 
-fdenormal-fp-math-f32 flags.

Only the cases where -fdenormal-fp-math is set to preserve-sign or 
positive-zero and -fdenormal-fp-math-f32 is unset are changed by this patch. 
Previously, these would still result in a "denormal-fp-math-f32"="ieee,ieee" 
attribute.


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

https://reviews.llvm.org/D122589

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/denormalfpmode-f32.c


Index: clang/test/CodeGen/denormalfpmode-f32.c
===
--- /dev/null
+++ clang/test/CodeGen/denormalfpmode-f32.c
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -S %s -emit-llvm -o - | FileCheck %s 
--check-prefixes=CHECK-ATTR,CHECK-NONE,CHECK-F32-NONE
+// RUN: %clang_cc1 -S -fdenormal-fp-math=ieee %s -emit-llvm -o - | FileCheck 
%s --check-prefixes=CHECK-ATTR,CHECK-NONE,CHECK-F32-NONE
+// RUN: %clang_cc1 -S -fdenormal-fp-math=preserve-sign %s -emit-llvm -o - | 
FileCheck %s --check-prefixes=CHECK-ATTR,CHECK-PS,CHECK-F32-NONE
+// RUN: %clang_cc1 -S -fdenormal-fp-math=positive-zero %s -emit-llvm -o - | 
FileCheck %s --check-prefixes=CHECK-ATTR,CHECK-PZ,CHECK-F32-NONE
+
+// RUN: %clang_cc1 -S -fdenormal-fp-math-f32=ieee %s -emit-llvm -o - | 
FileCheck %s --check-prefixes=CHECK-ATTR,CHECK-NONE,CHECK-F32-NONE
+// RUN: %clang_cc1 -S -fdenormal-fp-math=ieee -fdenormal-fp-math-f32=ieee %s 
-emit-llvm -o - | FileCheck %s 
--check-prefixes=CHECK-ATTR,CHECK-NONE,CHECK-F32-NONE
+// RUN: %clang_cc1 -S -fdenormal-fp-math=preserve-sign 
-fdenormal-fp-math-f32=ieee %s -emit-llvm -o - | FileCheck %s 
--check-prefixes=CHECK-ATTR,CHECK-PS,CHECK-F32-IEEE
+// RUN: %clang_cc1 -S -fdenormal-fp-math=positive-zero 
-fdenormal-fp-math-f32=ieee %s -emit-llvm -o - | FileCheck %s 
--check-prefixes=CHECK-ATTR,CHECK-PZ,CHECK-F32-IEEE
+
+// RUN: %clang_cc1 -S -fdenormal-fp-math-f32=preserve-sign %s -emit-llvm -o - 
| FileCheck %s --check-prefixes=CHECK-ATTR,CHECK-NONE,CHECK-F32-PS
+// RUN: %clang_cc1 -S -fdenormal-fp-math=ieee 
-fdenormal-fp-math-f32=preserve-sign %s -emit-llvm -o - | FileCheck %s 
--check-prefixes=CHECK-ATTR,CHECK-NONE,CHECK-F32-PS
+// RUN: %clang_cc1 -S -fdenormal-fp-math=preserve-sign 
-fdenormal-fp-math-f32=preserve-sign %s -emit-llvm -o - | FileCheck %s 
--check-prefixes=CHECK-ATTR,CHECK-PS,CHECK-F32-NONE
+// RUN: %clang_cc1 -S -fdenormal-fp-math=positive-zero 
-fdenormal-fp-math-f32=preserve-sign %s -emit-llvm -o - | FileCheck %s 
--check-prefixes=CHECK-ATTR,CHECK-PZ,CHECK-F32-PS
+
+// RUN: %clang_cc1 -S -fdenormal-fp-math-f32=positive-zero %s -emit-llvm -o - 
| FileCheck %s --check-prefixes=CHECK-ATTR,CHECK-NONE,CHECK-F32-PZ
+// RUN: %clang_cc1 -S -fdenormal-fp-math=ieee 
-fdenormal-fp-math-f32=positive-zero %s -emit-llvm -o - | FileCheck %s 
--check-prefixes=CHECK-ATTR,CHECK-NONE,CHECK-F32-PZ
+// RUN: %clang_cc1 -S -fdenormal-fp-math=preserve-sign 
-fdenormal-fp-math-f32=positive-zero %s -emit-llvm -o - | FileCheck %s 
--check-prefixes=CHECK-ATTR,CHECK-PS,CHECK-F32-PZ
+// RUN: %clang_cc1 -S -fdenormal-fp-math=positive-zero 
-fdenormal-fp-math-f32=positive-zero %s -emit-llvm -o - | FileCheck %s 
--check-prefixes=CHECK-ATTR,CHECK-PZ,CHECK-F32-NONE
+
+// CHECK-LABEL: main
+
+// CHECK-ATTR: attributes #0 =
+// CHECK-NONE-NOT:"denormal-fp-math"
+// CHECK-IEEE: "denormal-fp-math"="ieee,ieee"
+// CHECK-PS: "denormal-fp-math"="preserve-sign,preserve-sign"
+// CHECK-PZ: "denormal-fp-math"="positive-zero,positive-zero"
+// CHECK-F32-NONE-NOT:"denormal-fp-math-f32"
+// CHECK-F32-IEEE: "denormal-fp-math-f32"="ieee,ieee"
+// CHECK-F32-PS: "denormal-fp-math-f32"="preserve-sign,preserve-sign"
+// CHECK-F32-PZ: "denormal-fp-math-f32"="positive-zero,positive-zero"
+
+int main(void) {
+  return 0;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1497,7 +1497,8 @@
   if (Opts.FPDenormalMode != llvm::DenormalMode::getIEEE())
 GenerateArg(Args, OPT_fdenormal_fp_math_EQ, Opts.FPDenormalMode.str(), SA);
 
-  if (Opts.FP32DenormalMode != llvm::DenormalMode::getIEEE())
+  if ((Opts.FPDenormalMode != Opts.FP32DenormalMode) ||
+  (Opts.FP32DenormalMode != llvm::DenormalMode::getIEEE()))
 GenerateArg(Args, OPT_fdenormal_fp_math_f32_EQ, 
Opts.FP32DenormalMode.str(),
 SA);
 
@@ -1852,6 +1853,7 @@
   if (Arg *A = Args.getLastArg(OPT_fdenormal_fp_math_EQ)) {
 StringRef Val = A->getValue();
 Opts.FPDenormalMode = llvm::parseDenormalFPAttribute(Val);
+Opts.FP32DenormalMode = Opts.FPDenormalMode;
 if (!Opts.FPDenormalMode.isValid())
   Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
   }
Index: clang/lib/Driver/ToolChains/C

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Hirochika Matsumoto via Phabricator via cfe-commits
hkmatsumoto added inline comments.



Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:915
+/// Fold (icmp eq ctpop(X) 1) | (icmp eq X 0) into (icmp ult ctpop(X) 2) and
+/// fold (icmp ne ctpop(X) 1) & (icmp ne X 0) into (icmp uge ctpop(X) 2).
+static Value *foldIsPowerOf2OrZero(ICmpInst *Cmp0, ICmpInst *Cmp1, bool IsAnd,

spatel wrote:
> Why create ">= 2" instead of "> 1" directly? 
> 
> I don't think it makes the transform or code any clearer with ">= 2", and we 
> will always canonicalize to the other form, so I would prefer to go directly 
> to the final result for efficiency.
You're right. I thought it is fine to let another pass do further 
transformation but in retrospect, folding >= 2 to > 1 is so trivial that we 
should do it here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

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


[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Hirochika Matsumoto via Phabricator via cfe-commits
hkmatsumoto updated this revision to Diff 418869.
hkmatsumoto added a comment.

Create ctpop(X) > 1 instead of ctpop(X) >= 2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

Files:
  llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
  llvm/test/Transforms/InstCombine/ispow2.ll

Index: llvm/test/Transforms/InstCombine/ispow2.ll
===
--- llvm/test/Transforms/InstCombine/ispow2.ll
+++ llvm/test/Transforms/InstCombine/ispow2.ll
@@ -740,10 +740,8 @@
 define i1 @is_pow2or0_ctpop(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 1
@@ -755,10 +753,8 @@
 define i1 @is_pow2or0_ctpop_swap_cmp(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_swap_cmp(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[CMP]], [[ISZERO]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 1
@@ -770,10 +766,8 @@
 define i1 @is_pow2or0_ctpop_logical(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_logical(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 1
@@ -785,10 +779,8 @@
 define <2 x i1> @is_pow2or0_ctpop_commute_vec(<2 x i8> %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_commute_vec(
 ; CHECK-NEXT:[[T0:%.*]] = tail call <2 x i8> @llvm.ctpop.v2i8(<2 x i8> [[X:%.*]])
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq <2 x i8> [[T0]], 
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq <2 x i8> [[X]], zeroinitializer
-; CHECK-NEXT:[[R:%.*]] = or <2 x i1> [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret <2 x i1> [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult <2 x i8> [[T0]], 
+; CHECK-NEXT:ret <2 x i1> [[TMP1]]
 ;
   %t0 = tail call <2 x i8> @llvm.ctpop.v2i8(<2 x i8> %x)
   %cmp = icmp eq <2 x i8> %t0, 
@@ -807,8 +799,8 @@
 ; CHECK-NEXT:call void @use_i1(i1 [[CMP]])
 ; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
 ; CHECK-NEXT:call void @use_i1(i1 [[ISZERO]])
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   call void @use(i32 %t0)
@@ -828,8 +820,8 @@
 ; CHECK-NEXT:call void @use_i1(i1 [[CMP]])
 ; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
 ; CHECK-NEXT:call void @use_i1(i1 [[ISZERO]])
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   call void @use(i32 %t0)
@@ -940,10 +932,8 @@
 define i1 @isnot_pow2nor0_ctpop(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2nor0_ctpop(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp ne i32 [[T0]], 1
-; CHECK-NEXT:[[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ugt i32 [[T0]], 1
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ne i32 %t0, 1
@@ -955,10 +945,8 @@
 define i1 @isnot_pow2nor0_ctpop_swap_cmp(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2nor0_ctpop_swap_cmp(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp ne i32 [[T0]], 1
-; CHECK-NEXT:[[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = and i1 [[CMP]], [[NOTZERO]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ugt i32 [[T0]], 1
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ne i32 %t0, 1
@@ -970,10 +958,8 @@
 define i1 @isnot_pow2nor0_ctpop_logical(i3

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Sanjay Patel via Phabricator via cfe-commits
spatel accepted this revision.
spatel 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/D122077/new/

https://reviews.llvm.org/D122077

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


[PATCH] D122656: [C11] Improve the diagnostic when accessing a member of an atomic struct

2022-03-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.
aaron.ballman added reviewers: eli.friedman, rjmccall, jyknight, erichkeane, 
clang-language-wg.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

Member access for an atomic structure or union is unconditional undefined 
behavior (C11 6.5.2.3p5). However, we would issue a confusing error message 
about the base expression not being a structure or union type.

GCC issues a warning for this case. Clang now warns as well, but the warning is 
defaulted to an error because the actual access is still unsafe.

This fixes Issue 54563.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122656

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExprMember.cpp
  clang/test/Sema/atomic-expr.c


Index: clang/test/Sema/atomic-expr.c
===
--- clang/test/Sema/atomic-expr.c
+++ clang/test/Sema/atomic-expr.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -verify=off -fsyntax-only -Wno-atomic-access
+// off-no-diagnostics
 
 _Atomic(unsigned int) data1;
 int _Atomic data2;
@@ -75,3 +76,18 @@
   _Static_assert(__builtin_types_compatible_p(__typeof__(x = 2), int), 
"incorrect");
   _Static_assert(__builtin_types_compatible_p(__typeof__(x += 2), int), 
"incorrect");
 }
+
+// Ensure that member access of an atomic structure or union type is properly
+// diagnosed as being undefined behavior; Issue 54563.
+void func_16(void) {
+  _Atomic struct { int val; } x, *xp;
+  x.val = 12; // expected-error {{accessing a member of an atomic structure or 
union is undefined behavior}}
+  xp->val = 12; // expected-error {{accessing a member of an atomic structure 
or union is undefined behavior}}
+
+  _Atomic union {
+int ival;
+float fval;
+  } y, *yp;
+  y.ival = 12;   // expected-error {{accessing a member of an atomic structure 
or union is undefined behavior}}
+  yp->fval = 1.2f; // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+}
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1292,6 +1292,20 @@
 }
   }
 
+  // If the base type is an atomic type, this access is undefined behavior per
+  // C11 6.5.2.3p5. Instead of giving a typecheck error, we'll warn the user
+  // about the UB and recover by converting the atomic lvalue into a non-atomic
+  // lvalue. Because this is inherently unsafe as an atomic operation, the
+  // warning defaults to an error.
+  if (const auto *ATy = BaseType->getAs()) {
+S.Diag(OpLoc, diag::warn_atomic_member_access);
+BaseType = ATy->getValueType().getUnqualifiedType();
+BaseExpr = ImplicitCastExpr::Create(
+S.Context, IsArrow ? S.Context.getPointerType(BaseType) : BaseType,
+CK_AtomicToNonAtomic, BaseExpr.get(), nullptr,
+BaseExpr.get()->getValueKind(), FPOptionsOverride());
+  }
+
   // Handle field access to simple records.
   if (const RecordType *RTy = BaseType->getAs()) {
 TypoExpr *TE = nullptr;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6439,6 +6439,9 @@
 "%select{incomplete |array |function |reference |atomic |qualified 
"
 "|sizeless ||integer }0type "
 "%1 %select{|||which is not trivially copyable|}0">;
+def warn_atomic_member_access : Warning<
+  "accessing a member of an atomic structure or union is undefined behavior">,
+  InGroup>, DefaultError;
 
 // Expressions.
 def ext_sizeof_alignof_function_type : Extension<
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -85,6 +85,10 @@
 - Assignment expressions in C11 and later mode now properly strip the _Atomic
   qualifier when determining the type of the assignment expression. Fixes
   `Issue 48742 `_.
+- Improved the diagnostic when accessing a member of an atomic structure or
+  union object in C; was previously an unhelpful error, but now issues a
+  `-Watomic-access` warning which defaults to an error. Fixes
+  `Issue 54563 `_.
 - Unevaluated lambdas in dependant contexts no longer result in clang crashing.
   This fixes Issues `50376 
`_,
   `51414 `_,


Index: clang/test/Sema/atomic-expr.c
===
--- clang/t

[PATCH] D122069: [Object] Add binary format for bundling offloading metadata

2022-03-29 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added reviewers: grokos, ABataev, ronlieb, tianshilei1992.
JonChesterfield added a comment.

Added some reviewers. I'd much prefer this used an existing binary format, DIY 
is prone to errors and maintenance hassles down the road. Don't care as much 
about which format as about it being one with an existing, tested 
implementation and ideally existing inspection tools.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122069

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


[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Hirochika Matsumoto via Phabricator via cfe-commits
hkmatsumoto added a comment.

@spatel Since I don't have commit access, can you land this patch?
Please use "Hirochika Matsumoto  as my identity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

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


[PATCH] D122656: [C11] Improve the diagnostic when accessing a member of an atomic struct

2022-03-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaExprMember.cpp:1300
+  // warning defaults to an error.
+  if (const auto *ATy = BaseType->getAs()) {
+S.Diag(OpLoc, diag::warn_atomic_member_access);

This seems to apply to both C and C++.  I guess "_Atomic" is C only, so we get 
to define its behavior for C++?

What does GCC do in C++ mode?



Comment at: clang/test/Sema/atomic-expr.c:85
+  x.val = 12; // expected-error {{accessing a member of an atomic structure or 
union is undefined behavior}}
+  xp->val = 12; // expected-error {{accessing a member of an atomic structure 
or union is undefined behavior}}
+

This still catches RHS access as well, right?  

Also, does it still 'work' with the non-qualifier version of _Atomic?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122656

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


[clang] 5b6207f - [ADT] Flesh out HLSL raytracing environments

2022-03-29 Thread Chris Bieneman via cfe-commits

Author: Chris Bieneman
Date: 2022-03-29T09:43:03-05:00
New Revision: 5b6207f3cd5e266f3fda8085fcebe6dcf45dacbf

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

LOG: [ADT] Flesh out HLSL raytracing environments

Fleshing this out now allows me to rely on enum math to translate
values rather than having to translate the off cases.

I should have added this in the first pass, but wasn't thinking about
it.

Added: 


Modified: 
clang/lib/Frontend/InitPreprocessor.cpp
llvm/include/llvm/ADT/Triple.h
llvm/lib/Support/Triple.cpp
llvm/unittests/ADT/TripleTest.cpp

Removed: 




diff  --git a/clang/lib/Frontend/InitPreprocessor.cpp 
b/clang/lib/Frontend/InitPreprocessor.cpp
index 89e9cb0250043..011a6743214a7 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -401,11 +401,6 @@ static void InitializeStandardPredefinedMacros(const 
TargetInfo &TI,
 uint32_t StageInteger = StageInteger =
 (uint32_t)TI.getTriple().getEnvironment() -
 (uint32_t)llvm::Triple::Pixel;
-// TODO: When we add raytracing support we can clean this up
-if (TI.getTriple().getEnvironment() == llvm::Triple::Mesh)
-  StageInteger = (uint32_t)ShaderStage::Mesh;
-else if (TI.getTriple().getEnvironment() == llvm::Triple::Amplification)
-  StageInteger = (uint32_t)ShaderStage::Amplification;
 
 Builder.defineMacro("__SHADER_TARGET_STAGE", Twine(StageInteger));
 // Add target versions

diff  --git a/llvm/include/llvm/ADT/Triple.h b/llvm/include/llvm/ADT/Triple.h
index 99ddfca897e63..728b24e235769 100644
--- a/llvm/include/llvm/ADT/Triple.h
+++ b/llvm/include/llvm/ADT/Triple.h
@@ -246,6 +246,12 @@ class Triple {
 Domain,
 Compute,
 Library,
+RayGeneration,
+Intersection,
+AnyHit,
+ClosestHit,
+Miss,
+Callable,
 Mesh,
 Amplification,
 

diff  --git a/llvm/lib/Support/Triple.cpp b/llvm/lib/Support/Triple.cpp
index 2cde1c69b2f41..ef5152eb302bf 100644
--- a/llvm/lib/Support/Triple.cpp
+++ b/llvm/lib/Support/Triple.cpp
@@ -275,6 +275,12 @@ StringRef Triple::getEnvironmentTypeName(EnvironmentType 
Kind) {
   case Domain: return "domain";
   case Compute: return "compute";
   case Library: return "library";
+  case RayGeneration: return "raygeneration";
+  case Intersection: return "intersection";
+  case AnyHit: return "anyhit";
+  case ClosestHit: return "closesthit";
+  case Miss: return "miss";
+  case Callable: return "callable";
   case Mesh: return "mesh";
   case Amplification: return "amplification";
   }
@@ -608,6 +614,12 @@ static Triple::EnvironmentType parseEnvironment(StringRef 
EnvironmentName) {
   .StartsWith("domain", Triple::Domain)
   .StartsWith("compute", Triple::Compute)
   .StartsWith("library", Triple::Library)
+  .StartsWith("raygeneration", Triple::RayGeneration)
+  .StartsWith("intersection", Triple::Intersection)
+  .StartsWith("anyhit", Triple::AnyHit)
+  .StartsWith("closesthit", Triple::ClosestHit)
+  .StartsWith("miss", Triple::Miss)
+  .StartsWith("callable", Triple::Callable)
   .StartsWith("mesh", Triple::Mesh)
   .StartsWith("amplification", Triple::Amplification)
   .Default(Triple::UnknownEnvironment);
@@ -1902,3 +1914,19 @@ static_assert(Triple::Compute - Triple::Pixel == 5,
   "incorrect HLSL stage order");
 static_assert(Triple::Library - Triple::Pixel == 6,
   "incorrect HLSL stage order");
+static_assert(Triple::RayGeneration - Triple::Pixel == 7,
+  "incorrect HLSL stage order");
+static_assert(Triple::Intersection - Triple::Pixel == 8,
+  "incorrect HLSL stage order");
+static_assert(Triple::AnyHit - Triple::Pixel == 9,
+  "incorrect HLSL stage order");
+static_assert(Triple::ClosestHit - Triple::Pixel == 10,
+  "incorrect HLSL stage order");
+static_assert(Triple::Miss - Triple::Pixel == 11,
+  "incorrect HLSL stage order");
+static_assert(Triple::Callable - Triple::Pixel == 12,
+  "incorrect HLSL stage order");
+static_assert(Triple::Mesh - Triple::Pixel == 13,
+  "incorrect HLSL stage order");
+static_assert(Triple::Amplification - Triple::Pixel == 14,
+  "incorrect HLSL stage order");

diff  --git a/llvm/unittests/ADT/TripleTest.cpp 
b/llvm/unittests/ADT/TripleTest.cpp
index 3824769a823ab..ca3163f43883e 100644
--- a/llvm/unittests/ADT/TripleTest.cpp
+++ b/llvm/unittests/ADT/TripleTest.cpp
@@ -662,6 +662,42 @@ TEST(TripleTest, ParsedIDs) {
   EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
   EXPECT_EQ(Triple::ShaderModel, T.getOS());
   EXPECT_EQ(Triple::Library, T.getEnvironment());
+
+  T = Triple("dxil-unknown-shadermodel-raygeneration");
+  EXPECT_EQ(Triple::dxil, 

[PATCH] D122657: [NFC] Use range based loop.

2022-03-29 Thread Jun Zhang via Phabricator via cfe-commits
junaire created this revision.
junaire added reviewers: v.g.vassilev, rsmith.
Herald added a project: All.
junaire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Jun Zhang 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122657

Files:
  clang/lib/AST/ASTDiagnostic.cpp


Index: clang/lib/AST/ASTDiagnostic.cpp
===
--- clang/lib/AST/ASTDiagnostic.cpp
+++ clang/lib/AST/ASTDiagnostic.cpp
@@ -270,9 +270,9 @@
   std::string S = Ty.getAsString(Context.getPrintingPolicy());
   std::string CanS = CanTy.getAsString(Context.getPrintingPolicy());
 
-  for (unsigned I = 0, E = QualTypeVals.size(); I != E; ++I) {
+  for (const intptr_t &QualTypeVal : QualTypeVals) {
 QualType CompareTy =
-QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVals[I]));
+QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVal));
 if (CompareTy.isNull())
   continue;
 if (CompareTy == Ty)
@@ -302,10 +302,10 @@
   // Check to see if we already desugared this type in this
   // diagnostic.  If so, don't do it again.
   bool Repeated = false;
-  for (unsigned i = 0, e = PrevArgs.size(); i != e; ++i) {
+  for (auto &PrevArg : PrevArgs) {
 // TODO: Handle ak_declcontext case.
-if (PrevArgs[i].first == DiagnosticsEngine::ak_qualtype) {
-  void *Ptr = (void*)PrevArgs[i].second;
+if (PrevArg.first == DiagnosticsEngine::ak_qualtype) {
+  void *Ptr = (void *)PrevArg.second;
   QualType PrevTy(QualType::getFromOpaquePtr(Ptr));
   if (PrevTy == Ty) {
 Repeated = true;


Index: clang/lib/AST/ASTDiagnostic.cpp
===
--- clang/lib/AST/ASTDiagnostic.cpp
+++ clang/lib/AST/ASTDiagnostic.cpp
@@ -270,9 +270,9 @@
   std::string S = Ty.getAsString(Context.getPrintingPolicy());
   std::string CanS = CanTy.getAsString(Context.getPrintingPolicy());
 
-  for (unsigned I = 0, E = QualTypeVals.size(); I != E; ++I) {
+  for (const intptr_t &QualTypeVal : QualTypeVals) {
 QualType CompareTy =
-QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVals[I]));
+QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVal));
 if (CompareTy.isNull())
   continue;
 if (CompareTy == Ty)
@@ -302,10 +302,10 @@
   // Check to see if we already desugared this type in this
   // diagnostic.  If so, don't do it again.
   bool Repeated = false;
-  for (unsigned i = 0, e = PrevArgs.size(); i != e; ++i) {
+  for (auto &PrevArg : PrevArgs) {
 // TODO: Handle ak_declcontext case.
-if (PrevArgs[i].first == DiagnosticsEngine::ak_qualtype) {
-  void *Ptr = (void*)PrevArgs[i].second;
+if (PrevArg.first == DiagnosticsEngine::ak_qualtype) {
+  void *Ptr = (void *)PrevArg.second;
   QualType PrevTy(QualType::getFromOpaquePtr(Ptr));
   if (PrevTy == Ty) {
 Repeated = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

2022-03-29 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 418876.
balazske added a comment.

rebase, applied the review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118996

Files:
  clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-signal-handler.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-msc54-cpp.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-signal-handler/stdcpp.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c
  clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.cpp
@@ -0,0 +1,228 @@
+// RUN: %check_clang_tidy -std=c++14 %s bugprone-signal-handler %t -- -- -isystem %S/Inputs/Headers -isystem %S/Inputs/bugprone-signal-handler
+
+#include "stdcpp.h"
+#include "stdio.h"
+
+// Functions called "signal" that are different from the system version.
+typedef void (*callback_t)(int);
+void signal(int, callback_t, int);
+namespace ns {
+void signal(int, callback_t);
+}
+
+extern "C" void handler_unsafe(int) {
+  printf("xxx");
+}
+
+extern "C" void handler_unsafe_1(int) {
+  printf("xxx");
+}
+
+namespace test_invalid_handler {
+
+void handler_non_extern_c(int) {
+  printf("xxx");
+}
+
+struct A {
+  static void handler_member(int) {
+printf("xxx");
+  }
+};
+
+void test() {
+  std::signal(SIGINT, handler_unsafe_1);
+  // CHECK-MESSAGES: :[[@LINE-17]]:3: warning: standard function 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:23: note: function 'handler_unsafe_1' registered here as signal handler
+
+  std::signal(SIGINT, handler_non_extern_c);
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: functions with other than C linkage are not allowed as signal handler (until C++17) [bugprone-signal-handler]
+  std::signal(SIGINT, A::handler_member);
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: functions with other than C linkage are not allowed as signal handler (until C++17) [bugprone-signal-handler]
+  std::signal(SIGINT, [](int) { printf("xxx"); });
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: lambda function is not allowed as signal handler (until C++17) [bugprone-signal-handler]
+
+  // This case is (deliberately) not found by the checker.
+  std::signal(SIGINT, [](int) -> callback_t { return &handler_unsafe; }(1));
+}
+
+} // namespace test_invalid_handler
+
+namespace test_non_standard_signal_call {
+
+struct Signal {
+  static void signal(int, callback_t);
+};
+
+void test() {
+  // No diagnostics here. All these signal calls differ from the standard system one.
+  signal(SIGINT, handler_unsafe, 1);
+  ns::signal(SIGINT, handler_unsafe);
+  Signal::signal(SIGINT, handler_unsafe);
+  system_other::signal(SIGINT, handler_unsafe);
+}
+
+} // namespace test_non_standard_signal_call
+
+namespace test_cpp_construct_in_handler {
+
+struct Struct {
+  virtual ~Struct() {}
+  void f1();
+  int *begin();
+  int *end();
+  static void f2();
+};
+struct Derived : public Struct {
+};
+
+struct X {
+  X(int, float);
+};
+
+Struct *S_Global;
+const Struct *S_GlobalConst;
+
+void f_non_extern_c() {
+}
+
+void f_default_arg(int P1 = 0) {
+}
+
+extern "C" void handler_cpp(int) {
+  using namespace ::test_cpp_construct_in_handler;
+
+  // These calls are not found as problems.
+  // (Called functions are not analyzed if the current function has already
+  // other problems.)
+  f_non_extern_c();
+  Struct::f2();
+  // 'auto' is not disallowed
+  auto Auto = 28u;
+
+  Struct S;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: remark: internally, the statement is parsed as a 'CXXConstructExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  S_Global->f1();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: remark: internally, the statement is parsed as a 'CXXMemberCallExpr'
+  // CHECK-MESSAGES: :198:23: note: function 'handler_cpp' registered here as signal handler
+  const Struct &SRef = Struct();
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: C++-only construct is not allowed in signal handler (until C++17) [bugprone-signal-handler]
+  // CHECK-MESSAGES: :[[@

[PATCH] D122657: [NFC] Use range based loop.

2022-03-29 Thread Jun Zhang via Phabricator via cfe-commits
junaire updated this revision to Diff 418877.
junaire added a comment.

use const when possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122657

Files:
  clang/lib/AST/ASTDiagnostic.cpp


Index: clang/lib/AST/ASTDiagnostic.cpp
===
--- clang/lib/AST/ASTDiagnostic.cpp
+++ clang/lib/AST/ASTDiagnostic.cpp
@@ -270,9 +270,9 @@
   std::string S = Ty.getAsString(Context.getPrintingPolicy());
   std::string CanS = CanTy.getAsString(Context.getPrintingPolicy());
 
-  for (unsigned I = 0, E = QualTypeVals.size(); I != E; ++I) {
+  for (const intptr_t &QualTypeVal : QualTypeVals) {
 QualType CompareTy =
-QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVals[I]));
+QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVal));
 if (CompareTy.isNull())
   continue;
 if (CompareTy == Ty)
@@ -302,10 +302,10 @@
   // Check to see if we already desugared this type in this
   // diagnostic.  If so, don't do it again.
   bool Repeated = false;
-  for (unsigned i = 0, e = PrevArgs.size(); i != e; ++i) {
+  for (const auto &PrevArg : PrevArgs) {
 // TODO: Handle ak_declcontext case.
-if (PrevArgs[i].first == DiagnosticsEngine::ak_qualtype) {
-  void *Ptr = (void*)PrevArgs[i].second;
+if (PrevArg.first == DiagnosticsEngine::ak_qualtype) {
+  void *Ptr = (void *)PrevArg.second;
   QualType PrevTy(QualType::getFromOpaquePtr(Ptr));
   if (PrevTy == Ty) {
 Repeated = true;


Index: clang/lib/AST/ASTDiagnostic.cpp
===
--- clang/lib/AST/ASTDiagnostic.cpp
+++ clang/lib/AST/ASTDiagnostic.cpp
@@ -270,9 +270,9 @@
   std::string S = Ty.getAsString(Context.getPrintingPolicy());
   std::string CanS = CanTy.getAsString(Context.getPrintingPolicy());
 
-  for (unsigned I = 0, E = QualTypeVals.size(); I != E; ++I) {
+  for (const intptr_t &QualTypeVal : QualTypeVals) {
 QualType CompareTy =
-QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVals[I]));
+QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVal));
 if (CompareTy.isNull())
   continue;
 if (CompareTy == Ty)
@@ -302,10 +302,10 @@
   // Check to see if we already desugared this type in this
   // diagnostic.  If so, don't do it again.
   bool Repeated = false;
-  for (unsigned i = 0, e = PrevArgs.size(); i != e; ++i) {
+  for (const auto &PrevArg : PrevArgs) {
 // TODO: Handle ak_declcontext case.
-if (PrevArgs[i].first == DiagnosticsEngine::ak_qualtype) {
-  void *Ptr = (void*)PrevArgs[i].second;
+if (PrevArg.first == DiagnosticsEngine::ak_qualtype) {
+  void *Ptr = (void *)PrevArg.second;
   QualType PrevTy(QualType::getFromOpaquePtr(Ptr));
   if (PrevTy == Ty) {
 Repeated = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

2022-03-29 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 418880.
ymandel marked 3 inline comments as done.
ymandel added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122231

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1710,6 +1710,119 @@
  UnorderedElementsAre(Pair("check", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
+  // Pointers.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+void target() {
+  $ns::$optional opt;
+  if (opt.value_or(nullptr) != nullptr) {
+opt.value();
+/*[[check-ptrs-1]]*/
+  } else {
+opt.value();
+/*[[check-ptrs-2]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(Pair("check-ptrs-1", "safe"),
+   Pair("check-ptrs-2", "unsafe: input.cc:10:9")));
+
+  // Integers.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+void target() {
+  $ns::$optional opt;
+  if (opt.value_or(0) != 0) {
+opt.value();
+/*[[check-ints-1]]*/
+  } else {
+opt.value();
+/*[[check-ints-2]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(Pair("check-ints-1", "safe"),
+   Pair("check-ints-2", "unsafe: input.cc:10:9")));
+
+  // Strings.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+namespace std {
+  struct string {
+bool empty();
+  };
+}
+
+void target() {
+  $ns::$optional opt;
+  if (!opt.value_or("").empty()) {
+opt.value();
+/*[[check-strings-1]]*/
+  } else {
+opt.value();
+/*[[check-strings-2]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(Pair("check-strings-1", "safe"),
+   Pair("check-strings-2", "unsafe: input.cc:16:9")));
+
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+namespace std {
+  struct string {
+string(const char*);
+~string();
+bool empty();
+  };
+  bool operator!=(const string &LHS, const char *RHS);
+}
+
+void target() {
+  $ns::$optional opt;
+  if (opt.value_or("") != "") {
+opt.value();
+/*[[check-strings-neq-1]]*/
+  } else {
+opt.value();
+/*[[check-strings-neq-2]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(
+  Pair("check-strings-neq-1", "safe"),
+  Pair("check-strings-neq-2", "unsafe: input.cc:19:9")));
+
+  // Pointer-to-optional.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+void target() {
+  $ns::$optional opt;
+  auto *opt_p = &opt;
+  if (opt_p->value_or(0) != 0) {
+opt_p->value();
+/*[[check-pto-1]]*/
+  } else {
+opt_p->value();
+/*[[check-pto-2]]*/
+  }
+}
+  )code",
+  UnorderedElementsAre(Pair("check-pto-1", "safe"),
+   Pair("check-pto-2", "unsafe: input.cc:11:9")));
+}
+
 TEST_P(UncheckedOptionalAccessTest, Emplace) {
   ExpectLatticeChecksFor(R"(
 #include "unchecked_optional_access_test.h"
@@ -2008,5 +2121,4 @@
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
 // - invalidation (passing optional by non-const reference/pointer)
-// - `value_or(nullptr) != nullptr`, `value_or(0) != 0`, `value_or("").empty()`
 // - nested `optional` values
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -113,6 +113,43 @@
   hasArgument(1, hasOptionalType()));
 }
 
+constexpr llvm::StringLiteral ValueOrCallID = "ValueOrCall";
+
+auto isValueOrStringEmptyCall() {
+  // `opt.value_or("").empty()`
+  return cxxMemberCallExpr(
+  callee(cxxMethodDecl(hasName("empty"))),
+  onImplicitObjectArgument(ignoringImplicit(
+  cxxMemberCallExpr(on(expr(unless(cxxThisExpr(,
+callee(cxxMethodDecl(hasName("value_or"),
+ ofClass(optionalClass(,
+hasArgument(0, stringLiteral(hasSize(0
+ 

[PATCH] D122657: [NFC] Use range based loop.

2022-03-29 Thread Jun Zhang via Phabricator via cfe-commits
junaire updated this revision to Diff 418882.
junaire added a comment.

Simplify code a little bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122657

Files:
  clang/lib/AST/ASTDiagnostic.cpp


Index: clang/lib/AST/ASTDiagnostic.cpp
===
--- clang/lib/AST/ASTDiagnostic.cpp
+++ clang/lib/AST/ASTDiagnostic.cpp
@@ -270,9 +270,9 @@
   std::string S = Ty.getAsString(Context.getPrintingPolicy());
   std::string CanS = CanTy.getAsString(Context.getPrintingPolicy());
 
-  for (unsigned I = 0, E = QualTypeVals.size(); I != E; ++I) {
+  for (const intptr_t &QualTypeVal : QualTypeVals) {
 QualType CompareTy =
-QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVals[I]));
+QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVal));
 if (CompareTy.isNull())
   continue;
 if (CompareTy == Ty)
@@ -302,11 +302,11 @@
   // Check to see if we already desugared this type in this
   // diagnostic.  If so, don't do it again.
   bool Repeated = false;
-  for (unsigned i = 0, e = PrevArgs.size(); i != e; ++i) {
+  for (const auto &PrevArg : PrevArgs) {
 // TODO: Handle ak_declcontext case.
-if (PrevArgs[i].first == DiagnosticsEngine::ak_qualtype) {
-  void *Ptr = (void*)PrevArgs[i].second;
-  QualType PrevTy(QualType::getFromOpaquePtr(Ptr));
+if (PrevArg.first == DiagnosticsEngine::ak_qualtype) {
+  QualType PrevTy(
+  QualType::getFromOpaquePtr(reinterpret_cast(PrevArg.second)));
   if (PrevTy == Ty) {
 Repeated = true;
 break;


Index: clang/lib/AST/ASTDiagnostic.cpp
===
--- clang/lib/AST/ASTDiagnostic.cpp
+++ clang/lib/AST/ASTDiagnostic.cpp
@@ -270,9 +270,9 @@
   std::string S = Ty.getAsString(Context.getPrintingPolicy());
   std::string CanS = CanTy.getAsString(Context.getPrintingPolicy());
 
-  for (unsigned I = 0, E = QualTypeVals.size(); I != E; ++I) {
+  for (const intptr_t &QualTypeVal : QualTypeVals) {
 QualType CompareTy =
-QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVals[I]));
+QualType::getFromOpaquePtr(reinterpret_cast(QualTypeVal));
 if (CompareTy.isNull())
   continue;
 if (CompareTy == Ty)
@@ -302,11 +302,11 @@
   // Check to see if we already desugared this type in this
   // diagnostic.  If so, don't do it again.
   bool Repeated = false;
-  for (unsigned i = 0, e = PrevArgs.size(); i != e; ++i) {
+  for (const auto &PrevArg : PrevArgs) {
 // TODO: Handle ak_declcontext case.
-if (PrevArgs[i].first == DiagnosticsEngine::ak_qualtype) {
-  void *Ptr = (void*)PrevArgs[i].second;
-  QualType PrevTy(QualType::getFromOpaquePtr(Ptr));
+if (PrevArg.first == DiagnosticsEngine::ak_qualtype) {
+  QualType PrevTy(
+  QualType::getFromOpaquePtr(reinterpret_cast(PrevArg.second)));
   if (PrevTy == Ty) {
 Repeated = true;
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

2022-03-29 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked 29 inline comments as done.
balazske added a comment.

Code is updated, documentation (rst) not yet. I want to use `\` format of tags 
in the code comments. Many review comments are now out of place (because the 
list of functions was moved around in the file.)




Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:94
 
+SourceRange getSourceRangeOfStmt(const Stmt *S, ASTContext &Ctx) {
+  ParentMapContext &PM = Ctx.getParentMapContext();

whisperity wrote:
> Okay, this is interesting... Why is `Ctx` not `const`? Unless 
> **`get`**`Something()` is changing things (which is non-intuitive then...), 
> as far as I can tell, you're only reading from `Ctx`.
It does only reading, but `getParentMapContext` does not work with `const`.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:96
+  ParentMapContext &PM = Ctx.getParentMapContext();
+  DynTypedNode P = DynTypedNode::create(*S);
+  while (P.getSourceRange().getBegin().isInvalid()) {

whisperity wrote:
> Are these objects cleaned up properly in their destructor (same question for 
> `DynTypeNodeList`)?
I think it should work if there is not a function for deallocating.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:99-101
+if (PL.size() != 1)
+  return {};
+P = PL[0];

whisperity wrote:
> `size != 1` could still mean `size == 0` in which case taking an element 
> seems dangerous. Is triggering such UB possible here?
size is exactly 1 after the `if`



Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:180
+// No need to display a call chain and no need for more checks.
+(void)checkFunction(HandlerDecl, HandlerExpr, [](bool) {});
 return;

whisperity wrote:
> What is happening here? Why is the last parameter a callback(?) taking a 
> `bool`? Why is the lambda empty? I see it is a `std::function`, can't we 
> instead pass an empty function object there, and guard against calling it in 
> `checkFunction`, making things a bit more explicit?
Documentation of the function `checkFunction` tells what is the last parameter 
for: It is used to display the call chain notes (works as callback). A function 
object is used because then more information can be passed into it (instead of 
additional parameters to `checkFunction`) and it is possible to pass an empty 
function if needed. Here this function is not used, it is empty.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:280-281
+  << R;
+  diag(R.getBegin(), "statement class is: '%0'", DiagnosticIDs::Note)
+  << Name;
+  ChainReporter(/*SkipPathEnd=*/false);

whisperity wrote:
> I think this is a fine moment to use the `Remark` diagnostic category! ✨  It 
> is very likely that the average client will not look into Clang internals 
> here... (It is dubious whether or not emitting this diagnostic is meaningful 
> in the first place, but at least it helps with //some// 
> debugging/understanding for those who wish to dig deep.)
The statement class is often relatively easy to understand without looking into 
source code. Still it is a hint only, but there is no easy way to get a 
description for any statement class (without hard-coding into the checker).



Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:39-40
   /// Should not check function calls in the code (this part is done by the 
call
-  /// graph scan).
+  /// graph scan). If a problem is found in the "function properties" (other
+  /// than the code body) no more problems are to be reported. Otherwise every
+  /// found problem in the code body should be reported (not only the first

whisperity wrote:
> What is a "function property"? (Or is that an implementation detail of the 
> check?)
I meant a property of the function in everyday sense, for example is it "extern 
C" or not, `const` or not, and similar (anything that is not in the body part).



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-signal-handler.c:21
   // CHECK-NOTES: :[[@LINE-1]]:3: warning: system call 'printf' may not be 
asynchronous-safe; calling it from a signal handler may be dangerous 
[bugprone-signal-handler]
-  // CHECK-NOTES: :[[@LINE-2]]:3: note: function 'printf' called here from 
'handler_printf'
   // CHECK-NOTES: :[[@LINE+4]]:18: note: function 'handler_printf' registered 
here as signal handler

whisperity wrote:
> I really like that this trivial bit stops being repeated! Is this what the 
> `SkipPathEnd` does in practice?
Yes `SkipPathEnd` is added to remove these redundant notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118996


[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

2022-03-29 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked 5 inline comments as done.
balazske added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:254
+  return DC->isTranslationUnit();
+}*/
+

This will be removed (`FunctionDecl::isGlobal` should work for this purpose).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118996

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


[PATCH] D122069: [Object] Add binary format for bundling offloading metadata

2022-03-29 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D122069#3413960 , @JonChesterfield 
wrote:

> Added some reviewers. I'd much prefer this used an existing binary format, 
> DIY is prone to errors and maintenance hassles down the road. Don't care as 
> much about which format as about it being one with an existing, tested 
> implementation and ideally existing inspection tools.

I'm not married to the idea, and worst case scenario we can replace it with 
something else in the future. I'd like to get something like this working so I 
can finally make the new driver the default, so I'll just outline the problem 
and some of the potential solutions.

The issue is that we need to store some metadata along with a binary image so 
we know how to handle it at a later date. Currently we shove this in the name 
of the ELF section and parse it there, but that's not idea because more 
metadata will be needed in the future and it prevents us from doing things like 
relocatable linking or other merging (e.g. we want to store both an sm_70 and 
sm_35 image in the same file). So we want a binary format that can store some 
strings, other data. I'll just go over a brief overview of the options:

YAML / JSON
---

+ Ubiquitous simple text format for encoding object data
+ In-tree implementation
x Requires encoding to store the device image
x Will need binary padding and size calculation to make sure these merge 
properly in a section

Protocol Buffers


+ Well-tested
+ Implicit appending, no additional code required to handle merged sections.
x Out-of-tree, requires external dependencies to build and maintain in the 
future. No other use in Clang / LLVM

ELF
---

+ Ubiquitous tooling. Object extraction and copying for free
+ Simple key-value storage 
x Difficult to calculate size, will need to figure out the size of the buffer 
and write it in later so we can read multiple appended sections.
x Difficult to create. The Elf object writer is completely tied to the MC 
backend. YAML2ELF would require base64 or similar again

MSGPACK
---

+ Exists in-tree in some form and well tested
+ Supports key-value storage
x Doesn't know its size, will need to add padding and a size field

Custom format
-

+ Relatively simple implementation that solves this specific problem
x No existing tooling support, more error prone

I decided to go with the custom format because it was the simplest to get 
working for a proof of concept to solve the problem I was immediately facing. I 
think ELF would be the next best if someones could suggest a way to write the 
data and get the size. MSGPACK seems to be @JonChesterfield's preferred method 
because it has a lot of use at AMD, it would work as long as we can figure out 
its size and get alignment. Let me know what suggestions you have because I 
really want to move forward with this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122069

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


[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

2022-03-29 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 7 inline comments as done.
ymandel added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:142
+  // `opt.value_or(nullptr) != nullptr` and `opt.value_or(0) != 0`. Ideally,
+  // we'd support this pattern for any expression, but the AST does not have a
+  // generic expression comparison facility, so we specialize to common cases

xazax.hun wrote:
> Yeah, I think Clang is in a very sad state in this regard. We have a lot of 
> half done facilities littered all over the codebase, including:
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Analysis/CloneDetection.h
> https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp#L306
> https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp#L60
> 
> 
Right. I've added a FIXME. I think an elegant solution in this case would be a 
relational matcher for built-in types.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:147
+ anyOf(ComparesToSame(cxxNullPtrLiteralExpr()),
+   ComparesToSame(integerLiteral(equals(0);
+}

xazax.hun wrote:
> I wonder if we want to add `""` to support `opt.value_or("") != ""`. Not sure 
> how frequent would this be over the empty call.
Makes sense -- done.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:270
+  // needed.
+  BoolValue &ComparisonValue = MakeValue(Env, *HasValueVal);
+  auto *ComparisonExprLoc =

sgatev wrote:
> ymandel wrote:
> > sgatev wrote:
> > > ymandel wrote:
> > > > ymandel wrote:
> > > > > xazax.hun wrote:
> > > > > > xazax.hun wrote:
> > > > > > > ymandel wrote:
> > > > > > > > xazax.hun wrote:
> > > > > > > > > Is this the right way to initialize `ComparisonValue`?
> > > > > > > > > 
> > > > > > > > > Considering the expression: `opt.value_or(nullptr) != nullptr`
> > > > > > > > > * When `has_value == false`, `opt.value_or(nullptr)` will 
> > > > > > > > > return `nullptr`, so `!=` evaluates to false. This case seems 
> > > > > > > > > to check out.
> > > > > > > > > * However, when `has_value == true`, `opt` might still hold 
> > > > > > > > > an `nullptr` and `!=` could still evaluate to false. 
> > > > > > > > Thanks for digging into this. I think it's correct, but helpful 
> > > > > > > > to step through:
> > > > > > > > 
> > > > > > > > Its correctness depends on `MakeValue`, so I'll focus on that 
> > > > > > > > in particular. For the `nullptr` case, we'll get:
> > > > > > > > ```
> > > > > > > > HasValueVal && ContentsNotEqX
> > > > > > > > ```
> > > > > > > > So, when `has_value == true`, this basically reduces to 
> > > > > > > > `ContentsNotEqX`. Since that's an atom, the result is 
> > > > > > > > indeterminate, which I believe is the desired outcome.
> > > > > > > > 
> > > > > > > > WDYT?  Also, even if I've convinced you, please let me know how 
> > > > > > > > i can improve the comments. For that matter, would `MakeValue` 
> > > > > > > > be better with a more specific name, like "MakePredicate" or 
> > > > > > > > somesuch?
> > > > > > > I think what confuses me is that we do something different for 
> > > > > > > the 3 cases. You convinced me that `HasValueVal && 
> > > > > > > ContentsNotEqX` is correct. But we only do this for one branch 
> > > > > > > out of the 3.  What is the reason for that?
> > > > > > Oh, never mind. Yeah, I think changing `MakeValue` to 
> > > > > > `MakePredicate` would make this a bit clearer. After a second read 
> > > > > > now I understand better what is going on.
> > > > > Just to be clear: the three cases you mean are lines 273-283, or 
> > > > > something else?
> > > > and never mind my question, then (I rpelied before I saw your updated). 
> > > > I'll change the name and add comments.
> > > Can you elaborate on the three cases on lines 273-283? Why not simply do
> > > 
> > > ```
> > > auto &ComparisonExprLoc = Env.createStorageLocation(*ComparisonExpr);
> > > Env.setStorageLocation(ComparisonExpr, ComparisonExprLoc);
> > > Env.setValue(ComparisonExprLoc, ComparisonValue);
> > > ```
> > > Can you elaborate on the three cases on lines 273-283? Why not simply do
> > > 
> > > ```
> > > auto &ComparisonExprLoc = Env.createStorageLocation(*ComparisonExpr);
> > > Env.setStorageLocation(ComparisonExpr, ComparisonExprLoc);
> > > Env.setValue(ComparisonExprLoc, ComparisonValue);
> > > ```
> > 
> > for the second case: I think we should drop it -- I don't see a reason to 
> > maintain the previous value (if there is any). It might be a good idea for 
> > compositionality, but we're not doing that anywhere else, so it doesn't 
> > make sense here.
> > 
> > for the first and third case: I assumed that if the expression already has 
> > a location, we'd want

[PATCH] D122656: [C11] Improve the diagnostic when accessing a member of an atomic struct

2022-03-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked 2 inline comments as done.
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaExprMember.cpp:1300
+  // warning defaults to an error.
+  if (const auto *ATy = BaseType->getAs()) {
+S.Diag(OpLoc, diag::warn_atomic_member_access);

erichkeane wrote:
> This seems to apply to both C and C++.  I guess "_Atomic" is C only, so we 
> get to define its behavior for C++?
> 
> What does GCC do in C++ mode?
Clang supports `_Atomic` in C++ as an extension (with the C semantics), GCC 
does not.

However, after doing some testing, I actually question whether Clang supports 
`_Atomic` in C++ more or just "pretends everything will work out fine in C++ 
mode but nobody ever actually checked that". I'm not going to add additional 
test coverage for this because I'm going to see just how many issues I spot in 
C++ first (it may be a better approach to disable the extension in C++).



Comment at: clang/test/Sema/atomic-expr.c:85
+  x.val = 12; // expected-error {{accessing a member of an atomic structure or 
union is undefined behavior}}
+  xp->val = 12; // expected-error {{accessing a member of an atomic structure 
or union is undefined behavior}}
+

erichkeane wrote:
> This still catches RHS access as well, right?  
> 
> Also, does it still 'work' with the non-qualifier version of _Atomic?
> This still catches RHS access as well, right?

Yes, I'll add tests.

> Also, does it still 'work' with the non-qualifier version of _Atomic?

Yes, I'll add a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122656

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


[PATCH] D122656: [C11] Improve the diagnostic when accessing a member of an atomic struct

2022-03-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 418884.
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

Added test cases based on review feedback.


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

https://reviews.llvm.org/D122656

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExprMember.cpp
  clang/test/Sema/atomic-expr.c


Index: clang/test/Sema/atomic-expr.c
===
--- clang/test/Sema/atomic-expr.c
+++ clang/test/Sema/atomic-expr.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -verify=off -fsyntax-only -Wno-atomic-access
+// off-no-diagnostics
 
 _Atomic(unsigned int) data1;
 int _Atomic data2;
@@ -75,3 +76,30 @@
   _Static_assert(__builtin_types_compatible_p(__typeof__(x = 2), int), 
"incorrect");
   _Static_assert(__builtin_types_compatible_p(__typeof__(x += 2), int), 
"incorrect");
 }
+
+// Ensure that member access of an atomic structure or union type is properly
+// diagnosed as being undefined behavior; Issue 54563.
+void func_16(void) {
+  // LHS member access.
+  _Atomic struct { int val; } x, *xp;
+  x.val = 12;   // expected-error {{accessing a member of an atomic structure 
or union is undefined behavior}}
+  xp->val = 12; // expected-error {{accessing a member of an atomic structure 
or union is undefined behavior}}
+
+  _Atomic union {
+int ival;
+float fval;
+  } y, *yp;
+  y.ival = 12; // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+  yp->fval = 1.2f; // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+
+  // RHS member access.
+  int xval = x.val; // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+  xval = xp->val;   // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+  int yval = y.val; // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+  yval = yp->val;   // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+
+  // Using the type specifier instead of the type qualifier.
+  _Atomic(struct { int val; }) z;
+  z.val = 12;   // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+  int zval = z.val; // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+}
Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -1292,6 +1292,20 @@
 }
   }
 
+  // If the base type is an atomic type, this access is undefined behavior per
+  // C11 6.5.2.3p5. Instead of giving a typecheck error, we'll warn the user
+  // about the UB and recover by converting the atomic lvalue into a non-atomic
+  // lvalue. Because this is inherently unsafe as an atomic operation, the
+  // warning defaults to an error.
+  if (const auto *ATy = BaseType->getAs()) {
+S.Diag(OpLoc, diag::warn_atomic_member_access);
+BaseType = ATy->getValueType().getUnqualifiedType();
+BaseExpr = ImplicitCastExpr::Create(
+S.Context, IsArrow ? S.Context.getPointerType(BaseType) : BaseType,
+CK_AtomicToNonAtomic, BaseExpr.get(), nullptr,
+BaseExpr.get()->getValueKind(), FPOptionsOverride());
+  }
+
   // Handle field access to simple records.
   if (const RecordType *RTy = BaseType->getAs()) {
 TypoExpr *TE = nullptr;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6439,6 +6439,9 @@
 "%select{incomplete |array |function |reference |atomic |qualified 
"
 "|sizeless ||integer }0type "
 "%1 %select{|||which is not trivially copyable|}0">;
+def warn_atomic_member_access : Warning<
+  "accessing a member of an atomic structure or union is undefined behavior">,
+  InGroup>, DefaultError;
 
 // Expressions.
 def ext_sizeof_alignof_function_type : Extension<
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -85,6 +85,10 @@
 - Assignment expressions in C11 and later mode now properly strip the _Atomic
   qualifier when determining the type of the assignment expression. Fixes
   `Issue 48742 `_.
+- Improved the diagnostic when accessing a member of an atomic structure or
+  union object in C; was previously an unhelpful error, but now issues a
+  `-Watomic-access` warning which defaults to an err

[PATCH] D122656: [C11] Improve the diagnostic when accessing a member of an atomic struct

2022-03-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Sema/SemaExprMember.cpp:1300
+  // warning defaults to an error.
+  if (const auto *ATy = BaseType->getAs()) {
+S.Diag(OpLoc, diag::warn_atomic_member_access);

aaron.ballman wrote:
> erichkeane wrote:
> > This seems to apply to both C and C++.  I guess "_Atomic" is C only, so we 
> > get to define its behavior for C++?
> > 
> > What does GCC do in C++ mode?
> Clang supports `_Atomic` in C++ as an extension (with the C semantics), GCC 
> does not.
> 
> However, after doing some testing, I actually question whether Clang supports 
> `_Atomic` in C++ more or just "pretends everything will work out fine in C++ 
> mode but nobody ever actually checked that". I'm not going to add additional 
> test coverage for this because I'm going to see just how many issues I spot 
> in C++ first (it may be a better approach to disable the extension in C++).
Based on the examples I've seen, I think we're better off matching GCC's 
behavior here of disallowing _Atomic in both forms in C++, unless it is 
necessary/used to implement the std::atomic version in libc++ (obviously it 
isn't in libstdc++ if they disable it in c++ mode?).


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

https://reviews.llvm.org/D122656

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


[PATCH] D122659: [clang][preprocessor] Allow calling DumpToken() on annotation tokens

2022-03-29 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added a reviewer: aaron.ballman.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The `getSpelling()` call otherwise ultimately runs into an assertion because 
`getLength()` is called on an annotation token.

Just ran into this when debugging using `-dump-tokens`.
Sorry I can't really find a better reviewer, the rest of that function seems to 
be pretty ancient.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122659

Files:
  clang/lib/Lex/Preprocessor.cpp


Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -232,8 +232,10 @@
 }
 
 void Preprocessor::DumpToken(const Token &Tok, bool DumpFlags) const {
-  llvm::errs() << tok::getTokenName(Tok.getKind()) << " '"
-   << getSpelling(Tok) << "'";
+  llvm::errs() << tok::getTokenName(Tok.getKind());
+
+  if (!Tok.isAnnotation())
+llvm::errs() << " '" << getSpelling(Tok) << "'";
 
   if (!DumpFlags) return;
 


Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -232,8 +232,10 @@
 }
 
 void Preprocessor::DumpToken(const Token &Tok, bool DumpFlags) const {
-  llvm::errs() << tok::getTokenName(Tok.getKind()) << " '"
-   << getSpelling(Tok) << "'";
+  llvm::errs() << tok::getTokenName(Tok.getKind());
+
+  if (!Tok.isAnnotation())
+llvm::errs() << " '" << getSpelling(Tok) << "'";
 
   if (!DumpFlags) return;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

2022-03-29 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

Wow. This did take some iterations and I feel like I just added to the 
confusion at some point :D But the latest iteration looks much simpler and I'm 
confident it is right this time. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122231

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


[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

2022-03-29 Thread Sanjay Patel 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 rGa3cffc11509b: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) 
into ctpop(X) < 2 (authored by hkmatsumoto, committed by spatel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122077

Files:
  llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
  llvm/test/Transforms/InstCombine/ispow2.ll

Index: llvm/test/Transforms/InstCombine/ispow2.ll
===
--- llvm/test/Transforms/InstCombine/ispow2.ll
+++ llvm/test/Transforms/InstCombine/ispow2.ll
@@ -740,10 +740,8 @@
 define i1 @is_pow2or0_ctpop(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 1
@@ -755,10 +753,8 @@
 define i1 @is_pow2or0_ctpop_swap_cmp(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_swap_cmp(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[CMP]], [[ISZERO]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 1
@@ -770,10 +766,8 @@
 define i1 @is_pow2or0_ctpop_logical(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_logical(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[T0]], 1
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 1
@@ -785,10 +779,8 @@
 define <2 x i1> @is_pow2or0_ctpop_commute_vec(<2 x i8> %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_commute_vec(
 ; CHECK-NEXT:[[T0:%.*]] = tail call <2 x i8> @llvm.ctpop.v2i8(<2 x i8> [[X:%.*]])
-; CHECK-NEXT:[[CMP:%.*]] = icmp eq <2 x i8> [[T0]], 
-; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq <2 x i8> [[X]], zeroinitializer
-; CHECK-NEXT:[[R:%.*]] = or <2 x i1> [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret <2 x i1> [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult <2 x i8> [[T0]], 
+; CHECK-NEXT:ret <2 x i1> [[TMP1]]
 ;
   %t0 = tail call <2 x i8> @llvm.ctpop.v2i8(<2 x i8> %x)
   %cmp = icmp eq <2 x i8> %t0, 
@@ -807,8 +799,8 @@
 ; CHECK-NEXT:call void @use_i1(i1 [[CMP]])
 ; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
 ; CHECK-NEXT:call void @use_i1(i1 [[ISZERO]])
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   call void @use(i32 %t0)
@@ -828,8 +820,8 @@
 ; CHECK-NEXT:call void @use_i1(i1 [[CMP]])
 ; CHECK-NEXT:[[ISZERO:%.*]] = icmp eq i32 [[X]], 0
 ; CHECK-NEXT:call void @use_i1(i1 [[ISZERO]])
-; CHECK-NEXT:[[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ult i32 [[T0]], 2
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   call void @use(i32 %t0)
@@ -940,10 +932,8 @@
 define i1 @isnot_pow2nor0_ctpop(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2nor0_ctpop(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp ne i32 [[T0]], 1
-; CHECK-NEXT:[[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ugt i32 [[T0]], 1
+; CHECK-NEXT:ret i1 [[TMP1]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ne i32 %t0, 1
@@ -955,10 +945,8 @@
 define i1 @isnot_pow2nor0_ctpop_swap_cmp(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2nor0_ctpop_swap_cmp(
 ; CHECK-NEXT:[[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
-; CHECK-NEXT:[[CMP:%.*]] = icmp ne i32 [[T0]], 1
-; CHECK-NEXT:[[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT:[[R:%.*]] = and i1 [[CMP]], [[NOTZERO]]
-; CHECK-NEXT:ret i1 [[R]]
+; CHECK-NEXT:[[TMP1:%.*]] = icmp ugt i32 [[T0]], 1
+; CHECK-NEXT:ret i1 [[

[PATCH] D122661: [Clang] Do not warn on unused lifetime-extending vars with side effects...

2022-03-29 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin created this revision.
Herald added a project: All.
cor3ntin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

const auto & var = ObjectWithSideEffects();

Fixes https://github.com/llvm/llvm-project/issues/54489


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122661

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/warn-unused-variables.cpp

Index: clang/test/SemaCXX/warn-unused-variables.cpp
===
--- clang/test/SemaCXX/warn-unused-variables.cpp
+++ clang/test/SemaCXX/warn-unused-variables.cpp
@@ -154,13 +154,13 @@
 
 #include "Inputs/warn-unused-variables.h"
 
-namespace arrayRecords {
-
 class NonTriviallyDestructible {
 public:
   ~NonTriviallyDestructible() {}
 };
 
+namespace arrayRecords {
+
 struct Foo {
   int x;
   Foo(int x) : x(x) {}
@@ -196,7 +196,7 @@
   bar<2>();
 }
 
-}
+} // namespace arrayRecords
 
 #if __cplusplus >= 201103L
 namespace with_constexpr {
@@ -250,6 +250,32 @@
 template 
 void foo(T &t) {
   S s{t};
+  const auto &extended = S{t};
 }
 }
 #endif
+
+// Ensure we do not warn on lifetime extension
+namespace gh54489 {
+
+void f() {
+  const auto &a = NonTriviallyDestructible();
+  const auto &b = a; // expected-warning {{unused variable 'b'}}
+#if __cplusplus >= 201103L
+  const auto &&c = NonTriviallyDestructible();
+  auto &&d = c; // expected-warning {{unused variable 'd'}}
+#endif
+}
+
+struct S {
+  S() = default;
+  S(const S &) = default;
+  S(int);
+};
+
+template 
+void foo(T &t) {
+  const auto &extended = S{t};
+}
+
+} // namespace gh54489
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1889,15 +1889,30 @@
   // Types of valid local variables should be complete, so this should succeed.
   if (const VarDecl *VD = dyn_cast(D)) {
 
-// White-list anything with an __attribute__((unused)) type.
+const Expr *Init = VD->getInit();
+if (const ExprWithCleanups *Cleanups =
+dyn_cast_or_null(Init))
+  Init = Cleanups->getSubExpr();
+
 const auto *Ty = VD->getType().getTypePtr();
 
 // Only look at the outermost level of typedef.
 if (const TypedefType *TT = Ty->getAs()) {
+  // White-list anything with an __attribute__((unused)) type.
   if (TT->getDecl()->hasAttr())
 return false;
 }
 
+// Warn for reference variables whose initializtion performs lifetime
+// extension.
+if (const MaterializeTemporaryExpr *MTE =
+dyn_cast_or_null(Init)) {
+  if (MTE->getExtendingDecl()) {
+Ty = VD->getType().getNonReferenceType().getTypePtr();
+Init = MTE->getSubExpr()->IgnoreImplicitAsWritten();
+  }
+}
+
 // If we failed to complete the type for some reason, or if the type is
 // dependent, don't diagnose the variable.
 if (Ty->isIncompleteType() || Ty->isDependentType())
@@ -1916,10 +1931,7 @@
 if (!RD->hasTrivialDestructor() && !RD->hasAttr())
   return false;
 
-if (const Expr *Init = VD->getInit()) {
-  if (const ExprWithCleanups *Cleanups =
-  dyn_cast(Init))
-Init = Cleanups->getSubExpr();
+if (Init) {
   const CXXConstructExpr *Construct =
 dyn_cast(Init);
   if (Construct && !Construct->isElidable()) {
@@ -1931,10 +1943,16 @@
 
   // Suppress the warning if we don't know how this is constructed, and
   // it could possibly be non-trivial constructor.
-  if (Init->isTypeDependent())
+  if (Init->isTypeDependent()) {
 for (const CXXConstructorDecl *Ctor : RD->ctors())
   if (!Ctor->isTrivial())
 return false;
+  }
+
+  // Suppress the warning if the constructor is unresolved because
+  // its arguments are dependent.
+  if (isa(Init))
+return false;
 }
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122661: [Clang] Do not warn on unused lifetime-extending vars with side effects...

2022-03-29 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 418891.
cor3ntin added a comment.

Cleanup test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122661

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/warn-unused-variables.cpp

Index: clang/test/SemaCXX/warn-unused-variables.cpp
===
--- clang/test/SemaCXX/warn-unused-variables.cpp
+++ clang/test/SemaCXX/warn-unused-variables.cpp
@@ -154,13 +154,13 @@
 
 #include "Inputs/warn-unused-variables.h"
 
-namespace arrayRecords {
-
 class NonTriviallyDestructible {
 public:
   ~NonTriviallyDestructible() {}
 };
 
+namespace arrayRecords {
+
 struct Foo {
   int x;
   Foo(int x) : x(x) {}
@@ -196,7 +196,7 @@
   bar<2>();
 }
 
-}
+} // namespace arrayRecords
 
 #if __cplusplus >= 201103L
 namespace with_constexpr {
@@ -253,3 +253,28 @@
 }
 }
 #endif
+
+// Ensure we do not warn on lifetime extension
+namespace gh54489 {
+
+void f() {
+  const auto &a = NonTriviallyDestructible();
+  const auto &b = a; // expected-warning {{unused variable 'b'}}
+#if __cplusplus >= 201103L
+  const auto &&c = NonTriviallyDestructible();
+  auto &&d = c; // expected-warning {{unused variable 'd'}}
+#endif
+}
+
+struct S {
+  S() = default;
+  S(const S &) = default;
+  S(int);
+};
+
+template 
+void foo(T &t) {
+  const auto &extended = S{t};
+}
+
+} // namespace gh54489
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1889,15 +1889,30 @@
   // Types of valid local variables should be complete, so this should succeed.
   if (const VarDecl *VD = dyn_cast(D)) {
 
-// White-list anything with an __attribute__((unused)) type.
+const Expr *Init = VD->getInit();
+if (const ExprWithCleanups *Cleanups =
+dyn_cast_or_null(Init))
+  Init = Cleanups->getSubExpr();
+
 const auto *Ty = VD->getType().getTypePtr();
 
 // Only look at the outermost level of typedef.
 if (const TypedefType *TT = Ty->getAs()) {
+  // White-list anything with an __attribute__((unused)) type.
   if (TT->getDecl()->hasAttr())
 return false;
 }
 
+// Warn for reference variables whose initializtion performs lifetime
+// extension.
+if (const MaterializeTemporaryExpr *MTE =
+dyn_cast_or_null(Init)) {
+  if (MTE->getExtendingDecl()) {
+Ty = VD->getType().getNonReferenceType().getTypePtr();
+Init = MTE->getSubExpr()->IgnoreImplicitAsWritten();
+  }
+}
+
 // If we failed to complete the type for some reason, or if the type is
 // dependent, don't diagnose the variable.
 if (Ty->isIncompleteType() || Ty->isDependentType())
@@ -1916,10 +1931,7 @@
 if (!RD->hasTrivialDestructor() && !RD->hasAttr())
   return false;
 
-if (const Expr *Init = VD->getInit()) {
-  if (const ExprWithCleanups *Cleanups =
-  dyn_cast(Init))
-Init = Cleanups->getSubExpr();
+if (Init) {
   const CXXConstructExpr *Construct =
 dyn_cast(Init);
   if (Construct && !Construct->isElidable()) {
@@ -1931,10 +1943,16 @@
 
   // Suppress the warning if we don't know how this is constructed, and
   // it could possibly be non-trivial constructor.
-  if (Init->isTypeDependent())
+  if (Init->isTypeDependent()) {
 for (const CXXConstructorDecl *Ctor : RD->ctors())
   if (!Ctor->isTrivial())
 return false;
+  }
+
+  // Suppress the warning if the constructor is unresolved because
+  // its arguments are dependent.
+  if (isa(Init))
+return false;
 }
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122544: Utilize comparison operation implemented in APInt

2022-03-29 Thread Danny Mösch via Phabricator via cfe-commits
SimplyDanny added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp:175
 // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: suspicious usage of sizeof 
pointer 'sizeof(T)/sizeof(T)'
-int Test3() { return Foo<42>() + Bar(); }
+template <__int128_t N> 
+bool Baz() { return sizeof(A) < N; }

aaron.ballman wrote:
> aaronpuchert wrote:
> > This causes test failures on 32-bit architectures:
> > ```
> > /home/tcwg-buildbot/worker/clang-armv7-2stage/stage2/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-sizeof-expression.cpp.tmp.cpp:175:11:
> >  error: unknown type name '__int128_t' [clang-diagnostic-error]
> > template <__int128_t N> 
> >   ^
> > ```
> > 
> > It's probably best to wrap this in `#ifdef __SIZEOF_INT128__` lest we 
> > disable the entire test on the affected platforms.
> Good catch! @SimplyDanny, can you fix up the test?
Sure. I will fix it. Sorry for the issue ...

What is the process in this case? Push the fix directly to main or go the 
normal path via Phabricator review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122544

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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa created this revision.
yihanaa added reviewers: erichkeane, aaron.ballman.
yihanaa added projects: LLVM, clang.
Herald added a project: All.
yihanaa requested review of this revision.
Herald added a subscriber: cfe-commits.

1. Add constant array support for __builtin_dump_sturct, the style like lldb

struct:

  struct S {
int x;
int y : 4;
int : 0;
int b[2][2];
float f;
struct T {
  int i;
} t;
struct {
  int i;
} foo;
struct Bar {
int x;
const char *B;
  } bar[2];
  };

output:

  struct S {
  int x = 100
  int y : 4 = 1
  int : 0
  int[2][2] b = [
  [0] = [
  [0] = 1
  [1] = 2
  ]
  [1] = [
  [0] = 3
  [1] = 4
  ]
  ]
  float f = 0.00
  struct T {
  int i = 2022
  }
  struct S::(unnamed) {
  int i = 4096
  }
  struct Bar[2] bar = [
  [0] = {
  int x = 1024
  const char * B = This is struct Bar[0]
  }
  [1] = {
  int x = 2048
  const char * B = This is struct Bar[1]
  }
  ]
  }

2. beautify dump format, add indent.

for example:
struct:

  struct A {
int a;
struct B {
  int b;
  struct C {
struct D {
  int d;
  union E {
int x;
int y;
  } e;
} d;
int c;
  } c;
} b;
  };

Before:

  struct A {
  int a = 0
  struct B {
  int b = 0
  struct C {
  struct D {
  int d = 0
  union E {
  int x = 0
  int y = 0
  }
  }
  int c = 0
  }
  }
  }

After:

  struct A {
  int a = 0
  struct B {
  int b = 0
  struct C {
  struct D {
  int d = 0
  union E {
  int x = 0
  int y = 0
  }
  }
  int c = 0
  }
  }
  }

3. Remove anonymous tag locations, powered by 'PrintingPolicy'

struct:

  struct S {
int a;
struct /* Anonymous*/ {
  int x;
} b;
int c;
  };

Before:

  struct S {
  int a = 0
  struct S::(unnamed at ./builtin_dump_struct.c:20:3) {
  int x = 0
  }
  int c = 0
  }

After:

  struct S {
  int a = 0
  struct S::(unnamed) {
  int x = 0
  }
  int c = 0
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122662

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/dump-struct-builtin.c

Index: clang/test/CodeGen/dump-struct-builtin.c
===
--- clang/test/CodeGen/dump-struct-builtin.c
+++ clang/test/CodeGen/dump-struct-builtin.c
@@ -1,99 +1,116 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
 
-#include "Inputs/stdio.h"
-#include 
+typedef signed char int8_t;
+typedef unsigned char uint8_t;
+int printf(const char *, ...);
 
 // CHECK: @__const.unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"short a = %hd\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"short a = %hd\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U1:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit2.a = private unnamed_addr constant %struct.U2A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U2:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U2A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"unsigned short a = %hu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [28 x i8] c"unsigned short a = %hu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U2:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit3.a = private unnamed_addr constant %struct.U3A { i32 12 }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U3:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U3A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [12 x i8] c"int a = %d\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [16 x i8] c"int a = %d\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U3:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit4.a = private unnamed_addr constant %struct.U4A { i32 12 }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U4:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U4A {\0A\00", align 1
-// CHECK-

[PATCH] D122544: Utilize comparison operation implemented in APInt

2022-03-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp:175
 // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: suspicious usage of sizeof 
pointer 'sizeof(T)/sizeof(T)'
-int Test3() { return Foo<42>() + Bar(); }
+template <__int128_t N> 
+bool Baz() { return sizeof(A) < N; }

SimplyDanny wrote:
> aaron.ballman wrote:
> > aaronpuchert wrote:
> > > This causes test failures on 32-bit architectures:
> > > ```
> > > /home/tcwg-buildbot/worker/clang-armv7-2stage/stage2/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-sizeof-expression.cpp.tmp.cpp:175:11:
> > >  error: unknown type name '__int128_t' [clang-diagnostic-error]
> > > template <__int128_t N> 
> > >   ^
> > > ```
> > > 
> > > It's probably best to wrap this in `#ifdef __SIZEOF_INT128__` lest we 
> > > disable the entire test on the affected platforms.
> > Good catch! @SimplyDanny, can you fix up the test?
> Sure. I will fix it. Sorry for the issue ...
> 
> What is the process in this case? Push the fix directly to main or go the 
> normal path via Phabricator review?
When fixing a broken test, you can generally push directly to main. (And if the 
fix requires thought, it's usually best to revert the broken commit from main 
until you have the test fixed, then push the fixed commit back to main).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122544

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


[PATCH] D120662: [clang-offload-bundler] add -input/-output options

2022-03-29 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120662

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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

This looks to be quite the large patch, can you split this up into individual 
patches?  It isn't clear to me what all is going on everywhere.




Comment at: clang/docs/ReleaseNotes.rst:93
   and `51641 `_.
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct have bitfield. Now it fixed.

What is this from/for?  Is this left over from a previous patch?



Comment at: clang/docs/ReleaseNotes.rst:113
 -
-- The builtin function __builtin_dump_struct would crash clang when the target 
-  struct have bitfield. Now it fixed, and __builtin_dump_struct support dump
-  the bitwidth of bitfields.
-  This fixes `Issue 54462 
`_.
+- The builtin function __builtin_dump_struct support dump constant array and 
the 
+  bitwidth of bitfields in a struct.

"now supports dumping constant arrays" perhaps?



Comment at: clang/docs/ReleaseNotes.rst:114
+- The builtin function __builtin_dump_struct support dump constant array and 
the 
+  bitwidth of bitfields in a struct.
+  

This should probably be a different entry than the bitfield one.



Comment at: clang/docs/ReleaseNotes.rst:155
 
-- Improve __builtin_dump_struct:
-

Probably want a separate patch to fix these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122662

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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa updated this revision to Diff 418895.
yihanaa added a comment.

Remove unnecessary code format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122662

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/dump-struct-builtin.c

Index: clang/test/CodeGen/dump-struct-builtin.c
===
--- clang/test/CodeGen/dump-struct-builtin.c
+++ clang/test/CodeGen/dump-struct-builtin.c
@@ -5,95 +5,111 @@
 
 // CHECK: @__const.unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"short a = %hd\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"short a = %hd\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U1:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit2.a = private unnamed_addr constant %struct.U2A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U2:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U2A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"unsigned short a = %hu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [28 x i8] c"unsigned short a = %hu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U2:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit3.a = private unnamed_addr constant %struct.U3A { i32 12 }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U3:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U3A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [12 x i8] c"int a = %d\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [16 x i8] c"int a = %d\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U3:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit4.a = private unnamed_addr constant %struct.U4A { i32 12 }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U4:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U4A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [21 x i8] c"unsigned int a = %u\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [25 x i8] c"unsigned int a = %u\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U4:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit5.a = private unnamed_addr constant %struct.U5A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U5A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"long a = %ld\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [18 x i8] c"long a = %ld\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U5:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit6.a = private unnamed_addr constant %struct.U6A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U6:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U6A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [23 x i8] c"unsigned long a = %lu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [27 x i8] c"unsigned long a = %lu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U6:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit7.a = private unnamed_addr constant %struct.U7A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U7:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U7A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [20 x i8] c"long long a = %lld\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"long long a = %lld\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U7:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit8.a = private unnamed_addr constant %struct.U8A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U8:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U8A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U8:@[0-9]+]] = private unnamed_addr constant [29 x i8] c"unsigned long long a = %llu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U8:@[0-9]+]] = private unnamed_addr constant [33 x i8] c"unsigned long long a = %llu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U8:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 

[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

2022-03-29 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 3 inline comments as done.
ymandel added a comment.

In D122231#3414109 , @xazax.hun wrote:

> Wow. This did take some iterations and I feel like I just added to the 
> confusion at some point :D But the latest iteration looks much simpler and 
> I'm confident it is right this time. Thanks!

Not at all -- I think you raised some really good questions! Ultimately, my 
move from implication in the flow condition to tying it directly to the value 
was the wrong turn, and your questions effectively highlighted the issues.  :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122231

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


[clang-tools-extra] f10cee9 - [clang-tidy] Fix test failing on 32-bit architectures due to missing `__int128_t`

2022-03-29 Thread Danny Mösch via cfe-commits

Author: Danny Mösch
Date: 2022-03-29T17:58:12+02:00
New Revision: f10cee91ae07022e602d6a47e504e86796d49a7d

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

LOG: [clang-tidy] Fix test failing on 32-bit architectures due to missing 
`__int128_t`

This was caused by ff60af91ac0bbab12dd5ff5dc9b78bc1636f2d86. The reason for the 
failure is that
the type `__int128_t` is not available on 32-bit architectures. So just exclude 
the test case if
128-bit integers are not available.

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
index 605243cf7a782..c93a9382d14c9 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
@@ -172,10 +172,7 @@ int Foo() { int A[T]; return sizeof(T); }
 template 
 int Bar() { T A[5]; return sizeof(A[0]) / sizeof(T); }
 // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: suspicious usage of sizeof 
pointer 'sizeof(T)/sizeof(T)'
-template <__int128_t N> 
-bool Baz() { return sizeof(A) < N; }
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: suspicious comparison of 
'sizeof(expr)' to a constant
-int Test3() { return Foo<42>() + Bar() + Baz<-1>(); }
+int Test3() { return Foo<42>() + Bar(); }
 
 static const char* kABC = "abc";
 static const wchar_t* kDEF = L"def";
@@ -289,6 +286,13 @@ int Test6() {
   return sum;
 }
 
+#ifdef __SIZEOF_INT128__
+template <__int128_t N> 
+bool Baz() { return sizeof(A) < N; }
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: suspicious comparison of 
'sizeof(expr)' to a constant
+bool Test7() { return Baz<-1>(); }
+#endif
+
 int ValidExpressions() {
   int A[] = {1, 2, 3, 4};
   static const char str[] = "hello";



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


[PATCH] D122544: Utilize comparison operation implemented in APInt

2022-03-29 Thread Danny Mösch via Phabricator via cfe-commits
SimplyDanny marked 3 inline comments as done.
SimplyDanny added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp:175
 // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: suspicious usage of sizeof 
pointer 'sizeof(T)/sizeof(T)'
-int Test3() { return Foo<42>() + Bar(); }
+template <__int128_t N> 
+bool Baz() { return sizeof(A) < N; }

aaron.ballman wrote:
> SimplyDanny wrote:
> > aaron.ballman wrote:
> > > aaronpuchert wrote:
> > > > This causes test failures on 32-bit architectures:
> > > > ```
> > > > /home/tcwg-buildbot/worker/clang-armv7-2stage/stage2/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-sizeof-expression.cpp.tmp.cpp:175:11:
> > > >  error: unknown type name '__int128_t' [clang-diagnostic-error]
> > > > template <__int128_t N> 
> > > >   ^
> > > > ```
> > > > 
> > > > It's probably best to wrap this in `#ifdef __SIZEOF_INT128__` lest we 
> > > > disable the entire test on the affected platforms.
> > > Good catch! @SimplyDanny, can you fix up the test?
> > Sure. I will fix it. Sorry for the issue ...
> > 
> > What is the process in this case? Push the fix directly to main or go the 
> > normal path via Phabricator review?
> When fixing a broken test, you can generally push directly to main. (And if 
> the fix requires thought, it's usually best to revert the broken commit from 
> main until you have the test fixed, then push the fixed commit back to main).
Done. Fix is [[ https://reviews.llvm.org/rGf10cee91ae07 | here ]].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122544

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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa added a comment.

Thanks for review @erichkeane ,maybe i should split this patch.




Comment at: clang/docs/ReleaseNotes.rst:93
   and `51641 `_.
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct have bitfield. Now it fixed.

erichkeane wrote:
> What is this from/for?  Is this left over from a previous patch?
The fixed by me in https://reviews.llvm.org/D122248, this modification appeared 
in multiple places in ReleaseNote, so I removed the redundant.



Comment at: clang/docs/ReleaseNotes.rst:113
 -
-- The builtin function __builtin_dump_struct would crash clang when the target 
-  struct have bitfield. Now it fixed, and __builtin_dump_struct support dump
-  the bitwidth of bitfields.
-  This fixes `Issue 54462 
`_.
+- The builtin function __builtin_dump_struct support dump constant array and 
the 
+  bitwidth of bitfields in a struct.

erichkeane wrote:
> "now supports dumping constant arrays" perhaps?
Yes,i add constant array support int this patch.



Comment at: clang/docs/ReleaseNotes.rst:114
+- The builtin function __builtin_dump_struct support dump constant array and 
the 
+  bitwidth of bitfields in a struct.
+  

erichkeane wrote:
> This should probably be a different entry than the bitfield one.
should i separate the two?



Comment at: clang/docs/ReleaseNotes.rst:155
 
-- Improve __builtin_dump_struct:
-

erichkeane wrote:
> Probably want a separate patch to fix these.
Okay


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122662

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


[PATCH] D122663: Mark identifier prefixes as substitutable

2022-03-29 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk created this revision.
hvdijk added a reviewer: rsmith.
hvdijk added a project: clang.
Herald added a project: All.
hvdijk requested review of this revision.
Herald added a subscriber: cfe-commits.

The Itanium C++ ABI says prefixes are substitutable. For most prefixes we 
already handle this: the manglePrefix(const DeclContext *, bool) and 
manglePrefix(QualType) overloads explicitly handles substitutions or defer to 
functions that handle substitutions on their behalf. The 
manglePrefix(NestedNameSpecifier *) overload, however, is different and handles 
some cases implicitly, but not all. The Identifier case was not handled; this 
change adds handling for it, as well as a test case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122663

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/CodeGenCXX/mangle.cpp


Index: clang/test/CodeGenCXX/mangle.cpp
===
--- clang/test/CodeGenCXX/mangle.cpp
+++ clang/test/CodeGenCXX/mangle.cpp
@@ -1155,3 +1155,15 @@
   // CHECK-LABEL: @_ZN6test601fIiEEvDTplL_ZNS_1aEEcvT__EE
   template void f(int);
 }
+
+namespace test61 {
+  struct X {
+struct Y {
+  using a = int;
+  using b = int;
+};
+  };
+  template  void f(typename T::Y::a, typename T::Y::b) {}
+  // CHECK-LABEL: @_ZN6test611fINS_1XEEEvNT_1Y1aENS3_1bE
+  template void f(int, int);
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -442,6 +442,7 @@
 private:
 
   bool mangleSubstitution(const NamedDecl *ND);
+  bool mangleSubstitution(NestedNameSpecifier *NNS);
   bool mangleSubstitution(QualType T);
   bool mangleSubstitution(TemplateName Template);
   bool mangleSubstitution(uintptr_t Ptr);
@@ -455,6 +456,11 @@
 
 addSubstitution(reinterpret_cast(ND));
   }
+  void addSubstitution(NestedNameSpecifier *NNS) {
+NNS = Context.getASTContext().getCanonicalNestedNameSpecifier(NNS);
+
+addSubstitution(reinterpret_cast(NNS));
+  }
   void addSubstitution(QualType T);
   void addSubstitution(TemplateName Template);
   void addSubstitution(uintptr_t Ptr);
@@ -2041,12 +2047,16 @@
 return;
 
   case NestedNameSpecifier::Identifier:
+if (mangleSubstitution(qualifier))
+  return;
+
 // Member expressions can have these without prefixes, but that
 // should end up in mangleUnresolvedPrefix instead.
 assert(qualifier->getPrefix());
 manglePrefix(qualifier->getPrefix());
 
 mangleSourceName(qualifier->getAsIdentifier());
+addSubstitution(qualifier);
 return;
   }
 
@@ -5962,6 +5972,11 @@
   return mangleSubstitution(reinterpret_cast(ND));
 }
 
+bool CXXNameMangler::mangleSubstitution(NestedNameSpecifier *NNS) {
+  NNS = Context.getASTContext().getCanonicalNestedNameSpecifier(NNS);
+  return mangleSubstitution(reinterpret_cast(NNS));
+}
+
 /// Determine whether the given type has any qualifiers that are relevant for
 /// substitutions.
 static bool hasMangledSubstitutionQualifiers(QualType T) {


Index: clang/test/CodeGenCXX/mangle.cpp
===
--- clang/test/CodeGenCXX/mangle.cpp
+++ clang/test/CodeGenCXX/mangle.cpp
@@ -1155,3 +1155,15 @@
   // CHECK-LABEL: @_ZN6test601fIiEEvDTplL_ZNS_1aEEcvT__EE
   template void f(int);
 }
+
+namespace test61 {
+  struct X {
+struct Y {
+  using a = int;
+  using b = int;
+};
+  };
+  template  void f(typename T::Y::a, typename T::Y::b) {}
+  // CHECK-LABEL: @_ZN6test611fINS_1XEEEvNT_1Y1aENS3_1bE
+  template void f(int, int);
+}
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -442,6 +442,7 @@
 private:
 
   bool mangleSubstitution(const NamedDecl *ND);
+  bool mangleSubstitution(NestedNameSpecifier *NNS);
   bool mangleSubstitution(QualType T);
   bool mangleSubstitution(TemplateName Template);
   bool mangleSubstitution(uintptr_t Ptr);
@@ -455,6 +456,11 @@
 
 addSubstitution(reinterpret_cast(ND));
   }
+  void addSubstitution(NestedNameSpecifier *NNS) {
+NNS = Context.getASTContext().getCanonicalNestedNameSpecifier(NNS);
+
+addSubstitution(reinterpret_cast(NNS));
+  }
   void addSubstitution(QualType T);
   void addSubstitution(TemplateName Template);
   void addSubstitution(uintptr_t Ptr);
@@ -2041,12 +2047,16 @@
 return;
 
   case NestedNameSpecifier::Identifier:
+if (mangleSubstitution(qualifier))
+  return;
+
 // Member expressions can have these without prefixes, but that
 // should end up in mangleUnresolvedPrefix instead.
 assert(qualifier->getPrefix());
 manglePrefix(qualifier->getPrefix());
 
 mangleSourceName(qualifier->getAsIdentifier());
+addSubstitution(qualifier);
 return;
   }
 
@@ -5962,6 +5972,11 @@
   return mangleSubstitution(rein

[PATCH] D121951: [AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5

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

In D121951#3411856 , @scott.linder 
wrote:

> @yaxunl Does excluding device-libs via COV_None make sense?

That should work. Hopefully, it can cover most spurious warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121951

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


[PATCH] D122663: Mark identifier prefixes as substitutable

2022-03-29 Thread Harald van Dijk via Phabricator via cfe-commits
hvdijk added a comment.

Question: does this need to be covered by `-fclang-abi-compat=` when the prior 
mangling resulted in names that even llvm-cxxfilt agreed made no sense? (If it 
is needed, it should be an easy change.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122663

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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa updated this revision to Diff 418901.
yihanaa added a comment.

split another patch to fix ReleaseNotes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122662

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/dump-struct-builtin.c

Index: clang/test/CodeGen/dump-struct-builtin.c
===
--- clang/test/CodeGen/dump-struct-builtin.c
+++ clang/test/CodeGen/dump-struct-builtin.c
@@ -5,95 +5,111 @@
 
 // CHECK: @__const.unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"short a = %hd\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"short a = %hd\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U1:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit2.a = private unnamed_addr constant %struct.U2A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U2:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U2A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"unsigned short a = %hu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [28 x i8] c"unsigned short a = %hu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U2:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit3.a = private unnamed_addr constant %struct.U3A { i32 12 }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U3:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U3A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [12 x i8] c"int a = %d\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [16 x i8] c"int a = %d\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U3:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit4.a = private unnamed_addr constant %struct.U4A { i32 12 }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U4:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U4A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [21 x i8] c"unsigned int a = %u\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [25 x i8] c"unsigned int a = %u\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U4:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit5.a = private unnamed_addr constant %struct.U5A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U5A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"long a = %ld\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [18 x i8] c"long a = %ld\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U5:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit6.a = private unnamed_addr constant %struct.U6A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U6:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U6A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [23 x i8] c"unsigned long a = %lu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [27 x i8] c"unsigned long a = %lu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U6:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit7.a = private unnamed_addr constant %struct.U7A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U7:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U7A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [20 x i8] c"long long a = %lld\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"long long a = %lld\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U7:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit8.a = private unnamed_addr constant %struct.U8A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U8:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U8A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U8:@[0-9]+]] = private unnamed_addr constant [29 x i8] c"unsigned long long a = %llu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U8:@[0-9]+]] = private unnamed_addr constant [33 x i8] c"unsigned long long a = %llu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U8:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.un

[PATCH] D121951: [AMDGPU][OpenCL] Add "amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5

2022-03-29 Thread Sameer Sahasrabuddhe via Phabricator via cfe-commits
sameerds added a comment.

In D121951#3411856 , @scott.linder 
wrote:

> @yaxunl Does excluding device-libs via COV_None make sense?
>
> @sameerds Would you still rather we just not add this attribute in the 
> frontend at all? I'm OK with it if that is the consensus

Yes, I still think there is no need to emit that attribute in the frontend. It 
will always be inferred by the Attributor when optimization is enabled. This 
also eliminates the check for COV_None and there seems to be some uncertainty 
about COV_None anyway. This also eliminates the updates to all the tests where 
the no-hostcall-ptr attribute does not actually matter. If ever we need to 
check if hostcall is being used on OpenCL and COV < 5, we should do it per 
feature and inform the user appropriately.




Comment at: llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp:406-408
 if (Func.getParent()->getNamedMetadata("llvm.printf.fmts"))
   emitKernelArg(DL, Int8PtrTy, Align(8), ValueKind::HiddenPrintfBuffer);
+else if (!Func.hasFnAttribute("amdgpu-no-hostcall-ptr"))

scott.linder wrote:
> sameerds wrote:
> > I would structure this differently: If this is code-object-v4 or lower, 
> > then if  "llvm.printf.fmts" is present, then this kernel clearly contains 
> > OpenCL bits, and cannot use hostcall. So it's okay to just assume that the 
> > no-hostcall-ptr attribute is always present in this situation, which means 
> > the only metadata generated is for ValueKind::HiddenPrintfBuffer. Else if 
> > this is code-object-v5, then proceed to emit both metadata.
> > 
> > 
> I'm not sure I follow; doesn't the code as-is implement what you're 
> describing?
> 
> If the printf metadata is present, this will only ever emit the 
> `HiddenPrintfBuffer`, irrespective of the hostcall attribute. Otherwise, this 
> respects `amdgpu-no-hostcall-ptr` (i.e. for HIP and other languages).
> 
> The "if this is code-object-v4 or lower" portion is implicit, as this is just 
> the `MetadataStreamerV2` impl. The `MetadataStreamerV3` (below) and 
> `MetadataStreamerV4` (inherits from V3) impls below are similar. The 
> `MetadataStreamerV5` impl is already correct for V5 (i.e. supports both 
> argument kinds for the same kernel).
Your last para about different streamers cleared the confusion. So, this looks 
good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121951

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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa added a comment.

In D122662#3414199 , @erichkeane 
wrote:

> This looks to be quite the large patch, can you split this up into individual 
> patches?  It isn't clear to me what all is going on everywhere.

Thanks @erichkeane ,i have splited this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122662

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


[clang] 3c84e4a - [C11] Improve the diagnostic when accessing a member of an atomic struct

2022-03-29 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-03-29T12:14:36-04:00
New Revision: 3c84e4a0dbd08fc03bbcdd8354a984e0efcf7672

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

LOG: [C11] Improve the diagnostic when accessing a member of an atomic struct

Member access for an atomic structure or union is unconditional
undefined behavior (C11 6.5.2.3p5). However, we would issue a confusing
error message about the base expression not being a structure or union
type.

GCC issues a warning for this case. Clang now warns as well, but the
warning is defaulted to an error because the actual access is still
unsafe.

This fixes Issue 54563.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExprMember.cpp
clang/test/Sema/atomic-expr.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9e12ad17beeb9..fb363ecb783df 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -85,6 +85,10 @@ Bug Fixes
 - Assignment expressions in C11 and later mode now properly strip the _Atomic
   qualifier when determining the type of the assignment expression. Fixes
   `Issue 48742 `_.
+- Improved the diagnostic when accessing a member of an atomic structure or
+  union object in C; was previously an unhelpful error, but now issues a
+  `-Watomic-access` warning which defaults to an error. Fixes
+  `Issue 54563 `_.
 - Unevaluated lambdas in dependant contexts no longer result in clang crashing.
   This fixes Issues `50376 
`_,
   `51414 `_,

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d7a532856c2f1..172a10a65c8c0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6439,6 +6439,9 @@ def err_atomic_specifier_bad_type
 "%select{incomplete |array |function |reference |atomic |qualified 
"
 "|sizeless ||integer }0type "
 "%1 %select{|||which is not trivially copyable|}0">;
+def warn_atomic_member_access : Warning<
+  "accessing a member of an atomic structure or union is undefined behavior">,
+  InGroup>, DefaultError;
 
 // Expressions.
 def ext_sizeof_alignof_function_type : Extension<

diff  --git a/clang/lib/Sema/SemaExprMember.cpp 
b/clang/lib/Sema/SemaExprMember.cpp
index ad0999de034fc..152b39dabe361 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -1292,6 +1292,20 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult 
&R,
 }
   }
 
+  // If the base type is an atomic type, this access is undefined behavior per
+  // C11 6.5.2.3p5. Instead of giving a typecheck error, we'll warn the user
+  // about the UB and recover by converting the atomic lvalue into a non-atomic
+  // lvalue. Because this is inherently unsafe as an atomic operation, the
+  // warning defaults to an error.
+  if (const auto *ATy = BaseType->getAs()) {
+S.Diag(OpLoc, diag::warn_atomic_member_access);
+BaseType = ATy->getValueType().getUnqualifiedType();
+BaseExpr = ImplicitCastExpr::Create(
+S.Context, IsArrow ? S.Context.getPointerType(BaseType) : BaseType,
+CK_AtomicToNonAtomic, BaseExpr.get(), nullptr,
+BaseExpr.get()->getValueKind(), FPOptionsOverride());
+  }
+
   // Handle field access to simple records.
   if (const RecordType *RTy = BaseType->getAs()) {
 TypoExpr *TE = nullptr;

diff  --git a/clang/test/Sema/atomic-expr.c b/clang/test/Sema/atomic-expr.c
index 097ed4fd39ee4..0dceadf4e705c 100644
--- a/clang/test/Sema/atomic-expr.c
+++ b/clang/test/Sema/atomic-expr.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -verify=off -fsyntax-only -Wno-atomic-access
+// off-no-diagnostics
 
 _Atomic(unsigned int) data1;
 int _Atomic data2;
@@ -75,3 +76,30 @@ void func_15(void) {
   _Static_assert(__builtin_types_compatible_p(__typeof__(x = 2), int), 
"incorrect");
   _Static_assert(__builtin_types_compatible_p(__typeof__(x += 2), int), 
"incorrect");
 }
+
+// Ensure that member access of an atomic structure or union type is properly
+// diagnosed as being undefined behavior; Issue 54563.
+void func_16(void) {
+  // LHS member access.
+  _Atomic struct { int val; } x, *xp;
+  x.val = 12;   // expected-error {{accessing a member of an atomic structure 
or union is undefined behavior}}
+  xp->val = 12; // expected-error {{accessing a member of an atomic structure 
or union is undefined

[PATCH] D122656: [C11] Improve the diagnostic when accessing a member of an atomic struct

2022-03-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman added a comment.

Thanks for the review, I've commit in 3c84e4a0dbd08fc03bbcdd8354a984e0efcf7672 



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

https://reviews.llvm.org/D122656

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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

I'd also suggest splitting into the '3' things that you're trying to accomplish 
above.  The CGBuiltin.cpp code has way too much going on to reasonably review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122662

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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa added a comment.

In D122662#3414284 , @erichkeane 
wrote:

> I'd also suggest splitting into the '3' things that you're trying to 
> accomplish above.  The CGBuiltin.cpp code has way too much going on to 
> reasonably review.

These three things are mainly done in a new function dumpArray, and the 
dumpRecord is modified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122662

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


[clang] 43b34c9 - Fix a test failure.

2022-03-29 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-03-29T12:27:23-04:00
New Revision: 43b34c981ad303e5e030c7c619cb54997c2fc341

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

LOG: Fix a test failure.

This amends 3c84e4a0dbd08fc03bbcdd8354a984e0efcf7672 which had an
unsaved change when committed.

Added: 


Modified: 
clang/test/Sema/atomic-expr.c

Removed: 




diff  --git a/clang/test/Sema/atomic-expr.c b/clang/test/Sema/atomic-expr.c
index 0dceadf4e705c..86cd32ce338c8 100644
--- a/clang/test/Sema/atomic-expr.c
+++ b/clang/test/Sema/atomic-expr.c
@@ -95,8 +95,8 @@ void func_16(void) {
   // RHS member access.
   int xval = x.val; // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
   xval = xp->val;   // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
-  int yval = y.val; // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
-  yval = yp->val;   // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+  int yval = y.ival; // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
+  yval = yp->ival;   // expected-error {{accessing a member of an atomic 
structure or union is undefined behavior}}
 
   // Using the type specifier instead of the type qualifier.
   _Atomic(struct { int val; }) z;



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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa added a comment.

In D122662#3414284 , @erichkeane 
wrote:

> I'd also suggest splitting into the '3' things that you're trying to 
> accomplish above.  The CGBuiltin.cpp code has way too much going on to 
> reasonably review.



In D122662#3414284 , @erichkeane 
wrote:

> I'd also suggest splitting into the '3' things that you're trying to 
> accomplish above.  The CGBuiltin.cpp code has way too much going on to 
> reasonably review.

Maybe I should split the third thing into another separate patch, the first 
thing needs to modify the indent when adding constant array support, that is 
the third thing (modify the indent), what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122662

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


[PATCH] D122648: [clang][extractapi] Tie API and serialization to the FrontendAction

2022-03-29 Thread Daniel Grumberg via Phabricator via cfe-commits
dang updated this revision to Diff 418907.
dang added a comment.

Some errors slipped by when splitting this out of the macros patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122648

Files:
  clang/include/clang/ExtractAPI/FrontendActions.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -41,9 +41,8 @@
 /// information.
 class ExtractAPIVisitor : public RecursiveASTVisitor {
 public:
-  explicit ExtractAPIVisitor(ASTContext &Context)
-  : Context(Context),
-API(Context.getTargetInfo().getTriple(), Context.getLangOpts()) {}
+  ExtractAPIVisitor(ASTContext &Context, APISet &API)
+  : Context(Context), API(API) {}
 
   const APISet &getAPI() const { return API; }
 
@@ -304,42 +303,38 @@
   }
 
   ASTContext &Context;
-  APISet API;
+  APISet &API;
 };
 
 class ExtractAPIConsumer : public ASTConsumer {
 public:
-  ExtractAPIConsumer(ASTContext &Context, StringRef ProductName,
- std::unique_ptr OS)
-  : Visitor(Context), ProductName(ProductName), OS(std::move(OS)) {}
+  ExtractAPIConsumer(ASTContext &Context, APISet &API)
+  : Visitor(Context, API) {}
 
   void HandleTranslationUnit(ASTContext &Context) override {
 // Use ExtractAPIVisitor to traverse symbol declarations in the context.
 Visitor.TraverseDecl(Context.getTranslationUnitDecl());
-
-// Setup a SymbolGraphSerializer to write out collected API information in
-// the Symbol Graph format.
-// FIXME: Make the kind of APISerializer configurable.
-SymbolGraphSerializer SGSerializer(Visitor.getAPI(), ProductName);
-SGSerializer.serialize(*OS);
   }
 
 private:
   ExtractAPIVisitor Visitor;
-  std::string ProductName;
-  std::unique_ptr OS;
 };
 
 } // namespace
 
 std::unique_ptr
 ExtractAPIAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
-  std::unique_ptr OS = CreateOutputFile(CI, InFile);
+  OS = CreateOutputFile(CI, InFile);
   if (!OS)
 return nullptr;
-  return std::make_unique(
-  CI.getASTContext(), CI.getInvocation().getFrontendOpts().ProductName,
-  std::move(OS));
+
+  ProductName = CI.getFrontendOpts().ProductName;
+
+  // Now that we have enough information about the language options and the
+  // target triple, let's create the APISet before anyone uses it.
+  API = std::make_unique(CI.getTarget().getTriple(), CI.getLangOpts());
+
+  return std::make_unique(CI.getASTContext(), *API);
 }
 
 bool ExtractAPIAction::PrepareToExecuteAction(CompilerInstance &CI) {
@@ -371,6 +366,18 @@
   return true;
 }
 
+void ExtractAPIAction::EndSourceFileAction() {
+  if (!OS)
+return;
+
+  // Setup a SymbolGraphSerializer to write out collected API information in
+  // the Symbol Graph format.
+  // FIXME: Make the kind of APISerializer configurable.
+  SymbolGraphSerializer SGSerializer(*API, ProductName);
+  SGSerializer.serialize(*OS);
+  OS->flush();
+}
+
 std::unique_ptr
 ExtractAPIAction::CreateOutputFile(CompilerInstance &CI, StringRef InFile) {
   std::unique_ptr OS =
Index: clang/include/clang/ExtractAPI/FrontendActions.h
===
--- clang/include/clang/ExtractAPI/FrontendActions.h
+++ clang/include/clang/ExtractAPI/FrontendActions.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_EXTRACTAPI_FRONTEND_ACTIONS_H
 #define LLVM_CLANG_EXTRACTAPI_FRONTEND_ACTIONS_H
 
+#include "clang/ExtractAPI/API.h"
 #include "clang/Frontend/FrontendAction.h"
 
 namespace clang {
@@ -25,11 +26,19 @@
  StringRef InFile) override;
 
 private:
+  /// A representation of the APIs this action extracts.
+  std::unique_ptr API;
+
+  /// A stream to the output file of this action.
+  std::unique_ptr OS;
+
+  /// The product this action is extracting API information for.
+  std::string ProductName;
+
   /// The synthesized input buffer that contains all the provided input header
   /// files.
   std::unique_ptr Buffer;
 
-public:
   /// Prepare to execute the action on the given CompilerInstance.
   ///
   /// This is called before executing the action on any inputs. This generates a
@@ -37,8 +46,15 @@
   /// list with it before actually executing the action.
   bool PrepareToExecuteAction(CompilerInstance &CI) override;
 
+  /// Called after executing the action on the synthesized input buffer.
+  ///
+  /// Note: Now that we have gathered all the API definitions to surface we can
+  /// emit them in this callback.
+  void EndSourceFileAction() override;
+
   static std::unique_ptr
   CreateOutputFile(CompilerInstance &CI, StringRef InFile);
+
   static StringRef getInputBufferName() { return ""; }
 };
 
___
cfe-commits mailing lis

[PATCH] D122611: [clang][extract-api] Add support for macros

2022-03-29 Thread Daniel Grumberg via Phabricator via cfe-commits
dang marked 7 inline comments as done.
dang added inline comments.



Comment at: clang/include/clang/ExtractAPI/API.h:117
 
-  GlobalRecord(GVKind Kind, StringRef Name, StringRef USR, PresumedLoc Loc,
+  GlobalRecord(StringRef Name, GVKind Kind, StringRef USR, PresumedLoc Loc,
const AvailabilityInfo &Availability, LinkageInfo Linkage,

zixuw wrote:
> zixuw wrote:
> > Now that we are re-ordering it, would it look more organized if we push 
> > `GVKind Kind` further down so that the two `StringRef`s could be adjacent, 
> > and we also keep a common list of arguments (`Name, USR, Loc, Availability, 
> > ..., SubHeading`) the same across all kinds of records upfront? So maybe 
> > push to the end after `SubHeading` and before `Signature`.
> Also it seems that it becomes a requirement for the `*Record` c'tors that 
> `Name` should be the first parameter in order to use the `addTopLevelRecord` 
> template, though that's hidden in an anonymous namespace in the 
> implementation file.
> How can we communicate or document this clearly for future extensions?
Good catch! Correct, it isn't really a requirement in the sense that we could 
pass it in twice but since this was the only place that needed the change I 
felt it would improve the ergonomics of `addTopLevelRecord` to make the change 
here.

The way I see it it depends on whether we want to make use of 
`addTopLevelRecord` to associate stuff in the relevant map using a key that 
isn't the name of the record. If we don't want to do that, it might make sense 
to make the maps sets, which would convey the intent a bit better IMHO, but 
that would be a follow up patch.

Alternatively if we want to keep the structure as is, the easiest thing to do 
is to put a comment on top of the APIRecord base class that says that derived 
classes need to put the Name parameter as the first thing.



Comment at: clang/lib/ExtractAPI/API.cpp:32
+RecordTy *
+addTopLevelRecord(MapVector> &RecordMap,
+  StringRef Name, CtorArgsTy &&...CtorArgs) {

zixuw wrote:
> Does it make sense to just directly make the `*RecordMap` types in `APISet` 
> as a `template using`?
Yup agreed!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122611

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


[PATCH] D122573: [TBAA] Emit distinct TBAA tags for pointers with different depths,types.

2022-03-29 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added a comment.

In D122573#3412109 , @rjmccall wrote:

> Hmm.  We know that the big picture here, distinguishing pointers by pointee 
> type, is going to be disruptive and will probably need a specific 
> enabling/disabling option.  I'm not sure that distinguishing only by pointer 
> depth is a minor enough step that it shouldn't be treated the same way; in 
> particular, it's going to start treating `void *` as incompatible with e.g. 
> `char **`.

@rjmccall do you have any suggestions how to further reduce the initial step?

In D122573#3413445 , @xbolva00 wrote:

> Can you provide some flag?

It should be controlled by the existing `-fno-strict-aliasing`  flag, but there 
should probably be a dedicated flag as well to opt in the additional 
annotations.

In D122573#3412233 , @efriedma wrote:

> I'm a little skeptical about making TBAA more aggressive.  In most 
> situations, we're probably talking about tiny performance gains, and 
> currently there's no good way to check whether a codebase suffers from type 
> punning issues (either with compile-time analysis or runtime 
> instrumentation).  Probably LLVM itself does, but we have no way of knowing.

Agreed, strict aliasing violations are already a problem with the current level 
of TBAA support and we regularly see mis-compiles when new optimizations get 
enabled due to violations in projects. Improving precision of TBAA annotation 
is likely to expose more violations But there are cases where the additional 
guarantees could make a difference and other compiler implementations make use 
of them, so I think it would be worthwhile to work towards improving TBAA. I 
think we've also been through other disruptive transitions, like more 
aggressively adding `mustprogress`, but those were probably on a somewhat 
smaller scale.

Maybe we should try improve our tooling to detect violations in parallel to the 
effort in this patch? I am planning on trying to resurrect the type-sanitizer 
patches. I've also been playing around with a simple tool that's inserting 
assertions to check 2 pointers are not equal if TBAA claims they are no alias. 
The latter seems to surface a few violations in code in `llvm-test-suite` and 
also in tablegen. That is even without the current patch applied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122573

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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D122662#3414319 , @yihanaa wrote:

> In D122662#3414284 , @erichkeane 
> wrote:
>
>> I'd also suggest splitting into the '3' things that you're trying to 
>> accomplish above.  The CGBuiltin.cpp code has way too much going on to 
>> reasonably review.
>
>
>
> In D122662#3414284 , @erichkeane 
> wrote:
>
>> I'd also suggest splitting into the '3' things that you're trying to 
>> accomplish above.  The CGBuiltin.cpp code has way too much going on to 
>> reasonably review.
>
> Maybe I should split the third thing into another separate patch, the first 
> thing needs to modify the indent when adding constant array support, that is 
> the third thing (modify the indent), what do you think?

I would suggest piling them from #3, to #2 to #1.  That is: "Change the 
anonymous printing" in the first, add the 'sub-element-tabbing' in 2nd, and 
'add constant array support' in 3rd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122662

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


[PATCH] D122662: [Clang][CodeGen] Add constant array support for __builtin_dump_sturct, and beautify dump format

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa added a comment.

In D122662#3414364 , @erichkeane 
wrote:

> In D122662#3414319 , @yihanaa wrote:
>
>> In D122662#3414284 , @erichkeane 
>> wrote:
>>
>>> I'd also suggest splitting into the '3' things that you're trying to 
>>> accomplish above.  The CGBuiltin.cpp code has way too much going on to 
>>> reasonably review.
>>
>>
>>
>> In D122662#3414284 , @erichkeane 
>> wrote:
>>
>>> I'd also suggest splitting into the '3' things that you're trying to 
>>> accomplish above.  The CGBuiltin.cpp code has way too much going on to 
>>> reasonably review.
>>
>> Maybe I should split the third thing into another separate patch, the first 
>> thing needs to modify the indent when adding constant array support, that is 
>> the third thing (modify the indent), what do you think?
>
> I would suggest piling them from #3, to #2 to #1.  That is: "Change the 
> anonymous printing" in the first, add the 'sub-element-tabbing' in 2nd, and 
> 'add constant array support' in 3rd.

Thanks for the suggestion @erichkeane  , I am trying to do this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122662

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


[PATCH] D122495: [clang][extract-api] Use correct language info from inputs

2022-03-29 Thread Daniel Grumberg via Phabricator via cfe-commits
dang accepted this revision.
dang 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/D122495/new/

https://reviews.llvm.org/D122495

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


[PATCH] D122495: [clang][extract-api] Use correct language info from inputs

2022-03-29 Thread Zixu Wang 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 rG15bf0e567375: [clang][extract-api] Use correct language info 
from inputs (authored by zixuw).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122495

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/language.c

Index: clang/test/ExtractAPI/language.c
===
--- /dev/null
+++ clang/test/ExtractAPI/language.c
@@ -0,0 +1,166 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/c.reference.output.json.in >> \
+// RUN: %t/c.reference.output.json
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/objc.reference.output.json.in >> \
+// RUN: %t/objc.reference.output.json
+
+// RUN: %clang -extract-api -x c-header -target arm64-apple-macosx \
+// RUN: %t/c.h -o %t/c.output.json | FileCheck -allow-empty %s
+// RUN: %clang -extract-api -x objective-c-header -target arm64-apple-macosx \
+// RUN: %t/objc.h -o %t/objc.output.json | FileCheck -allow-empty %s
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/c.output.json >> %t/c.output-normalized.json
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/objc.output.json >> %t/objc.output-normalized.json
+
+// RUN: diff %t/c.reference.output.json %t/c.output-normalized.json
+// RUN: diff %t/objc.reference.output.json %t/objc.output-normalized.json
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:
+
+//--- c.h
+char c;
+
+//--- objc.h
+char objc;
+
+//--- c.reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationhips": [],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:C",
+  "spelling": "char"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "c"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "c",
+"precise": "c:@c"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "c.var"
+  },
+  "location": {
+"character": 6,
+"line": 1,
+"uri": "file://INPUT_DIR/c.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "c"
+  }
+],
+"title": "c"
+  }
+}
+  ]
+}
+//--- objc.reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationhips": [],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:C",
+  "spelling": "char"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "objc"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:@objc"
+  },
+  "kind": {
+"displayName": "Global Variable",
+"identifier": "objective-c.var"
+  },
+  "location": {
+"character": 6,
+"line": 1,
+"uri": "file://INPUT_DIR/objc.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "objc"
+  }
+],
+"title": "objc"
+  }
+}
+  ]
+}
Index: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
===
--- clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -151,11 +151,9 @@
   return Availbility;
 }
 
-/// Get the short language name string for interface language references.
-Stri

[clang] 15bf0e5 - [clang][extract-api] Use correct language info from inputs

2022-03-29 Thread Zixu Wang via cfe-commits

Author: Zixu Wang
Date: 2022-03-29T10:06:08-07:00
New Revision: 15bf0e567375c977cf7d7b48465eb1561e890b54

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

LOG: [clang][extract-api] Use correct language info from inputs

The current way of getting the `clang::Language` from `LangOptions` does
not handle Objective-C correctly because `clang::Language::ObjC` does
not correspond to any `LangStandard`. This patch passes the correct
`Language` from the frontend input information.

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

Added: 
clang/test/ExtractAPI/language.c

Modified: 
clang/include/clang/ExtractAPI/API.h
clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

Removed: 




diff  --git a/clang/include/clang/ExtractAPI/API.h 
b/clang/include/clang/ExtractAPI/API.h
index eb72450f87d23..b22d110475908 100644
--- a/clang/include/clang/ExtractAPI/API.h
+++ b/clang/include/clang/ExtractAPI/API.h
@@ -309,8 +309,8 @@ class APISet {
   /// Get the target triple for the ExtractAPI invocation.
   const llvm::Triple &getTarget() const { return Target; }
 
-  /// Get the language options used to parse the APIs.
-  const LangOptions &getLangOpts() const { return LangOpts; }
+  /// Get the language used by the APIs.
+  Language getLanguage() const { return Lang; }
 
   const GlobalRecordMap &getGlobals() const { return Globals; }
   const EnumRecordMap &getEnums() const { return Enums; }
@@ -328,8 +328,8 @@ class APISet {
   /// \returns a StringRef of the copied string in APISet::Allocator.
   StringRef copyString(StringRef String);
 
-  APISet(const llvm::Triple &Target, const LangOptions &LangOpts)
-  : Target(Target), LangOpts(LangOpts) {}
+  APISet(const llvm::Triple &Target, Language Lang)
+  : Target(Target), Lang(Lang) {}
 
 private:
   /// BumpPtrAllocator to store generated/copied strings.
@@ -338,7 +338,7 @@ class APISet {
   llvm::BumpPtrAllocator StringAllocator;
 
   const llvm::Triple Target;
-  const LangOptions LangOpts;
+  const Language Lang;
 
   GlobalRecordMap Globals;
   EnumRecordMap Enums;

diff  --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp 
b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
index 0636e6de7cc26..dc3e067a837e2 100644
--- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -41,9 +41,8 @@ namespace {
 /// information.
 class ExtractAPIVisitor : public RecursiveASTVisitor {
 public:
-  explicit ExtractAPIVisitor(ASTContext &Context)
-  : Context(Context),
-API(Context.getTargetInfo().getTriple(), Context.getLangOpts()) {}
+  ExtractAPIVisitor(ASTContext &Context, Language Lang)
+  : Context(Context), API(Context.getTargetInfo().getTriple(), Lang) {}
 
   const APISet &getAPI() const { return API; }
 
@@ -309,9 +308,9 @@ class ExtractAPIVisitor : public 
RecursiveASTVisitor {
 
 class ExtractAPIConsumer : public ASTConsumer {
 public:
-  ExtractAPIConsumer(ASTContext &Context, StringRef ProductName,
+  ExtractAPIConsumer(ASTContext &Context, StringRef ProductName, Language Lang,
  std::unique_ptr OS)
-  : Visitor(Context), ProductName(ProductName), OS(std::move(OS)) {}
+  : Visitor(Context, Lang), ProductName(ProductName), OS(std::move(OS)) {}
 
   void HandleTranslationUnit(ASTContext &Context) override {
 // Use ExtractAPIVisitor to traverse symbol declarations in the context.
@@ -339,6 +338,7 @@ ExtractAPIAction::CreateASTConsumer(CompilerInstance &CI, 
StringRef InFile) {
 return nullptr;
   return std::make_unique(
   CI.getASTContext(), CI.getInvocation().getFrontendOpts().ProductName,
+  CI.getFrontendOpts().Inputs.back().getKind().getLanguage(),
   std::move(OS));
 }
 

diff  --git a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp 
b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
index 54276f6cf1de1..cde81ecf0abd4 100644
--- a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -151,11 +151,9 @@ Optional serializeAvailability(const 
AvailabilityInfo &Avail) {
   return Availbility;
 }
 
-/// Get the short language name string for interface language references.
-StringRef getLanguageName(const LangOptions &LangOpts) {
-  auto LanguageKind =
-  LangStandard::getLangStandardForKind(LangOpts.LangStd).getLanguage();
-  switch (LanguageKind) {
+/// Get the language name string for interface language references.
+StringRef getLanguageName(Language Lang) {
+  switch (Lang) {
   case Language::C:
 return "c";
   case Language::ObjC:
@@ -185,11 +183,10 @@ StringRef getLanguageName(const LangOptions &LangOpts) {
 ///
 /// The identifier property of a symbol c

[PATCH] D122573: [TBAA] Emit distinct TBAA tags for pointers with different depths,types.

2022-03-29 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

>> Maybe we should try improve our tooling to detect violations in parallel to 
>> the effort in this patch? I am planning on trying to resurrect the 
>> type-sanitizer patches. I've also been playing around with a simple tool 
>> that's inserting assertions to check 2 pointers are not equal if TBAA claims 
>> they are no alias. The latter seems to surface a few violations in code in 
>> llvm-test-suite and also in tablegen. That is even without the current patch 
>> applied.

Amazing!

Even your simple tool could be very useful, this functionality could be mapped 
to something like gcc’s -Wstrict-aliasing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122573

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


[PATCH] D122611: [clang][extract-api] Add support for macros

2022-03-29 Thread Daniel Grumberg via Phabricator via cfe-commits
dang marked an inline comment as done.
dang added inline comments.



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:446-457
+void ExtractAPIAction::EndSourceFileAction() {
+  if (!OS)
+return;
+
+  // Setup a SymbolGraphSerializer to write out collected API information in
+  // the Symbol Graph format.
+  // FIXME: Make the kind of APISerializer configurable.

zixuw wrote:
> Worth moving this change and the `APISet` move and relevant structural 
> changes into its own patch, as mentioned in D122446.
https://reviews.llvm.org/D122648


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122611

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


[PATCH] D122668: [Clang][doc][NFC]Remove duplicate items in ReleaseNotes

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa created this revision.
yihanaa added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
yihanaa requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Remove duplicate items in ReleaseNotes for __builtin_dump_struct, the code 
changes int patch https://reviews.llvm.org/D122248


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122668

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -90,7 +90,9 @@
   `51414 `_,
   `51416 `_,
   and `51641 `_.
-
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct have bitfield. Now it fixed.
+  This fixes Issue `Issue 54462 
`_.
 
 Improvements to Clang's diagnostics
 ^^^
@@ -107,10 +109,11 @@
 
 Non-comprehensive list of changes in this release
 -
-- The builtin function __builtin_dump_struct would crash clang when the target 
-  struct have bitfield. Now it fixed, and __builtin_dump_struct support dump
-  the bitwidth of bitfields.
-  This fixes `Issue 54462 
`_.
+- Improve __builtin_dump_struct:
+
+  - Support bitfields in struct and union.
+  
+  - Improve the dump format, dump both bitwidth(if its a bitfield) and field 
value.
 
 New Compiler Flags
 --
@@ -152,12 +155,6 @@
 - The ``__declspec(naked)`` attribute can no longer be written on a member
   function in Microsoft compatibility mode, matching the behavior of cl.exe.
 
-- Improve __builtin_dump_struct:
-
-  - Support bitfields in struct and union.
-  
-  - Improve the dump format, dump both bitwidth(if its a bitfield) and field 
value.
-
 Windows Support
 ---
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -90,7 +90,9 @@
   `51414 `_,
   `51416 `_,
   and `51641 `_.
-
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct have bitfield. Now it fixed.
+  This fixes Issue `Issue 54462 `_.
 
 Improvements to Clang's diagnostics
 ^^^
@@ -107,10 +109,11 @@
 
 Non-comprehensive list of changes in this release
 -
-- The builtin function __builtin_dump_struct would crash clang when the target 
-  struct have bitfield. Now it fixed, and __builtin_dump_struct support dump
-  the bitwidth of bitfields.
-  This fixes `Issue 54462 `_.
+- Improve __builtin_dump_struct:
+
+  - Support bitfields in struct and union.
+  
+  - Improve the dump format, dump both bitwidth(if its a bitfield) and field value.
 
 New Compiler Flags
 --
@@ -152,12 +155,6 @@
 - The ``__declspec(naked)`` attribute can no longer be written on a member
   function in Microsoft compatibility mode, matching the behavior of cl.exe.
 
-- Improve __builtin_dump_struct:
-
-  - Support bitfields in struct and union.
-  
-  - Improve the dump format, dump both bitwidth(if its a bitfield) and field value.
-
 Windows Support
 ---
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122668: [Clang][doc][NFC]Remove duplicate items in ReleaseNotes

2022-03-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

2 small bits, otherwise LGTM.




Comment at: clang/docs/ReleaseNotes.rst:94
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct have bitfield. Now it fixed.
+  This fixes Issue `Issue 54462 
`_.





Comment at: clang/docs/ReleaseNotes.rst:113
+- Improve __builtin_dump_struct:
+
+  - Support bitfields in struct and union.

extranious newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122668

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


[PATCH] D122669: [AMDGPU][OpenCL] Remove "printf and hostcall" diagnostic

2022-03-29 Thread Scott Linder via Phabricator via cfe-commits
scott.linder created this revision.
Herald added subscribers: hsmhsm, foad, Naghasan, ldrumm, kerbowa, hiraditya, 
t-tye, Anastasia, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl, arsenm.
Herald added a project: All.
scott.linder requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, wdng.
Herald added projects: clang, LLVM.

The diagnostic is unreliable, and triggers even for dead uses of
hostcall that may exist when linking the device-libs at lower
optimization levels.

Eliminate the diagnostic, and directly document the limitation for
OpenCL before code object V5.

Make some NFC changes to clarify the related code in the
MetadataStreamer.

Add a clang test to tie OCL sources containing printf to the backend IR
tests for this situation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122669

Files:
  clang/test/CodeGenOpenCL/amdgpu-printf.cl
  llvm/docs/AMDGPUUsage.rst
  llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
  llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
  llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
  llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll

Index: llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll
===
--- llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll
+++ /dev/null
@@ -1,18 +0,0 @@
-; RUN: not opt -S -mtriple=amdgcn-unknown-unknown -amdgpu-printf-runtime-binding < %s 2>&1 | FileCheck %s
-
-@.str = private unnamed_addr addrspace(2) constant [6 x i8] c"%s:%d\00", align 1
-
-define amdgpu_kernel void @test_kernel(i32 %n) {
-entry:
-  %str = alloca [9 x i8], align 1
-  %arraydecay = getelementptr inbounds [9 x i8], [9 x i8]* %str, i32 0, i32 0
-  %call1 = call i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(2)* @.str, i32 0, i32 0), i8* %arraydecay, i32 %n)
-  %call2 = call <2 x i64> (i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64) @__ockl_hostcall_internal(i8* undef, i32 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9)
-  ret void
-}
-
-declare i32 @printf(i8 addrspace(2)*, ...)
-
-declare <2 x i64> @__ockl_hostcall_internal(i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64)
-
-; CHECK: error: Cannot use both printf and hostcall in the same module
Index: llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
@@ -0,0 +1,19 @@
+; RUN: opt -S -mtriple=amdgcn-unknown-unknown -amdgpu-printf-runtime-binding < %s 2>&1 | FileCheck %s
+
+@.str = private unnamed_addr addrspace(4) constant [6 x i8] c"%s:%d\00", align 1
+
+define amdgpu_kernel void @test_kernel(i32 %n) {
+entry:
+  %str = alloca [9 x i8], align 1, addrspace(5)
+  %arraydecay = getelementptr inbounds [9 x i8], [9 x i8] addrspace(5)* %str, i32 0, i32 0
+  %call1 = call i32 (i8 addrspace(4)*, ...) @printf(i8 addrspace(4)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(4)* @.str, i32 0, i32 0), i8 addrspace(5)* %arraydecay, i32 %n)
+  %call2 = call <2 x i64> (i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64) @__ockl_hostcall_internal(i8* undef, i32 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9)
+  ret void
+}
+
+declare i32 @printf(i8 addrspace(4)*, ...)
+
+declare <2 x i64> @__ockl_hostcall_internal(i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64)
+
+; CHECK-NOT: error:
+; CHECK-NOT: warning:
Index: llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
@@ -563,15 +563,6 @@
   if (Printfs.empty())
 return false;
 
-  if (auto HostcallFunction = M.getFunction("__ockl_hostcall_internal")) {
-for (auto &U : HostcallFunction->uses()) {
-  if (auto *CI = dyn_cast(U.getUser())) {
-M.getContext().emitError(
-CI, "Cannot use both printf and hostcall in the same module");
-  }
-}
-  }
-
   TD = &M.getDataLayout();
 
   return lowerPrintfForGpu(M);
Index: llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
@@ -401,17 +401,15 @@
   auto Int8PtrTy = Type::getInt8PtrTy(Func.getContext(),
   AMDGPUAS::GLOBAL_ADDRESS);
 
-  // Emit "printf buffer" argument if printf is used, otherwise emit dummy
-  // "none" argument.
   if (HiddenArgNumBytes >= 32) {
+// We forbid the use of features requiring hostcall when compiling OpenCL
+// before code object V5, which makes the mutual exclusion between the
+// "printf buffer" and "hostcall buffer" here sound.
 if (Func.getParent()->getNamedMetadata("llvm.printf.fmts"))
   em

[PATCH] D122668: [Clang][doc][NFC]Remove duplicate items in ReleaseNotes

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa updated this revision to Diff 418920.
yihanaa added a comment.

Fixed "Description" issue and removed useless newlines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122668

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -90,7 +90,9 @@
   `51414 `_,
   `51416 `_,
   and `51641 `_.
-
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct contains a bitfield. It now correctly handles bitfields.
+  This fixes Issue `Issue 54462 
`_.
 
 Improvements to Clang's diagnostics
 ^^^
@@ -107,10 +109,9 @@
 
 Non-comprehensive list of changes in this release
 -
-- The builtin function __builtin_dump_struct would crash clang when the target 
-  struct have bitfield. Now it fixed, and __builtin_dump_struct support dump
-  the bitwidth of bitfields.
-  This fixes `Issue 54462 
`_.
+- Improve __builtin_dump_struct:
+  - Support bitfields in struct and union.
+  - Improve the dump format, dump both bitwidth(if its a bitfield) and field 
value.
 
 New Compiler Flags
 --
@@ -152,12 +153,6 @@
 - The ``__declspec(naked)`` attribute can no longer be written on a member
   function in Microsoft compatibility mode, matching the behavior of cl.exe.
 
-- Improve __builtin_dump_struct:
-
-  - Support bitfields in struct and union.
-  
-  - Improve the dump format, dump both bitwidth(if its a bitfield) and field 
value.
-
 Windows Support
 ---
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -90,7 +90,9 @@
   `51414 `_,
   `51416 `_,
   and `51641 `_.
-
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct contains a bitfield. It now correctly handles bitfields.
+  This fixes Issue `Issue 54462 `_.
 
 Improvements to Clang's diagnostics
 ^^^
@@ -107,10 +109,9 @@
 
 Non-comprehensive list of changes in this release
 -
-- The builtin function __builtin_dump_struct would crash clang when the target 
-  struct have bitfield. Now it fixed, and __builtin_dump_struct support dump
-  the bitwidth of bitfields.
-  This fixes `Issue 54462 `_.
+- Improve __builtin_dump_struct:
+  - Support bitfields in struct and union.
+  - Improve the dump format, dump both bitwidth(if its a bitfield) and field value.
 
 New Compiler Flags
 --
@@ -152,12 +153,6 @@
 - The ``__declspec(naked)`` attribute can no longer be written on a member
   function in Microsoft compatibility mode, matching the behavior of cl.exe.
 
-- Improve __builtin_dump_struct:
-
-  - Support bitfields in struct and union.
-  
-  - Improve the dump format, dump both bitwidth(if its a bitfield) and field value.
-
 Windows Support
 ---
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122668: [Clang][doc][NFC]Remove duplicate items in ReleaseNotes

2022-03-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.

LGTM.  Let me know if you need this committed for you, just provide "name 
" for the commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122668

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


[PATCH] D122668: [Clang][doc][NFC]Remove duplicate items in ReleaseNotes

2022-03-29 Thread Wang Yihan via Phabricator via cfe-commits
yihanaa marked 2 inline comments as done.
yihanaa added a comment.

In D122668#3414445 , @erichkeane 
wrote:

> 2 small bits, otherwise LGTM.

Thanks for review @erichkeane ,i have update the diff, but i don’t have commit 
access, can you land this patch for me? Please use “wangyihan 
1135831...@qq.com” to commit the change.😉


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122668

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


[PATCH] D121951: [AMDGPU][OpenCL] Remove "printf and hostcall" diagnostic

2022-03-29 Thread Scott Linder via Phabricator via cfe-commits
scott.linder updated this revision to Diff 418922.
scott.linder retitled this revision from "[AMDGPU][OpenCL] Add 
"amdgpu-no-hostcall-ptr" in Clang codegen pre-COV_5" to "[AMDGPU][OpenCL] 
Remove "printf and hostcall" diagnostic".
scott.linder edited the summary of this revision.
scott.linder added a comment.

Remove frontend changes, and just drop the diagnostic in the backend. Update
some comments and naming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121951

Files:
  clang/test/CodeGenOpenCL/amdgpu-printf.cl
  llvm/docs/AMDGPUUsage.rst
  llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
  llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
  llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
  llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll

Index: llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll
===
--- llvm/test/CodeGen/AMDGPU/opencl-printf-no-hostcall.ll
+++ /dev/null
@@ -1,18 +0,0 @@
-; RUN: not opt -S -mtriple=amdgcn-unknown-unknown -amdgpu-printf-runtime-binding < %s 2>&1 | FileCheck %s
-
-@.str = private unnamed_addr addrspace(2) constant [6 x i8] c"%s:%d\00", align 1
-
-define amdgpu_kernel void @test_kernel(i32 %n) {
-entry:
-  %str = alloca [9 x i8], align 1
-  %arraydecay = getelementptr inbounds [9 x i8], [9 x i8]* %str, i32 0, i32 0
-  %call1 = call i32 (i8 addrspace(2)*, ...) @printf(i8 addrspace(2)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(2)* @.str, i32 0, i32 0), i8* %arraydecay, i32 %n)
-  %call2 = call <2 x i64> (i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64) @__ockl_hostcall_internal(i8* undef, i32 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9)
-  ret void
-}
-
-declare i32 @printf(i8 addrspace(2)*, ...)
-
-declare <2 x i64> @__ockl_hostcall_internal(i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64)
-
-; CHECK: error: Cannot use both printf and hostcall in the same module
Index: llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/opencl-printf-and-hostcall.ll
@@ -0,0 +1,19 @@
+; RUN: opt -S -mtriple=amdgcn-unknown-unknown -amdgpu-printf-runtime-binding < %s 2>&1 | FileCheck %s
+
+@.str = private unnamed_addr addrspace(4) constant [6 x i8] c"%s:%d\00", align 1
+
+define amdgpu_kernel void @test_kernel(i32 %n) {
+entry:
+  %str = alloca [9 x i8], align 1, addrspace(5)
+  %arraydecay = getelementptr inbounds [9 x i8], [9 x i8] addrspace(5)* %str, i32 0, i32 0
+  %call1 = call i32 (i8 addrspace(4)*, ...) @printf(i8 addrspace(4)* getelementptr inbounds ([6 x i8], [6 x i8] addrspace(4)* @.str, i32 0, i32 0), i8 addrspace(5)* %arraydecay, i32 %n)
+  %call2 = call <2 x i64> (i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64) @__ockl_hostcall_internal(i8* undef, i32 1, i64 2, i64 3, i64 4, i64 5, i64 6, i64 7, i64 8, i64 9)
+  ret void
+}
+
+declare i32 @printf(i8 addrspace(4)*, ...)
+
+declare <2 x i64> @__ockl_hostcall_internal(i8*, i32, i64, i64, i64, i64, i64, i64, i64, i64)
+
+; CHECK-NOT: error:
+; CHECK-NOT: warning:
Index: llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp
@@ -563,15 +563,6 @@
   if (Printfs.empty())
 return false;
 
-  if (auto HostcallFunction = M.getFunction("__ockl_hostcall_internal")) {
-for (auto &U : HostcallFunction->uses()) {
-  if (auto *CI = dyn_cast(U.getUser())) {
-M.getContext().emitError(
-CI, "Cannot use both printf and hostcall in the same module");
-  }
-}
-  }
-
   TD = &M.getDataLayout();
 
   return lowerPrintfForGpu(M);
Index: llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
@@ -401,17 +401,15 @@
   auto Int8PtrTy = Type::getInt8PtrTy(Func.getContext(),
   AMDGPUAS::GLOBAL_ADDRESS);
 
-  // Emit "printf buffer" argument if printf is used, otherwise emit dummy
-  // "none" argument.
   if (HiddenArgNumBytes >= 32) {
+// We forbid the use of features requiring hostcall when compiling OpenCL
+// before code object V5, which makes the mutual exclusion between the
+// "printf buffer" and "hostcall buffer" here sound.
 if (Func.getParent()->getNamedMetadata("llvm.printf.fmts"))
   emitKernelArg(DL, Int8PtrTy, Align(8), ValueKind::HiddenPrintfBuffer);
-else if (!Func.hasFnAttribute("amdgpu-no-hostcall-ptr")) {
-  // The printf runtime binding pass should have ensured that hostcall and
-  // printf are not used in the same module.
-  assert(!Func.getParent()->getNamedM

[PATCH] D121951: [AMDGPU][OpenCL] Remove "printf and hostcall" diagnostic

2022-03-29 Thread Scott Linder via Phabricator via cfe-commits
scott.linder added a comment.

In D121951#3414271 , @sameerds wrote:

> In D121951#3411856 , @scott.linder 
> wrote:
>
>> @yaxunl Does excluding device-libs via COV_None make sense?
>>
>> @sameerds Would you still rather we just not add this attribute in the 
>> frontend at all? I'm OK with it if that is the consensus
>
> Yes, I still think there is no need to emit that attribute in the frontend. 
> It will always be inferred by the Attributor when optimization is enabled. 
> This also eliminates the check for COV_None and there seems to be some 
> uncertainty about COV_None anyway. This also eliminates the updates to all 
> the tests where the no-hostcall-ptr attribute does not actually matter. If 
> ever we need to check if hostcall is being used on OpenCL and COV < 5, we 
> should do it per feature and inform the user appropriately.

OK, that makes sense to me. I think I was not interpreting the purpose of the 
attributes correctly, but if they are essentially just optimization remarks 
then I don't have any issue with not ensuring they are present at -O0.

I updated the patch, let me know what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121951

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


[clang] 6c32075 - [Clang][doc][NFC]Remove duplicate items in ReleaseNotes

2022-03-29 Thread Erich Keane via cfe-commits

Author: wangyihan
Date: 2022-03-29T10:30:34-07:00
New Revision: 6c32075d29d4fe3985aa8a98b23c33902190a15d

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

LOG: [Clang][doc][NFC]Remove duplicate items in ReleaseNotes

Remove duplicate items in ReleaseNotes for __builtin_dump_struct, the
code changes int patch https://reviews.llvm.org/D122248

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index fb363ecb783df..e893f5b4627c7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -94,7 +94,9 @@ Bug Fixes
   `51414 `_,
   `51416 `_,
   and `51641 `_.
-
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct contains a bitfield. It now correctly handles bitfields.
+  This fixes Issue `Issue 54462 
`_.
 
 Improvements to Clang's diagnostics
 ^^^
@@ -111,10 +113,9 @@ Improvements to Clang's diagnostics
 
 Non-comprehensive list of changes in this release
 -
-- The builtin function __builtin_dump_struct would crash clang when the target 
-  struct have bitfield. Now it fixed, and __builtin_dump_struct support dump
-  the bitwidth of bitfields.
-  This fixes `Issue 54462 
`_.
+- Improve __builtin_dump_struct:
+  - Support bitfields in struct and union.
+  - Improve the dump format, dump both bitwidth(if its a bitfield) and field 
value.
 
 New Compiler Flags
 --
@@ -156,12 +157,6 @@ Attribute Changes in Clang
 - The ``__declspec(naked)`` attribute can no longer be written on a member
   function in Microsoft compatibility mode, matching the behavior of cl.exe.
 
-- Improve __builtin_dump_struct:
-
-  - Support bitfields in struct and union.
-  
-  - Improve the dump format, dump both bitwidth(if its a bitfield) and field 
value.
-
 Windows Support
 ---
 



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


[PATCH] D122668: [Clang][doc][NFC]Remove duplicate items in ReleaseNotes

2022-03-29 Thread Erich Keane 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 rG6c32075d29d4: [Clang][doc][NFC]Remove duplicate items in 
ReleaseNotes (authored by yihanaa, committed by erichkeane).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122668

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -94,7 +94,9 @@
   `51414 `_,
   `51416 `_,
   and `51641 `_.
-
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct contains a bitfield. It now correctly handles bitfields.
+  This fixes Issue `Issue 54462 
`_.
 
 Improvements to Clang's diagnostics
 ^^^
@@ -111,10 +113,9 @@
 
 Non-comprehensive list of changes in this release
 -
-- The builtin function __builtin_dump_struct would crash clang when the target 
-  struct have bitfield. Now it fixed, and __builtin_dump_struct support dump
-  the bitwidth of bitfields.
-  This fixes `Issue 54462 
`_.
+- Improve __builtin_dump_struct:
+  - Support bitfields in struct and union.
+  - Improve the dump format, dump both bitwidth(if its a bitfield) and field 
value.
 
 New Compiler Flags
 --
@@ -156,12 +157,6 @@
 - The ``__declspec(naked)`` attribute can no longer be written on a member
   function in Microsoft compatibility mode, matching the behavior of cl.exe.
 
-- Improve __builtin_dump_struct:
-
-  - Support bitfields in struct and union.
-  
-  - Improve the dump format, dump both bitwidth(if its a bitfield) and field 
value.
-
 Windows Support
 ---
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -94,7 +94,9 @@
   `51414 `_,
   `51416 `_,
   and `51641 `_.
-
+- The builtin function __builtin_dump_struct would crash clang when the target 
+  struct contains a bitfield. It now correctly handles bitfields.
+  This fixes Issue `Issue 54462 `_.
 
 Improvements to Clang's diagnostics
 ^^^
@@ -111,10 +113,9 @@
 
 Non-comprehensive list of changes in this release
 -
-- The builtin function __builtin_dump_struct would crash clang when the target 
-  struct have bitfield. Now it fixed, and __builtin_dump_struct support dump
-  the bitwidth of bitfields.
-  This fixes `Issue 54462 `_.
+- Improve __builtin_dump_struct:
+  - Support bitfields in struct and union.
+  - Improve the dump format, dump both bitwidth(if its a bitfield) and field value.
 
 New Compiler Flags
 --
@@ -156,12 +157,6 @@
 - The ``__declspec(naked)`` attribute can no longer be written on a member
   function in Microsoft compatibility mode, matching the behavior of cl.exe.
 
-- Improve __builtin_dump_struct:
-
-  - Support bitfields in struct and union.
-  
-  - Improve the dump format, dump both bitwidth(if its a bitfield) and field value.
-
 Windows Support
 ---
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122446: [clang][extract-api] Add Objective-C interface support

2022-03-29 Thread Zixu Wang via Phabricator via cfe-commits
zixuw marked an inline comment as done.
zixuw added inline comments.



Comment at: clang/lib/ExtractAPI/DeclarationFragments.cpp:642-648
+// Instantiate template for FunctionDecl.
+template FunctionSignature
+DeclarationFragmentsBuilder::getFunctionSignature(const FunctionDecl *);
+
+// Instantiate template for ObjCMethodDecl.
+template FunctionSignature
+DeclarationFragmentsBuilder::getFunctionSignature(const ObjCMethodDecl *);

dang wrote:
> Do we need to explicitly instantiate them? Wouldn't they get instantiated as 
> needed?
Yeah since we are implementing the templated method here instead of in the 
header file, we need to explicitly instantiate it so that the symbol ends up in 
`DeclarationFragments.o` and picked up by the linker.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:375
+  case APIRecord::RK_ObjCIvar:
+Kind["identifier"] = AddLangPrefix("ivar");
+Kind["displayName"] = "Instance Variable";

dang wrote:
> zixuw wrote:
> > dang wrote:
> > > this should probably be more explicit to keep in line with how things are 
> > > currently done. Maybe something like "instance.variable"
> > Right, naming completely new things here so worth the discussion.
> > To keep in line with the current convention, I don't see instance methods 
> > having an `instance.` prefix. It's just `method` as opposed to 
> > `type.method`.
> > Also global variable is just `var`, if we really need to add the 
> > `instance.` prefix (which I still don't think makes much sense for the 
> > above reason), wouldn't it be `instance.var`?
> Actually, now that I think about it, we should just make them properties. 
> There is already a precedent of doing that for struct fields, and ivars are 
> pretty much the equivalent of a struct field for an interface.
For my understanding, struct fields got named "property" because they behave 
similar as properties, and there are no other possible member kinds in a struct 
to overload that concept. While in an Objective-C container, there are actual 
literal Objective-C "properties." How do we distinguish between ivars and 
properties if they're the same kind?

We might also reconsider the kind for struct fields as "instance property" 
isn't really accurate and largely Objective-C-ish.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122446

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


[PATCH] D122573: [TBAA] Emit distinct TBAA tags for pointers with different depths,types.

2022-03-29 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> Maybe we should try improve our tooling to detect violations in parallel to 
> the effort in this patch? I am planning on trying to resurrect the 
> type-sanitizer patches. I've also been playing around with a simple tool 
> that's inserting assertions to check 2 pointers are not equal if TBAA claims 
> they are no alias. The latter seems to surface a few violations in code in 
> llvm-test-suite and also in tablegen. That is even without the current patch 
> applied.

I'd be more comfortable if we had tooling, yes.  The current state of "only a 
compiler expert can figure out if your code has undefined behavior" is the most 
scary part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122573

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


  1   2   >