[PATCH] D109544: [OpenMP] Add flag for setting debug in the offloading device

2021-09-13 Thread Michał Górny via Phabricator via cfe-commits
mgorny added a comment.

This change causes `clang-ast-dump` to segfault for me while building clang:

  [399/399] ASTNodeAPI.json
  FAILED: tools/clang/lib/Tooling/ASTNodeAPI.json 
/home/mgorny/git/llvm-project/build/tools/clang/lib/Tooling/ASTNodeAPI.json 
  cd /home/mgorny/git/llvm-project/build/tools/clang/lib/Tooling && 
/home/mgorny/git/llvm-project/build/bin/clang-ast-dump --skip-processing=0 -I 
/home/mgorny/git/llvm-project/build/lib/clang/14.0.0/include -I 
/home/mgorny/git/llvm-project/llvm/../clang/include -I 
/home/mgorny/git/llvm-project/build/tools/clang/include -I 
/home/mgorny/git/llvm-project/build/include -I 
/home/mgorny/git/llvm-project/llvm/include -I 
/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/include/g++-v11 -I 
/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/include/g++-v11/x86_64-pc-linux-gnu -I 
/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/include/g++-v11/backward -I 
/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/include -I 
/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/include-fixed -I /usr/include 
--json-output-path 
/home/mgorny/git/llvm-project/build/tools/clang/lib/Tooling/ASTNodeAPI.json
  ninja: build stopped: subcommand failed.



  Reading symbols from /home/mgorny/git/llvm-project/build/bin/clang-ast-dump...
  [New LWP 230743]
  [Thread debugging using libthread_db enabled]
  Using host libthread_db library "/lib64/libthread_db.so.1".
  Core was generated by `/home/mgorny/git/llvm-project/build/bin/clang-ast-dump 
--skip-processing=0 -I /'.
  Program terminated with signal SIGSEGV, Segmentation fault.
  #0  0x7ff68a87 in llvm::BumpPtrAllocatorImpl::~BumpPtrAllocatorImpl() ()
 from 
/home/mgorny/git/llvm-project/build/bin/../lib/../lib/libclangLex.so.14git
  (gdb) bt
  #0  0x7ff68a87 in llvm::BumpPtrAllocatorImpl::~BumpPtrAllocatorImpl() ()
 from 
/home/mgorny/git/llvm-project/build/bin/../lib/../lib/libclangLex.so.14git
  #1  0x7ffa1404 in clang::HeaderSearch::~HeaderSearch() ()
 from 
/home/mgorny/git/llvm-project/build/bin/../lib/../lib/libclangLex.so.14git
  #2  0x7ffa1859 in clang::Preprocessor::~Preprocessor() ()
 from 
/home/mgorny/git/llvm-project/build/bin/../lib/../lib/libclangLex.so.14git
  #3  0x7ff224d3175c in 
std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() ()
 from 
/home/mgorny/git/llvm-project/build/bin/../lib/libclangFrontend.so.14git
  #4  0x7ff224d4c801 in clang::CompilerInstance::~CompilerInstance() ()
 from 
/home/mgorny/git/llvm-project/build/bin/../lib/libclangFrontend.so.14git
  #5  0x556c688f8a95 in main ()

I'm going to try RelWithDebInfo build but I'm afraid I don't have sufficient 
free space for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109544

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


[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

2021-09-13 Thread Ronan Keryell via Phabricator via cfe-commits
keryell added inline comments.



Comment at: clang/lib/Basic/Targets/SPIR.h:59
+// translation). This mapping is enabled when the language mode is HIP.
+1, // cuda_device
+// cuda_constant pointer can be casted to default/"flat" pointer, but in

Anastasia wrote:
> bader wrote:
> > Anastasia wrote:
> > > I am slightly confused as in the LLVM project those address spaces are 
> > > for SPIR not SPIR-V though. It is however used outside of LLVM project by 
> > > some tools like SPIRV-LLVM Translator as a path to SPIR-V, but it has 
> > > only been done as a workaround since we had no SPIR-V support in the LLVM 
> > > project yet. And if we are adding it let's do it clean to avoid/resolve 
> > > any confusion.
> > > 
> > > I think we need to keep both because some vendors do target/use SPIR but 
> > > not SPIR-V.
> > > 
> > > So if you are interested in SPIR-V and not SPIR you should probably add a 
> > > new target that will make things cleaner.
> > > I think we need to keep both because some vendors do target/use SPIR but 
> > > not SPIR-V.
> > 
> > @Anastasia, could you elaborate more on the difference between SPIR and 
> > SPIR-V?
> > I would like to understand what these terms mean in the context of LLVM 
> > project.
> Their conceptual differences are just that they are two different 
> intermediate formats.
> 
> The important thing to highlight is that it is not impossible that some 
> vendors use SPIR (without using SPIR-V) even despite the fact it has been 
> discontinued by Khronos. 
> 
> Nobody has deprecated or discontinued SPIR in the LLVM project yet.
> Their conceptual differences are just that they are two different 
> intermediate formats.
> 
> The important thing to highlight is that it is not impossible that some 
> vendors use SPIR (without using SPIR-V) even despite the fact it has been 
> discontinued by Khronos. 
> 
> Nobody has deprecated or discontinued SPIR in the LLVM project yet.

All the official Xilinx OpenCL stack is based on legacy SPIR (encoded in LLVM 
6.x IR but this is another story) and I suspect this is the case for other 
companies.
So, do not deprecate or discontinue, please. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108621

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


[PATCH] D109609: [C++4OpenCL] Add support for multiple address spaced destructors

2021-09-13 Thread Ronan Keryell via Phabricator via cfe-commits
keryell added a comment.

While it might be possible to extend arbitrarily C++, I have the feeling that 
having just 1 destructor and have a different code path-code according the 
address space would not be enough.
It is possible to write:

  ~MyDestructor() {
if constexpr (SomeAPIReturningAddressSpace() == ...::Global) {
  /* Global address space code */
}
else if constexpr (...) {
}
  }

It is possible to build higher-level dispatching constructs from this in OpenCL 
C++ library, for example à la `std::visit` from `std::variant`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109609

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


[PATCH] D108370: [clang-tidy] Fix wrong FixIt about union in cppcoreguidelines-pro-type-member-init

2021-09-13 Thread gehry via Phabricator via cfe-commits
Sockke added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp:534
+
+union U2 {
+  U2() {}

aaron.ballman wrote:
> A related interesting test would be:
> ```
> union U3 {
>   U3() {}
> 
>   struct {
> int B;
>   } b;
>   int A;
> };
> ```
> I would expect `A` or `b` to need initialization for the union, and `B` to 
> need initialization for the struct.
> A related interesting test would be:
> ```
> union U3 {
>   U3() {}
> 
>   struct {
> int B;
>   } b;
>   int A;
> };
> ```
> I would expect `A` or `b` to need initialization for the union, and `B` to 
> need initialization for the struct.

I apologize for the delay in responding. It seems to me that `struct {}b` has 
no user-defined constructor, so all members are initialized to their defaults, 
it is unnecessary to initialize `B` explicitly. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108370

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


[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-13 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

In D69764#2996365 , @steveire wrote:

> FYI - there's nothing "anti-inclusive" about East/West.

Thank you for your comment.

Personally I don't think that the question of not using East/West is whether 
these terms are inclusive or not.
It's more about being straight to the point, not multiplying multiple options 
(clang-format has lots of them already).
Moreover, Left/Right is IMO universally understood (we have left-to-right or 
right-to-left writing, left/right margin and so on, these words seem to be more 
popular in typography-related contexts).
East/West is on the other hand not that universal and often misunderstood. It 
also adds an unnecessary cognitive charge, because one needs to translate it to 
left/right in their head anyway.
Tangentially, the left part of my screen is not necessarily on the west...

My 2 cents.


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

https://reviews.llvm.org/D69764

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


[PATCH] D109659: Fix scan-build-py executable lookup path

2021-09-13 Thread serge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc84755a046bb: Fix scan-build-py executable lookup path 
(authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109659

Files:
  clang/tools/scan-build-py/lib/libscanbuild/analyze.py


Index: clang/tools/scan-build-py/lib/libscanbuild/analyze.py
===
--- clang/tools/scan-build-py/lib/libscanbuild/analyze.py
+++ clang/tools/scan-build-py/lib/libscanbuild/analyze.py
@@ -39,8 +39,10 @@
 
 __all__ = ['scan_build', 'analyze_build', 'analyze_compiler_wrapper']
 
-COMPILER_WRAPPER_CC = 'analyze-cc'
-COMPILER_WRAPPER_CXX = 'analyze-c++'
+scanbuild_dir = os.path.dirname(__import__('sys').argv[0])
+
+COMPILER_WRAPPER_CC = os.path.join(scanbuild_dir, '..', 'libexec', 
'analyze-cc')
+COMPILER_WRAPPER_CXX = os.path.join(scanbuild_dir, '..', 'libexec', 
'analyze-c++')
 
 CTU_EXTDEF_MAP_FILENAME = 'externalDefMap.txt'
 CTU_TEMP_DEFMAP_FOLDER = 'tmpExternalDefMaps'


Index: clang/tools/scan-build-py/lib/libscanbuild/analyze.py
===
--- clang/tools/scan-build-py/lib/libscanbuild/analyze.py
+++ clang/tools/scan-build-py/lib/libscanbuild/analyze.py
@@ -39,8 +39,10 @@
 
 __all__ = ['scan_build', 'analyze_build', 'analyze_compiler_wrapper']
 
-COMPILER_WRAPPER_CC = 'analyze-cc'
-COMPILER_WRAPPER_CXX = 'analyze-c++'
+scanbuild_dir = os.path.dirname(__import__('sys').argv[0])
+
+COMPILER_WRAPPER_CC = os.path.join(scanbuild_dir, '..', 'libexec', 'analyze-cc')
+COMPILER_WRAPPER_CXX = os.path.join(scanbuild_dir, '..', 'libexec', 'analyze-c++')
 
 CTU_EXTDEF_MAP_FILENAME = 'externalDefMap.txt'
 CTU_TEMP_DEFMAP_FOLDER = 'tmpExternalDefMaps'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] c84755a - Fix scan-build-py executable lookup path

2021-09-13 Thread via cfe-commits

Author: serge-sans-paille
Date: 2021-09-13T11:01:47+02:00
New Revision: c84755a046bbdcd0564693e30b2508034b06002b

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

LOG: Fix scan-build-py executable lookup path

Once installed, scan-build-py doesn't know anything about its auxiliary
executable and can't find them.
Use relative path wrt. scan-build-py script.

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

Added: 


Modified: 
clang/tools/scan-build-py/lib/libscanbuild/analyze.py

Removed: 




diff  --git a/clang/tools/scan-build-py/lib/libscanbuild/analyze.py 
b/clang/tools/scan-build-py/lib/libscanbuild/analyze.py
index 9a249a8e15cb9..d83ff2aefab98 100644
--- a/clang/tools/scan-build-py/lib/libscanbuild/analyze.py
+++ b/clang/tools/scan-build-py/lib/libscanbuild/analyze.py
@@ -39,8 +39,10 @@
 
 __all__ = ['scan_build', 'analyze_build', 'analyze_compiler_wrapper']
 
-COMPILER_WRAPPER_CC = 'analyze-cc'
-COMPILER_WRAPPER_CXX = 'analyze-c++'
+scanbuild_dir = os.path.dirname(__import__('sys').argv[0])
+
+COMPILER_WRAPPER_CC = os.path.join(scanbuild_dir, '..', 'libexec', 
'analyze-cc')
+COMPILER_WRAPPER_CXX = os.path.join(scanbuild_dir, '..', 'libexec', 
'analyze-c++')
 
 CTU_EXTDEF_MAP_FILENAME = 'externalDefMap.txt'
 CTU_TEMP_DEFMAP_FOLDER = 'tmpExternalDefMaps'



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


[PATCH] D109544: [OpenMP] Add flag for setting debug in the offloading device

2021-09-13 Thread Michał Górny via Phabricator via cfe-commits
mgorny added a comment.

I'm sorry, it seems to have been caused by ccache. After clearing the cache, I 
can't reproduce anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109544

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


[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

2021-09-13 Thread Alexey Bader via Phabricator via cfe-commits
bader added inline comments.



Comment at: clang/lib/Basic/Targets/SPIR.h:59
+// translation). This mapping is enabled when the language mode is HIP.
+1, // cuda_device
+// cuda_constant pointer can be casted to default/"flat" pointer, but in

keryell wrote:
> Anastasia wrote:
> > bader wrote:
> > > Anastasia wrote:
> > > > I am slightly confused as in the LLVM project those address spaces are 
> > > > for SPIR not SPIR-V though. It is however used outside of LLVM project 
> > > > by some tools like SPIRV-LLVM Translator as a path to SPIR-V, but it 
> > > > has only been done as a workaround since we had no SPIR-V support in 
> > > > the LLVM project yet. And if we are adding it let's do it clean to 
> > > > avoid/resolve any confusion.
> > > > 
> > > > I think we need to keep both because some vendors do target/use SPIR 
> > > > but not SPIR-V.
> > > > 
> > > > So if you are interested in SPIR-V and not SPIR you should probably add 
> > > > a new target that will make things cleaner.
> > > > I think we need to keep both because some vendors do target/use SPIR 
> > > > but not SPIR-V.
> > > 
> > > @Anastasia, could you elaborate more on the difference between SPIR and 
> > > SPIR-V?
> > > I would like to understand what these terms mean in the context of LLVM 
> > > project.
> > Their conceptual differences are just that they are two different 
> > intermediate formats.
> > 
> > The important thing to highlight is that it is not impossible that some 
> > vendors use SPIR (without using SPIR-V) even despite the fact it has been 
> > discontinued by Khronos. 
> > 
> > Nobody has deprecated or discontinued SPIR in the LLVM project yet.
> > Their conceptual differences are just that they are two different 
> > intermediate formats.
> > 
> > The important thing to highlight is that it is not impossible that some 
> > vendors use SPIR (without using SPIR-V) even despite the fact it has been 
> > discontinued by Khronos. 
> > 
> > Nobody has deprecated or discontinued SPIR in the LLVM project yet.
> 
> All the official Xilinx OpenCL stack is based on legacy SPIR (encoded in LLVM 
> 6.x IR but this is another story) and I suspect this is the case for other 
> companies.
> So, do not deprecate or discontinue, please. :-)
> The important thing to highlight is that it is not impossible that some 
> vendors use SPIR (without using SPIR-V) even despite the fact it has been 
> discontinued by Khronos.
> Nobody has deprecated or discontinued SPIR in the LLVM project yet.

Strictly speaking `SPIR` is not defined as an intermediate language. Khronos 
defines `SPIR-1.2` and `SPIR-2.0` formats which are based on LLVM 3.2 and LLVM 
3.4 version (https://www.khronos.org/spir/). There is no definition of SPIR 
format based on current version of LLVM IR. Another note is that metadata and 
intrinsics emitted for OpenCL with clang-14 doesn't follow neither `SPIR-1.2` 
nor `SPIR-2.0`.

I always think of LLVM IR as leaving thing that is subject to change by LLVM 
community, so tools working with LLVM IR must adjust to the particular version 
(e.g. release version like LLVM 13 or ToT). We apply this logic to 
SPIRV-LLVM-Translator tool and update it according to LLVM format changes (e.g. 
kernel argument information defined in Khronos spec must be named metadata 
whereas clang emits function metadata).

> I am slightly confused as in the LLVM project those address spaces are for 
> SPIR not SPIR-V though.
[skip]
> Their conceptual differences are just that they are two different 
> intermediate formats.

If this is the only difference, I don't think it a good idea to create another 
LLVM target to separate SPIR and SPIR-V. From my point of view it creates logic 
ambiguity and code duplication with no additional value. @Anastasia, what 
problems do you see if we continue treating LLVM IR with spir* target triple as 
LLVM IR representation of SPIR-V format?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108621

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


[clang] 3731de6 - [X86] Adjust Keylocker handle mem size

2021-09-13 Thread Xiang1 Zhang via cfe-commits

Author: Xiang1 Zhang
Date: 2021-09-13T17:59:33+08:00
New Revision: 3731de6b7f2d42d40151f9574636bc4d5ccfa5e3

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

LOG: [X86] Adjust Keylocker handle mem size

Reviewed By: Topper Craig

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/keylockerintrin.h
clang/test/CodeGen/X86/keylocker.c
llvm/test/CodeGen/X86/keylocker-intrinsics.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 727fc19f1a23f..d485e8e767692 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -14905,7 +14905,7 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
 
 Value *Call = Builder.CreateCall(CGM.getIntrinsic(IID), {Ops[0], Ops[1]});
 
-for (int i = 0; i < 6; ++i) {
+for (int i = 0; i < 3; ++i) {
   Value *Extract = Builder.CreateExtractValue(Call, i + 1);
   Value *Ptr = Builder.CreateConstGEP1_32(Int8Ty, Ops[2], i * 16);
   Ptr = Builder.CreateBitCast(
@@ -14921,7 +14921,7 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
 Value *Call =
 Builder.CreateCall(CGM.getIntrinsic(IID), {Ops[0], Ops[1], Ops[2]});
 
-for (int i = 0; i < 7; ++i) {
+for (int i = 0; i < 4; ++i) {
   Value *Extract = Builder.CreateExtractValue(Call, i + 1);
   Value *Ptr = Builder.CreateConstGEP1_32(Int8Ty, Ops[3], i * 16);
   Ptr = Builder.CreateBitCast(

diff  --git a/clang/lib/Headers/keylockerintrin.h 
b/clang/lib/Headers/keylockerintrin.h
index 68b0a5689618a..ad9428e6c8b57 100644
--- a/clang/lib/Headers/keylockerintrin.h
+++ b/clang/lib/Headers/keylockerintrin.h
@@ -99,7 +99,7 @@ _mm_loadiwkey (unsigned int __ctl, __m128i __intkey,
 }
 
 /// Wrap a 128-bit AES key from __key into a key handle and output in
-/// ((__m128i*)__h) to ((__m128i*)__h) + 5  and a 32-bit value as return.
+/// ((__m128i*)__h) to ((__m128i*)__h) + 2  and a 32-bit value as return.
 /// The explicit source operand __htype specifies handle restrictions.
 ///
 /// \headerfile 
@@ -120,9 +120,6 @@ _mm_loadiwkey (unsigned int __ctl, __m128i __intkey,
 /// MEM[__h+127:__h] := Handle[127:0]   // AAD
 /// MEM[__h+255:__h+128] := Handle[255:128] // Integrity Tag
 /// MEM[__h+383:__h+256] := Handle[383:256] // CipherText
-/// MEM[__h+511:__h+384] := 0 // Reserved for future usage
-/// MEM[__h+639:__h+512] := 0 // Reserved for future usage
-/// MEM[__h+767:__h+640] := 0 // Reserved for future usage
 /// OF := 0
 /// SF := 0
 /// ZF := 0
@@ -136,7 +133,7 @@ _mm_encodekey128_u32(unsigned int __htype, __m128i __key, 
void *__h) {
 }
 
 /// Wrap a 256-bit AES key from __key_hi:__key_lo into a key handle, then
-/// output handle in ((__m128i*)__h) to ((__m128i*)__h) + 6 and
+/// output handle in ((__m128i*)__h) to ((__m128i*)__h) + 3 and
 /// a 32-bit value as return.
 /// The explicit source operand __htype specifies handle restrictions.
 ///
@@ -160,9 +157,6 @@ _mm_encodekey128_u32(unsigned int __htype, __m128i __key, 
void *__h) {
 /// MEM[__h+255:__h+128] := Handle[255:128] // Tag
 /// MEM[__h+383:__h+256] := Handle[383:256] // CipherText[127:0]
 /// MEM[__h+511:__h+384] := Handle[511:384] // CipherText[255:128]
-/// MEM[__h+639:__h+512] := 0 // Reserved for future usage
-/// MEM[__h+767:__h+640] := 0 // Reserved for future usage
-/// MEM[__h+895:__h+768] := 0 Integrity// Reserved for future usage
 /// OF := 0
 /// SF := 0
 /// ZF := 0

diff  --git a/clang/test/CodeGen/X86/keylocker.c 
b/clang/test/CodeGen/X86/keylocker.c
index ded6e57bfb8b6..a87281c5bd3fc 100644
--- a/clang/test/CodeGen/X86/keylocker.c
+++ b/clang/test/CodeGen/X86/keylocker.c
@@ -98,20 +98,8 @@ void test_loadiwkey(unsigned int ctl, __m128i intkey, 
__m128i enkey_lo, __m128i
 // CHECK64-NEXT:[[TMP13:%.*]] = getelementptr i8, i8* [[TMP5]], i32 32
 // CHECK64-NEXT:[[TMP14:%.*]] = bitcast i8* [[TMP13]] to <2 x i64>*
 // CHECK64-NEXT:store <2 x i64> [[TMP12]], <2 x i64>* [[TMP14]], align 1
-// CHECK64-NEXT:[[TMP15:%.*]] = extractvalue { i32, <2 x i64>, <2 x i64>, 
<2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } [[TMP6]], 4
-// CHECK64-NEXT:[[TMP16:%.*]] = getelementptr i8, i8* [[TMP5]], i32 48
-// CHECK64-NEXT:[[TMP17:%.*]] = bitcast i8* [[TMP16]] to <2 x i64>*
-// CHECK64-NEXT:store <2 x i64> [[TMP15]], <2 x i64>* [[TMP17]], align 1
-// CHECK64-NEXT:[[TMP18:%.*]] = extractvalue { i32, <2 x i64>, <2 x i64>, 
<2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } [[TMP6]], 5
-// CHECK64-NEXT:[[TMP19:%.*]] = getelementptr i8, i8* [[TMP5]], i32 64
-// CHECK64-NEXT:[[TMP20:%.*]] = bitcast i8* [[TMP19]] to <2 x i64>*
-// CHECK64-NEXT:store <2 x i64> [[TMP18]], <2 x i64>* [[TMP2

[clang] bdce8d4 - Revert "[X86] Adjust Keylocker handle mem size"

2021-09-13 Thread Xiang1 Zhang via cfe-commits

Author: Xiang1 Zhang
Date: 2021-09-13T18:00:46+08:00
New Revision: bdce8d40c6da56f1c95a8d7bfeac12b1ffce79cf

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

LOG: Revert "[X86] Adjust Keylocker handle mem size"

This reverts commit 3731de6b7f2d42d40151f9574636bc4d5ccfa5e3.

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/keylockerintrin.h
clang/test/CodeGen/X86/keylocker.c
llvm/test/CodeGen/X86/keylocker-intrinsics.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index d485e8e767692..727fc19f1a23f 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -14905,7 +14905,7 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
 
 Value *Call = Builder.CreateCall(CGM.getIntrinsic(IID), {Ops[0], Ops[1]});
 
-for (int i = 0; i < 3; ++i) {
+for (int i = 0; i < 6; ++i) {
   Value *Extract = Builder.CreateExtractValue(Call, i + 1);
   Value *Ptr = Builder.CreateConstGEP1_32(Int8Ty, Ops[2], i * 16);
   Ptr = Builder.CreateBitCast(
@@ -14921,7 +14921,7 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
 Value *Call =
 Builder.CreateCall(CGM.getIntrinsic(IID), {Ops[0], Ops[1], Ops[2]});
 
-for (int i = 0; i < 4; ++i) {
+for (int i = 0; i < 7; ++i) {
   Value *Extract = Builder.CreateExtractValue(Call, i + 1);
   Value *Ptr = Builder.CreateConstGEP1_32(Int8Ty, Ops[3], i * 16);
   Ptr = Builder.CreateBitCast(

diff  --git a/clang/lib/Headers/keylockerintrin.h 
b/clang/lib/Headers/keylockerintrin.h
index ad9428e6c8b57..68b0a5689618a 100644
--- a/clang/lib/Headers/keylockerintrin.h
+++ b/clang/lib/Headers/keylockerintrin.h
@@ -99,7 +99,7 @@ _mm_loadiwkey (unsigned int __ctl, __m128i __intkey,
 }
 
 /// Wrap a 128-bit AES key from __key into a key handle and output in
-/// ((__m128i*)__h) to ((__m128i*)__h) + 2  and a 32-bit value as return.
+/// ((__m128i*)__h) to ((__m128i*)__h) + 5  and a 32-bit value as return.
 /// The explicit source operand __htype specifies handle restrictions.
 ///
 /// \headerfile 
@@ -120,6 +120,9 @@ _mm_loadiwkey (unsigned int __ctl, __m128i __intkey,
 /// MEM[__h+127:__h] := Handle[127:0]   // AAD
 /// MEM[__h+255:__h+128] := Handle[255:128] // Integrity Tag
 /// MEM[__h+383:__h+256] := Handle[383:256] // CipherText
+/// MEM[__h+511:__h+384] := 0 // Reserved for future usage
+/// MEM[__h+639:__h+512] := 0 // Reserved for future usage
+/// MEM[__h+767:__h+640] := 0 // Reserved for future usage
 /// OF := 0
 /// SF := 0
 /// ZF := 0
@@ -133,7 +136,7 @@ _mm_encodekey128_u32(unsigned int __htype, __m128i __key, 
void *__h) {
 }
 
 /// Wrap a 256-bit AES key from __key_hi:__key_lo into a key handle, then
-/// output handle in ((__m128i*)__h) to ((__m128i*)__h) + 3 and
+/// output handle in ((__m128i*)__h) to ((__m128i*)__h) + 6 and
 /// a 32-bit value as return.
 /// The explicit source operand __htype specifies handle restrictions.
 ///
@@ -157,6 +160,9 @@ _mm_encodekey128_u32(unsigned int __htype, __m128i __key, 
void *__h) {
 /// MEM[__h+255:__h+128] := Handle[255:128] // Tag
 /// MEM[__h+383:__h+256] := Handle[383:256] // CipherText[127:0]
 /// MEM[__h+511:__h+384] := Handle[511:384] // CipherText[255:128]
+/// MEM[__h+639:__h+512] := 0 // Reserved for future usage
+/// MEM[__h+767:__h+640] := 0 // Reserved for future usage
+/// MEM[__h+895:__h+768] := 0 Integrity// Reserved for future usage
 /// OF := 0
 /// SF := 0
 /// ZF := 0

diff  --git a/clang/test/CodeGen/X86/keylocker.c 
b/clang/test/CodeGen/X86/keylocker.c
index a87281c5bd3fc..ded6e57bfb8b6 100644
--- a/clang/test/CodeGen/X86/keylocker.c
+++ b/clang/test/CodeGen/X86/keylocker.c
@@ -98,8 +98,20 @@ void test_loadiwkey(unsigned int ctl, __m128i intkey, 
__m128i enkey_lo, __m128i
 // CHECK64-NEXT:[[TMP13:%.*]] = getelementptr i8, i8* [[TMP5]], i32 32
 // CHECK64-NEXT:[[TMP14:%.*]] = bitcast i8* [[TMP13]] to <2 x i64>*
 // CHECK64-NEXT:store <2 x i64> [[TMP12]], <2 x i64>* [[TMP14]], align 1
-// CHECK64-NEXT:[[TMP15:%.*]] = extractvalue { i32, <2 x i64>, <2 x i64>, 
<2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } [[TMP6]], 0
-// CHECK64-NEXT:ret i32 [[TMP15]]
+// CHECK64-NEXT:[[TMP15:%.*]] = extractvalue { i32, <2 x i64>, <2 x i64>, 
<2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } [[TMP6]], 4
+// CHECK64-NEXT:[[TMP16:%.*]] = getelementptr i8, i8* [[TMP5]], i32 48
+// CHECK64-NEXT:[[TMP17:%.*]] = bitcast i8* [[TMP16]] to <2 x i64>*
+// CHECK64-NEXT:store <2 x i64> [[TMP15]], <2 x i64>* [[TMP17]], align 1
+// CHECK64-NEXT:[[TMP18:%.*]] = extractvalue { i32, <2 x i64>, <2 x i64>, 
<2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } [[TMP6]], 5
+// CHECK64-NEXT:[[TMP19:%.*]] = getelementptr

[clang] c81d6ab - [X86] Adjust Keylocker handle mem size

2021-09-13 Thread Xiang1 Zhang via cfe-commits

Author: Xiang1 Zhang
Date: 2021-09-13T18:03:27+08:00
New Revision: c81d6ab8758224ab950716d9533df79c5b5fb706

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

LOG: [X86] Adjust Keylocker handle mem size

Reviewed By: Topper Craig

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/keylockerintrin.h
clang/test/CodeGen/X86/keylocker.c
llvm/test/CodeGen/X86/keylocker-intrinsics.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 727fc19f1a23f..d485e8e767692 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -14905,7 +14905,7 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
 
 Value *Call = Builder.CreateCall(CGM.getIntrinsic(IID), {Ops[0], Ops[1]});
 
-for (int i = 0; i < 6; ++i) {
+for (int i = 0; i < 3; ++i) {
   Value *Extract = Builder.CreateExtractValue(Call, i + 1);
   Value *Ptr = Builder.CreateConstGEP1_32(Int8Ty, Ops[2], i * 16);
   Ptr = Builder.CreateBitCast(
@@ -14921,7 +14921,7 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
 Value *Call =
 Builder.CreateCall(CGM.getIntrinsic(IID), {Ops[0], Ops[1], Ops[2]});
 
-for (int i = 0; i < 7; ++i) {
+for (int i = 0; i < 4; ++i) {
   Value *Extract = Builder.CreateExtractValue(Call, i + 1);
   Value *Ptr = Builder.CreateConstGEP1_32(Int8Ty, Ops[3], i * 16);
   Ptr = Builder.CreateBitCast(

diff  --git a/clang/lib/Headers/keylockerintrin.h 
b/clang/lib/Headers/keylockerintrin.h
index 68b0a5689618a..ad9428e6c8b57 100644
--- a/clang/lib/Headers/keylockerintrin.h
+++ b/clang/lib/Headers/keylockerintrin.h
@@ -99,7 +99,7 @@ _mm_loadiwkey (unsigned int __ctl, __m128i __intkey,
 }
 
 /// Wrap a 128-bit AES key from __key into a key handle and output in
-/// ((__m128i*)__h) to ((__m128i*)__h) + 5  and a 32-bit value as return.
+/// ((__m128i*)__h) to ((__m128i*)__h) + 2  and a 32-bit value as return.
 /// The explicit source operand __htype specifies handle restrictions.
 ///
 /// \headerfile 
@@ -120,9 +120,6 @@ _mm_loadiwkey (unsigned int __ctl, __m128i __intkey,
 /// MEM[__h+127:__h] := Handle[127:0]   // AAD
 /// MEM[__h+255:__h+128] := Handle[255:128] // Integrity Tag
 /// MEM[__h+383:__h+256] := Handle[383:256] // CipherText
-/// MEM[__h+511:__h+384] := 0 // Reserved for future usage
-/// MEM[__h+639:__h+512] := 0 // Reserved for future usage
-/// MEM[__h+767:__h+640] := 0 // Reserved for future usage
 /// OF := 0
 /// SF := 0
 /// ZF := 0
@@ -136,7 +133,7 @@ _mm_encodekey128_u32(unsigned int __htype, __m128i __key, 
void *__h) {
 }
 
 /// Wrap a 256-bit AES key from __key_hi:__key_lo into a key handle, then
-/// output handle in ((__m128i*)__h) to ((__m128i*)__h) + 6 and
+/// output handle in ((__m128i*)__h) to ((__m128i*)__h) + 3 and
 /// a 32-bit value as return.
 /// The explicit source operand __htype specifies handle restrictions.
 ///
@@ -160,9 +157,6 @@ _mm_encodekey128_u32(unsigned int __htype, __m128i __key, 
void *__h) {
 /// MEM[__h+255:__h+128] := Handle[255:128] // Tag
 /// MEM[__h+383:__h+256] := Handle[383:256] // CipherText[127:0]
 /// MEM[__h+511:__h+384] := Handle[511:384] // CipherText[255:128]
-/// MEM[__h+639:__h+512] := 0 // Reserved for future usage
-/// MEM[__h+767:__h+640] := 0 // Reserved for future usage
-/// MEM[__h+895:__h+768] := 0 Integrity// Reserved for future usage
 /// OF := 0
 /// SF := 0
 /// ZF := 0

diff  --git a/clang/test/CodeGen/X86/keylocker.c 
b/clang/test/CodeGen/X86/keylocker.c
index ded6e57bfb8b6..a87281c5bd3fc 100644
--- a/clang/test/CodeGen/X86/keylocker.c
+++ b/clang/test/CodeGen/X86/keylocker.c
@@ -98,20 +98,8 @@ void test_loadiwkey(unsigned int ctl, __m128i intkey, 
__m128i enkey_lo, __m128i
 // CHECK64-NEXT:[[TMP13:%.*]] = getelementptr i8, i8* [[TMP5]], i32 32
 // CHECK64-NEXT:[[TMP14:%.*]] = bitcast i8* [[TMP13]] to <2 x i64>*
 // CHECK64-NEXT:store <2 x i64> [[TMP12]], <2 x i64>* [[TMP14]], align 1
-// CHECK64-NEXT:[[TMP15:%.*]] = extractvalue { i32, <2 x i64>, <2 x i64>, 
<2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } [[TMP6]], 4
-// CHECK64-NEXT:[[TMP16:%.*]] = getelementptr i8, i8* [[TMP5]], i32 48
-// CHECK64-NEXT:[[TMP17:%.*]] = bitcast i8* [[TMP16]] to <2 x i64>*
-// CHECK64-NEXT:store <2 x i64> [[TMP15]], <2 x i64>* [[TMP17]], align 1
-// CHECK64-NEXT:[[TMP18:%.*]] = extractvalue { i32, <2 x i64>, <2 x i64>, 
<2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } [[TMP6]], 5
-// CHECK64-NEXT:[[TMP19:%.*]] = getelementptr i8, i8* [[TMP5]], i32 64
-// CHECK64-NEXT:[[TMP20:%.*]] = bitcast i8* [[TMP19]] to <2 x i64>*
-// CHECK64-NEXT:store <2 x i64> [[TMP18]], <2 x i64>* [[TMP2

[PATCH] D109488: [X86] Adjust Keylocker store register num for encodekey128/256

2021-09-13 Thread Xiang Zhang 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 rGc81d6ab87582: [X86] Adjust Keylocker handle mem size 
(authored by xiangzhangllvm).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109488

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/keylockerintrin.h
  clang/test/CodeGen/X86/keylocker.c
  llvm/test/CodeGen/X86/keylocker-intrinsics.ll

Index: llvm/test/CodeGen/X86/keylocker-intrinsics.ll
===
--- llvm/test/CodeGen/X86/keylocker-intrinsics.ll
+++ llvm/test/CodeGen/X86/keylocker-intrinsics.ll
@@ -36,40 +36,24 @@
 define i32 @test_encodekey128_u32(i32 %htype, <2 x i64> %key, <2 x i64>* nocapture %h0, <2 x i64>* nocapture %h1, <2 x i64>* nocapture %h2, <2 x i64>* nocapture %h3, <2 x i64>* nocapture %h4, <2 x i64>* nocapture %h5) nounwind {
 ; X64-LABEL: test_encodekey128_u32:
 ; X64:   # %bb.0: # %entry
-; X64-NEXT:movq {{[0-9]+}}(%rsp), %r10
 ; X64-NEXT:encodekey128 %edi, %eax
 ; X64-NEXT:movaps %xmm0, (%rsi)
 ; X64-NEXT:movaps %xmm1, (%rdx)
 ; X64-NEXT:movaps %xmm2, (%rcx)
-; X64-NEXT:movaps %xmm4, (%r8)
-; X64-NEXT:movaps %xmm5, (%r9)
-; X64-NEXT:movaps %xmm6, (%r10)
 ; X64-NEXT:retq
 ;
 ; X32-LABEL: test_encodekey128_u32:
 ; X32:   # %bb.0: # %entry
-; X32-NEXT:pushl %ebp
-; X32-NEXT:pushl %ebx
-; X32-NEXT:pushl %edi
 ; X32-NEXT:pushl %esi
 ; X32-NEXT:movl {{[0-9]+}}(%esp), %ecx
 ; X32-NEXT:movl {{[0-9]+}}(%esp), %edx
 ; X32-NEXT:movl {{[0-9]+}}(%esp), %esi
-; X32-NEXT:movl {{[0-9]+}}(%esp), %edi
-; X32-NEXT:movl {{[0-9]+}}(%esp), %ebx
-; X32-NEXT:movl {{[0-9]+}}(%esp), %ebp
 ; X32-NEXT:movl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:encodekey128 %eax, %eax
-; X32-NEXT:vmovaps %xmm0, (%ebp)
-; X32-NEXT:vmovaps %xmm1, (%ebx)
-; X32-NEXT:vmovaps %xmm2, (%edi)
-; X32-NEXT:vmovaps %xmm4, (%esi)
-; X32-NEXT:vmovaps %xmm5, (%edx)
-; X32-NEXT:vmovaps %xmm6, (%ecx)
+; X32-NEXT:vmovaps %xmm0, (%esi)
+; X32-NEXT:vmovaps %xmm1, (%edx)
+; X32-NEXT:vmovaps %xmm2, (%ecx)
 ; X32-NEXT:popl %esi
-; X32-NEXT:popl %edi
-; X32-NEXT:popl %ebx
-; X32-NEXT:popl %ebp
 ; X32-NEXT:retl
 entry:
   %0 = tail call { i32, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } @llvm.x86.encodekey128(i32 %htype, <2 x i64> %key)
@@ -79,53 +63,36 @@
   store <2 x i64> %2, <2 x i64>* %h1, align 16
   %3 = extractvalue { i32, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } %0, 3
   store <2 x i64> %3, <2 x i64>* %h2, align 16
-  %4 = extractvalue { i32, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } %0, 4
-  store <2 x i64> %4, <2 x i64>* %h3, align 16
-  %5 = extractvalue { i32, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } %0, 5
-  store <2 x i64> %5, <2 x i64>* %h4, align 16
-  %6 = extractvalue { i32, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } %0, 6
-  store <2 x i64> %6, <2 x i64>* %h5, align 16
-  %7 = extractvalue { i32, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } %0, 0
-  ret i32 %7
+  %4 = extractvalue { i32, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64>, <2 x i64> } %0, 0
+  ret i32 %4
 }
 
 define i32 @test_encodekey256_u32(i32 %htype, <2 x i64> %key_lo, <2 x i64> %key_hi, <2 x i64>* nocapture %h0, <2 x i64>* nocapture %h1, <2 x i64>* nocapture %h2, <2 x i64>* nocapture %h3, <2 x i64>* nocapture %h4, <2 x i64>* nocapture %h5, <2 x  i64>* nocapture readnone %h6) nounwind {
 ; X64-LABEL: test_encodekey256_u32:
 ; X64:   # %bb.0: # %entry
-; X64-NEXT:movq {{[0-9]+}}(%rsp), %r10
 ; X64-NEXT:encodekey256 %edi, %eax
 ; X64-NEXT:movaps %xmm0, (%rsi)
 ; X64-NEXT:movaps %xmm1, (%rdx)
 ; X64-NEXT:movaps %xmm2, (%rcx)
 ; X64-NEXT:movaps %xmm3, (%r8)
-; X64-NEXT:movaps %xmm4, (%r9)
-; X64-NEXT:movaps %xmm5, (%r10)
 ; X64-NEXT:retq
 ;
 ; X32-LABEL: test_encodekey256_u32:
 ; X32:   # %bb.0: # %entry
-; X32-NEXT:pushl %ebp
-; X32-NEXT:pushl %ebx
 ; X32-NEXT:pushl %edi
 ; X32-NEXT:pushl %esi
 ; X32-NEXT:movl {{[0-9]+}}(%esp), %ecx
 ; X32-NEXT:movl {{[0-9]+}}(%esp), %edx
 ; X32-NEXT:movl {{[0-9]+}}(%esp), %esi
 ; X32-NEXT:movl {{[0-9]+}}(%esp), %edi
-; X32-NEXT:movl {{[0-9]+}}(%esp), %ebx
-; X32-NEXT:movl {{[0-9]+}}(%esp), %ebp
 ; X32-NEXT:movl {{[0-9]+}}(%esp), %eax
 ; X32-NEXT:encodekey256 %eax, %eax
-; X32-NEXT:vmovaps %xmm0, (%ebp)
-; X32-NEXT:vmovaps %xmm1, (%ebx)
-; X32-NEXT:vmovaps %xmm2, (%edi)
-; X32-NEXT:vmovaps %xmm3, (%esi)
-; X32-NEXT:vmovaps %xmm4, (%edx)
-; X32-NEXT:vmovaps %xmm5, (%ecx)
+; X32-NEXT:vmovaps %xmm0, (%edi)

[clang] d86a947 - [Sema] Add test for __builtin_fminf errors.

2021-09-13 Thread Florian Hahn via cfe-commits

Author: Florian Hahn
Date: 2021-09-13T11:12:06+01:00
New Revision: d86a947bb91cc67452057b4bc25c0c7734b5139c

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

LOG: [Sema] Add test for __builtin_fminf errors.

Added: 
clang/test/Sema/overloaded-math-builtins.c

Modified: 


Removed: 




diff  --git a/clang/test/Sema/overloaded-math-builtins.c 
b/clang/test/Sema/overloaded-math-builtins.c
new file mode 100644
index 0..e18b98ab8d554
--- /dev/null
+++ b/clang/test/Sema/overloaded-math-builtins.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 %s -pedantic -verify -triple=x86_64-apple-darwin9
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+
+float test_fminf(float f, int i, int *ptr, float4 v) {
+  float r1 = __builtin_fminf(f, ptr);
+  // expected-error@-1 {{passing 'int *' to parameter of incompatible type 
'float'}}
+  float r2 = __builtin_fminf(ptr, f);
+  // expected-error@-1 {{passing 'int *' to parameter of incompatible type 
'float'}}
+  float r3 = __builtin_fminf(v, f);
+  // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to 
parameter of incompatible type 'float'}}
+  float r4 = __builtin_fminf(f, v);
+  // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to 
parameter of incompatible type 'float'}}
+
+
+  int *r5 = __builtin_fminf(f, f);
+  // expected-error@-1 {{initializing 'int *' with an expression of 
incompatible type 'float'}}
+
+  int *r6 = __builtin_fminf(f, v);
+  // expected-error@-1 {{passing 'float4' (vector of 4 'float' values) to 
parameter of incompatible type 'float'}}
+}



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


[PATCH] D109681: [RISCV] add Half-precision test for vle/vse

2021-09-13 Thread Shao-Ce Sun via Phabricator via cfe-commits
achieveartificialintelligence created this revision.
achieveartificialintelligence added reviewers: craig.topper, HsiangKai.
Herald added subscribers: vkmr, frasercrmck, evandro, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, 
kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, asb.
achieveartificialintelligence requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109681

Files:
  clang/test/CodeGen/RISCV/rvv-intrinsics/vle.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vse.c

Index: clang/test/CodeGen/RISCV/rvv-intrinsics/vse.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics/vse.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics/vse.c
@@ -1195,3 +1195,63 @@
 void test_vse16_v_f16m8(_Float16 *base, vfloat16m8_t value, size_t vl) {
   return vse16_v_f16m8(base, value, vl);
 }
+
+// CHECK-RV64-LABEL: @test_vse16_v_f16mf4_m(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vse.mask.nxv1f16.i64( [[VALUE:%.*]], * [[TMP0]],  [[MASK:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vse16_v_f16mf4_m (vbool64_t mask, _Float16 *base, vfloat16mf4_t value, size_t vl) {
+  return vse16_v_f16mf4_m (mask, base, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vse16_v_f16mf2_m(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vse.mask.nxv2f16.i64( [[VALUE:%.*]], * [[TMP0]],  [[MASK:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vse16_v_f16mf2_m (vbool32_t mask, _Float16 *base, vfloat16mf2_t value, size_t vl) {
+  return vse16_v_f16mf2_m (mask, base, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vse16_v_f16m1_m(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vse.mask.nxv4f16.i64( [[VALUE:%.*]], * [[TMP0]],  [[MASK:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vse16_v_f16m1_m (vbool16_t mask, _Float16 *base, vfloat16m1_t value, size_t vl) {
+  return vse16_v_f16m1_m (mask, base, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vse16_v_f16m2_m(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vse.mask.nxv8f16.i64( [[VALUE:%.*]], * [[TMP0]],  [[MASK:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vse16_v_f16m2_m (vbool8_t mask, _Float16 *base, vfloat16m2_t value, size_t vl) {
+  return vse16_v_f16m2_m (mask, base, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vse16_v_f16m4_m(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vse.mask.nxv16f16.i64( [[VALUE:%.*]], * [[TMP0]],  [[MASK:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vse16_v_f16m4_m (vbool4_t mask, _Float16 *base, vfloat16m4_t value, size_t vl) {
+  return vse16_v_f16m4_m (mask, base, value, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vse16_v_f16m8_m(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:call void @llvm.riscv.vse.mask.nxv32f16.i64( [[VALUE:%.*]], * [[TMP0]],  [[MASK:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret void
+//
+void test_vse16_v_f16m8_m (vbool2_t mask, _Float16 *base, vfloat16m8_t value, size_t vl) {
+  return vse16_v_f16m8_m (mask, base, value, vl);
+}
Index: clang/test/CodeGen/RISCV/rvv-intrinsics/vle.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics/vle.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics/vle.c
@@ -1195,3 +1195,63 @@
 vfloat16m8_t test_vle16_v_f16m8(const _Float16 *base, size_t vl) {
   return vle16_v_f16m8(base, vl);
 }
+
+// CHECK-RV64-LABEL: @test_vle16_v_f16mf4_m(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:[[TMP1:%.*]] = call  @llvm.riscv.vle.mask.nxv1f16.i64( [[MASKEDOFF:%.*]], * [[TMP0]],  [[MASK:%.*]], i64 [[VL:%.*]])
+// CHECK-RV64-NEXT:ret  [[TMP1]]
+//
+vfloat16mf4_t test_vle16_v_f16mf4_m (vbool64_t mask, vfloat16mf4_t maskedoff, const _Float16 *base, size_t vl) {
+  return vle16_v_f16mf4_m (mask, maskedoff, base, vl);
+}
+
+// CHECK-RV64-LABEL: @test_vle16_v_f16mf2_m(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = bitcast half* [[BASE:%.*]] to *
+// CHECK-RV64-NEXT:[[TMP1:%.*]] = call  @llvm.riscv.vle.mask.nxv2f16.i64( [[MASKEDOFF:%.*]], * [[TMP0]],  [[MASK:%.*]], i64 [[VL:%.*]])
+// CHECK

[PATCH] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

2021-09-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

I like everything I see here so far! As soon as those debug functions are in, 
the patch should land!




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:236-240
+  /// At any stream operation that can cause (multiple type of) bugs, we can
+  /// determine the failure reason text by only knowing the bug type. The same
+  /// text is applicable at every operation that may cause that bug. This map
+  /// is used to lookup the message text in a note tag that is added at the
+  /// failing operation.

What this means might be obvious to us, because we are very familiar with this 
and similar checkers, but I'm sure its confusing for anybody else, even for 
seasoned analyzer developers. I'd prefer if comments like these were 
accompanied with examples. There are a few decent ones I think in some of the 
comments I left on this revision, feel free to copy one here.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:408-410
+  /// At a bug report the last operation in the path that has added this kind 
of
+  /// NoteTag is the one that caused the bug. It is enough to know the bug type
+  /// to determine the note tag text.

How about you explain this logic thoroughly in one comment (maybe above 
`BugMessages`), and replace these last 3 lines with "See the comments for 
`BugMessages`."?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106262

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


[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.

2021-09-13 Thread Jay Foad via Phabricator via cfe-commits
foad added a comment.

What is a "keep constructor"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109483

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


[clang] d353d1c - [OpenCL] Support cl_ext_float_atomics

2021-09-13 Thread Sven van Haastregt via cfe-commits

Author: Sven van Haastregt
Date: 2021-09-13T12:12:40+01:00
New Revision: d353d1c50112a1cb315eccdab18ce7bd1563cd06

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

LOG: [OpenCL] Support cl_ext_float_atomics

See https://github.com/KhronosGroup/OpenCL-Docs/pull/552 for initial
specification.

Patch by Haonan Yang.

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

Added: 


Modified: 
clang/lib/Headers/opencl-c-base.h
clang/lib/Headers/opencl-c.h
clang/lib/Sema/OpenCLBuiltins.td
clang/test/Headers/opencl-c-header.cl
clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Removed: 




diff  --git a/clang/lib/Headers/opencl-c-base.h 
b/clang/lib/Headers/opencl-c-base.h
index f3605c659d952..7cc356015d959 100644
--- a/clang/lib/Headers/opencl-c-base.h
+++ b/clang/lib/Headers/opencl-c-base.h
@@ -25,6 +25,25 @@
 #define cl_khr_integer_dot_product 1
 #define __opencl_c_integer_dot_product_input_4x8bit 1
 #define __opencl_c_integer_dot_product_input_4x8bit_packed 1
+#define cl_ext_float_atomics 1
+#ifdef cl_khr_fp16
+#define __opencl_c_ext_fp16_global_atomic_load_store 1
+#define __opencl_c_ext_fp16_local_atomic_load_store 1
+#define __opencl_c_ext_fp16_global_atomic_add 1
+#define __opencl_c_ext_fp16_local_atomic_add 1
+#define __opencl_c_ext_fp16_global_atomic_min_max 1
+#define __opencl_c_ext_fp16_local_atomic_min_max 1
+#endif
+#ifdef cl_khr_fp64
+#define __opencl_c_ext_fp64_global_atomic_add 1
+#define __opencl_c_ext_fp64_local_atomic_add 1
+#define __opencl_c_ext_fp64_global_atomic_min_max 1
+#define __opencl_c_ext_fp64_local_atomic_min_max 1
+#endif
+#define __opencl_c_ext_fp32_global_atomic_add 1
+#define __opencl_c_ext_fp32_local_atomic_add 1
+#define __opencl_c_ext_fp32_global_atomic_min_max 1
+#define __opencl_c_ext_fp32_local_atomic_min_max 1
 
 #endif // defined(__SPIR__)
 #endif // (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)

diff  --git a/clang/lib/Headers/opencl-c.h b/clang/lib/Headers/opencl-c.h
index bb3ca6aae20a2..e960228594801 100644
--- a/clang/lib/Headers/opencl-c.h
+++ b/clang/lib/Headers/opencl-c.h
@@ -13637,6 +13637,215 @@ uintptr_t __ovld atomic_fetch_sub_explicit(volatile 
__local atomic_uintptr_t *ob
 #endif //defined(cl_khr_int64_base_atomics) && 
defined(cl_khr_int64_extended_atomics)
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_3_0
 
+// The functionality added by cl_ext_float_atomics extension
+#if defined(cl_ext_float_atomics)
+
+#if defined(__opencl_c_ext_fp32_global_atomic_min_max)
+float __ovld atomic_fetch_min(volatile __global atomic_float *object,
+  float operand);
+float __ovld atomic_fetch_max(volatile __global atomic_float *object,
+  float operand);
+float __ovld atomic_fetch_min_explicit(volatile __global atomic_float *object,
+   float operand, memory_order order);
+float __ovld atomic_fetch_max_explicit(volatile __global atomic_float *object,
+   float operand, memory_order order);
+float __ovld atomic_fetch_min_explicit(volatile __global atomic_float *object,
+   float operand, memory_order order,
+   memory_scope scope);
+float __ovld atomic_fetch_max_explicit(volatile __global atomic_float *object,
+   float operand, memory_order order,
+   memory_scope scope);
+#endif // defined(__opencl_c_ext_fp32_global_atomic_min_max)
+
+#if defined(__opencl_c_ext_fp32_local_atomic_min_max)
+float __ovld atomic_fetch_min(volatile __local atomic_float *object,
+  float operand);
+float __ovld atomic_fetch_max(volatile __local atomic_float *object,
+  float operand);
+float __ovld atomic_fetch_min_explicit(volatile __local atomic_float *object,
+   float operand, memory_order order);
+float __ovld atomic_fetch_max_explicit(volatile __local atomic_float *object,
+   float operand, memory_order order);
+float __ovld atomic_fetch_min_explicit(volatile __local atomic_float *object,
+   float operand, memory_order order,
+   memory_scope scope);
+float __ovld atomic_fetch_max_explicit(volatile __local atomic_float *object,
+   float operand, memory_order order,
+   memory_scope scope);
+#endif // defined(__opencl_c_ext_fp32_local_atomic_min_max)
+
+#if defined(__opencl_c_ext_fp32_global_atomic_min_max) &&  
\
+defined(__opencl_c_ext_fp32_local_atomic_min_max)
+

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

2021-09-13 Thread Sven van Haastregt 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 rGd353d1c50112: [OpenCL] Support cl_ext_float_atomics 
(authored by svenvh).
Herald added a subscriber: ldrumm.

Changed prior to commit:
  https://reviews.llvm.org/D106343?vs=368256&id=372212#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106343

Files:
  clang/lib/Headers/opencl-c-base.h
  clang/lib/Headers/opencl-c.h
  clang/lib/Sema/OpenCLBuiltins.td
  clang/test/Headers/opencl-c-header.cl
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -122,6 +122,27 @@
 }
 #endif
 
+#if !defined(NO_HEADER) && !defined(NO_FP64) && __OPENCL_C_VERSION__ >= 200
+// Check added atomic_fetch_ functions by cl_ext_float_atomics
+// extension can be called
+void test_atomic_fetch_with_address_space(volatile __generic atomic_float *a_float,
+  volatile __generic atomic_double *a_double,
+  volatile __local atomic_float *a_float_local,
+  volatile __local atomic_double *a_double_local,
+  volatile __global atomic_float *a_float_global,
+  volatile __global atomic_double *a_double_global) {
+  float f1, resf1;
+  double d1, resd1;
+  resf1 = atomic_fetch_min(a_float, f1);
+  resf1 = atomic_fetch_max_explicit(a_float_local, f1, memory_order_seq_cst);
+  resf1 = atomic_fetch_add_explicit(a_float_global, f1, memory_order_seq_cst, memory_scope_work_group);
+
+  resd1 = atomic_fetch_min(a_double, d1);
+  resd1 = atomic_fetch_max_explicit(a_double_local, d1, memory_order_seq_cst);
+  resd1 = atomic_fetch_add_explicit(a_double_global, d1, memory_order_seq_cst, memory_scope_work_group);
+}
+#endif // !defined(NO_HEADER) && __OPENCL_C_VERSION__ >= 200
+
 // Test old atomic overloaded with generic address space in C++ for OpenCL.
 #if __OPENCL_C_VERSION__ >= 200
 void test_legacy_atomics_cpp(__generic volatile unsigned int *a) {
Index: clang/test/Headers/opencl-c-header.cl
===
--- clang/test/Headers/opencl-c-header.cl
+++ clang/test/Headers/opencl-c-header.cl
@@ -135,6 +135,51 @@
 #if __opencl_c_integer_dot_product_input_4x8bit_packed != 1
 #error "Incorrectly defined __opencl_c_integer_dot_product_input_4x8bit_packed"
 #endif
+#if cl_ext_float_atomics != 1
+#error "Incorrectly defined cl_ext_float_atomics"
+#endif
+#if __opencl_c_ext_fp16_global_atomic_load_store != 1
+#error "Incorrectly defined __opencl_c_ext_fp16_global_atomic_load_store"
+#endif
+#if __opencl_c_ext_fp16_local_atomic_load_store != 1
+#error "Incorrectly defined __opencl_c_ext_fp16_local_atomic_load_store"
+#endif
+#if __opencl_c_ext_fp16_global_atomic_add != 1
+#error "Incorrectly defined __opencl_c_ext_fp16_global_atomic_add"
+#endif
+#if __opencl_c_ext_fp32_global_atomic_add != 1
+#error "Incorrectly defined __opencl_c_ext_fp32_global_atomic_add"
+#endif
+#if __opencl_c_ext_fp64_global_atomic_add != 1
+#error "Incorrectly defined __opencl_c_ext_fp64_global_atomic_add"
+#endif
+#if __opencl_c_ext_fp16_local_atomic_add != 1
+#error "Incorrectly defined __opencl_c_ext_fp16_local_atomic_add"
+#endif
+#if __opencl_c_ext_fp32_local_atomic_add != 1
+#error "Incorrectly defined __opencl_c_ext_fp32_local_atomic_add"
+#endif
+#if __opencl_c_ext_fp64_local_atomic_add != 1
+#error "Incorrectly defined __opencl_c_ext_fp64_local_atomic_add"
+#endif
+#if __opencl_c_ext_fp16_global_atomic_min_max != 1
+#error "Incorrectly defined __opencl_c_ext_fp16_global_atomic_min_max"
+#endif
+#if __opencl_c_ext_fp32_global_atomic_min_max != 1
+#error "Incorrectly defined __opencl_c_ext_fp32_global_atomic_min_max"
+#endif
+#if __opencl_c_ext_fp64_global_atomic_min_max != 1
+#error "Incorrectly defined __opencl_c_ext_fp64_global_atomic_min_max"
+#endif
+#if __opencl_c_ext_fp16_local_atomic_min_max != 1
+#error "Incorrectly defined __opencl_c_ext_fp16_local_atomic_min_max"
+#endif
+#if __opencl_c_ext_fp32_local_atomic_min_max != 1
+#error "Incorrectly defined __opencl_c_ext_fp32_local_atomic_min_max"
+#endif
+#if __opencl_c_ext_fp64_local_atomic_min_max != 1
+#error "Incorrectly defined __opencl_c_ext_fp64_local_atomic_min_max"
+#endif
 
 #else
 
@@ -171,6 +216,51 @@
 #ifdef __opencl_c_integer_dot_product_input_4x8bit_packed
 #error "Incorrect __opencl_c_integer_dot_product_input_4x8bit_packed define"
 #endif
+#ifdef cl_ext_float_atomics
+#error "Incorrect cl_ext_float_atomics define"
+#endif
+#ifdef __opencl_c_ext_fp16_global_atomic_load_store
+#error "Incorrectly __o

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

2021-09-13 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh added a comment.

Apologies for the delayed response.

In D106343#2967089 , @haonanya wrote:

> Hi, svenvh.
> Should we use cl_khr_int64_base_atomics and cl_khr_int64_extended_atomics to 
> guard  the functions using atomic_double type?
> Thanks very much.
>
>   #if defined(__opencl_c_ext_fp64_local_atomic_min_max)
>   double __ovld atomic_fetch_min(volatile __local atomic_double *object,
>  double operand);
>   #endif

This is perhaps something to raise at the specification level?

We can adjust the guards after any followup discussion if needed.  To progress 
the support of this extension, I've just committed your patch (with some minor 
whitespace fixes).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106343

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


[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

2021-09-13 Thread Henry Linjamäki via Phabricator via cfe-commits
linjamaki added inline comments.



Comment at: clang/lib/Basic/Targets/SPIR.h:233
+if (Opts.HIP && Opts.CUDAIsDevice)
+  // Enable address space mapping from HIP to SPIR-V.
+  // See comment on the SPIRDefIsGenMap table.

Anastasia wrote:
> My guess is that this is not only HIP specific but for example the same 
> applies to SYCL.
> 
> I am not sure if it makes more sense to move this into a `BaseSPIRTargetInfo` 
> since this is not really SPIR-V specific logic. It is just a clang design 
> misalignment between two address space concepts that has to be addressed 
> properly at some point.
> 
The DefaultIsGeneric AS mapping is enabled for SYCL in the 
BaseSPIRTargetInfo::adjust (which also means the mapping is available for both 
the SPIR and SPIR-V targets). On the other hand, the AS mapping for HIPSPV is 
enabled in SPIRVTargetInfo::adjust only as we intend to emit SPIR-V only. I’m 
under the impression that this is what was wanted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108621

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


[PATCH] D108695: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

2021-09-13 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0213d7ec0c50: [analyzer][NFCI] Allow clients of 
NoStateChangeFuncVisitor to check entire… (authored by Szelethus).

Changed prior to commit:
  https://reviews.llvm.org/D108695?vs=370549&id=372217#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108695

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/unittests/StaticAnalyzer/CMakeLists.txt
  clang/unittests/StaticAnalyzer/CallEventTest.cpp
  clang/unittests/StaticAnalyzer/CheckerRegistration.h
  clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
  clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
  clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Index: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
===
--- clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -51,7 +51,7 @@
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
   EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
-  EXPECT_EQ(Diags, "test.CustomChecker:Custom diagnostic description\n");
+  EXPECT_EQ(Diags, "test.CustomChecker: Custom diagnostic description\n");
 }
 
 //===--===//
@@ -169,7 +169,7 @@
 TEST(RegisterDeps, UnsatisfiedDependency) {
   std::string Diags;
   EXPECT_TRUE(runCheckerOnCode("void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "test.RegistrationOrder:test.RegistrationOrder\n");
+  EXPECT_EQ(Diags, "test.RegistrationOrder: test.RegistrationOrder\n");
 }
 
 //===--===//
@@ -272,7 +272,7 @@
   std::string Diags;
   EXPECT_TRUE(runCheckerOnCode(
   "void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "test.RegistrationOrder:test.WeakDep\ntest."
+  EXPECT_EQ(Diags, "test.RegistrationOrder: test.WeakDep\ntest."
"Dep\ntest.RegistrationOrder\n");
   Diags.clear();
 
@@ -280,31 +280,33 @@
   // but the dependencies are switched.
   EXPECT_TRUE(runCheckerOnCode(
   "void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "test.RegistrationOrder:test.Dep\ntest."
+  EXPECT_EQ(Diags, "test.RegistrationOrder: test.Dep\ntest."
"RegistrationOrder\ntest.WeakDep\n");
   Diags.clear();
 
   // Weak dependencies dont prevent dependent checkers from being enabled.
   EXPECT_TRUE(runCheckerOnCode(
   "void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "test.RegistrationOrder:test.Dep\ntest.RegistrationOrder\n");
+  EXPECT_EQ(Diags,
+"test.RegistrationOrder: test.Dep\ntest.RegistrationOrder\n");
   Diags.clear();
 
   // Nor will they be enabled just because a dependent checker is.
   EXPECT_TRUE(runCheckerOnCode(
   "void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "test.RegistrationOrder:test.Dep\ntest.RegistrationOrder\n");
+  EXPECT_EQ(Diags,
+"test.RegistrationOrder: test.Dep\ntest.RegistrationOrder\n");
   Diags.clear();
 
   EXPECT_TRUE(
   runCheckerOnCode("void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "test.RegistrationOrder:test.WeakDep2\ntest."
+  EXPECT_EQ(Diags, "test.RegistrationOrder: test.WeakDep2\ntest."
"Dep\ntest.RegistrationOrder\n");
   Diags.clear();
 
   EXPECT_TRUE(
   runCheckerOnCode("void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "test.RegistrationOrder:test.WeakDep2\ntest."
+  EXPECT_EQ(Diags, "test.RegistrationOrder: test.WeakDep2\ntest."
"WeakDep\ntest.Dep\ntest.RegistrationOrder\n");
   Diags.clear();
 }
@@ -414,7 +416,7 @@
   std::string Diags;
   EXPECT_TRUE(
   runCheckerOnCode("void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "test.RegistrationOrder:test.StrongDep\ntest."
+  EXPECT_EQ(Diags, "test.RegistrationOrder: test.StrongDep\ntest."
"WeakDep\ntest.Dep\ntest.RegistrationOrder\n");
   Diags.clear();
 
@@ -424,14 +426,14 @@
   // established in between the modeling portion and the weak dependency.
   EXPECT_TRUE(
   runCheckerOnCode("void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "test.RegistrationOrder:test.WeakDep\ntest."
+  EXPECT_EQ(Diags, "test.RegistrationOrder: test.WeakDep\ntest."
"StrongDep\ntest.Dep\ntest.RegistrationOrder\n");
   Diags.clear();
 
   // If a weak dependency is disabled, the checker itself can still be enabled.
   EXPECT_TRUE(runCheckerOnCode(
   "void f() {int i;}", Diags));
-  EXPECT_EQ(Diags, "test.RegistrationOrder:test.Dep\ntest."
+  EXPECT_EQ(Diags, "test.RegistrationOrder: test.Dep\ntest."
"RegistrationOrder\ntest.StrongDep\n");
   Diags.clear();
 
@

[clang] 0213d7e - [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

2021-09-13 Thread Kristóf Umann via cfe-commits

Author: Kristóf Umann
Date: 2021-09-13T13:50:01+02:00
New Revision: 0213d7ec0c501414d12020737fdc47e47e4392d9

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

LOG: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire 
function calls, rather than each ExplodedNode in it

Fix a compilation error due to a missing 'template' keyword.

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

Added: 
clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/unittests/StaticAnalyzer/CMakeLists.txt
clang/unittests/StaticAnalyzer/CallEventTest.cpp
clang/unittests/StaticAnalyzer/CheckerRegistration.h
clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index 139b0dcd51704..c42521376af92 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -633,6 +633,9 @@ class CXXConstructorCall;
 /// Descendants can define what a "state change is", like a change of value
 /// to a memory region, liveness, etc. For function calls where the state did
 /// not change as defined, a custom note may be constructed.
+///
+/// For a minimal example, check out
+/// clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp.
 class NoStateChangeFuncVisitor : public BugReporterVisitor {
 private:
   /// Frames modifying the state as defined in \c wasModifiedBeforeCallExit.
@@ -643,6 +646,8 @@ class NoStateChangeFuncVisitor : public BugReporterVisitor {
   /// many times (going up the path for each node and checking whether the
   /// region was written into) we instead lazily compute the stack frames
   /// along the path.
+  // TODO: Can't we just use a map instead? This is likely not as cheap as it
+  // makes the code 
diff icult to read.
   llvm::SmallPtrSet FramesModifying;
   llvm::SmallPtrSet FramesModifyingCalculated;
 
@@ -651,6 +656,8 @@ class NoStateChangeFuncVisitor : public BugReporterVisitor {
   /// The calculation is cached in FramesModifying.
   bool isModifiedInFrame(const ExplodedNode *CallExitBeginN);
 
+  void markFrameAsModifying(const StackFrameContext *SCtx);
+
   /// Write to \c FramesModifying all stack frames along the path in the 
current
   /// stack frame which modifies the state.
   void findModifyingFrames(const ExplodedNode *const CallExitBeginN);
@@ -658,11 +665,37 @@ class NoStateChangeFuncVisitor : public 
BugReporterVisitor {
 protected:
   bugreporter::TrackingKind TKind;
 
-  /// \return Whether the state was modified from the current node, \CurrN, to
-  /// the end of the stack fram, at \p CallExitBeginN.
-  virtual bool
-  wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
-const ExplodedNode *CallExitBeginN) = 0;
+  /// \return Whether the state was modified from the current node, \p CurrN, 
to
+  /// the end of the stack frame, at \p CallExitBeginN. \p CurrN and
+  /// \p CallExitBeginN are always in the same stack frame.
+  /// Clients should override this callback when a state change is important
+  /// not only on the entire function call, but inside of it as well.
+  /// Example: we may want to leave a note about the lack of locking/unlocking
+  /// on a particular mutex, but not if inside the function its state was
+  /// changed, but also restored. wasModifiedInFunction() wouldn't know of this
+  /// change.
+  virtual bool wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
+ const ExplodedNode *CallExitBeginN) {
+return false;
+  }
+
+  /// \return Whether the state was modified in the inlined function call in
+  /// between \p CallEnterN and \p CallExitEndN. Mind that the stack frame
+  /// retrieved from a CallEnterN and CallExitEndN is the *caller's* stack
+  /// frame! The inlined function's stack should be retrieved from either the
+  /// immediate successor to \p CallEnterN or immediate predecessor to
+  /// \p CallExitEndN.
+  /// Clients should override this function if a state changes local to the
+  /// inlined function are not interesting, only the change occuring as a
+  /// result of it.
+  /// Example: we want to leave a not about a leaked resource object not being
+  /// deallocated / its ownership changed inside a function, and we don't care
+  ///

[PATCH] D109544: [OpenMP] Add flag for setting debug in the offloading device

2021-09-13 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D109544#2996913 , @mgorny wrote:

> I'm sorry, it seems to have been caused by ccache. After clearing the cache, 
> I can't reproduce anymore.

The problem is that clang-ast-dump has incomplete dependences and sometimes 
picks stuff up from the install folder.
If you delete install it works, ccache might have a similar effect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109544

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


[clang] 8008009 - [OpenCL] Initialize temporaries in the private address space

2021-09-13 Thread Ole Strohm via cfe-commits

Author: Ole Strohm
Date: 2021-09-13T12:56:04+01:00
New Revision: 8008009fd25bf51c2c85c612bfefec64e975bbe4

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

LOG: [OpenCL] Initialize temporaries in the private address space

This patch fixes initializing temporaries, which are currently initialized
without an address space, meaning that no constructor can ever be applicable.
Now they will be constructed in the private addrspace.

Fixes the second issue in PR43296.

Reviewed By: Anastasia

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

Added: 
clang/test/SemaOpenCLCXX/temporaries.clcpp

Modified: 
clang/include/clang/Sema/Initialization.h
clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp

Removed: 




diff  --git a/clang/include/clang/Sema/Initialization.h 
b/clang/include/clang/Sema/Initialization.h
index 8feb66995f95c..4664861c4e32a 100644
--- a/clang/include/clang/Sema/Initialization.h
+++ b/clang/include/clang/Sema/Initialization.h
@@ -335,8 +335,15 @@ class alignas(8) InitializedEntity {
   }
 
   /// Create the initialization entity for a temporary.
-  static InitializedEntity InitializeTemporary(TypeSourceInfo *TypeInfo) {
-return InitializeTemporary(TypeInfo, TypeInfo->getType());
+  static InitializedEntity InitializeTemporary(ASTContext &Context,
+   TypeSourceInfo *TypeInfo) {
+QualType Type = TypeInfo->getType();
+if (Context.getLangOpts().OpenCLCPlusPlus) {
+  assert(!Type.hasAddressSpace() && "Temporary already has address 
space!");
+  Type = Context.getAddrSpaceQualType(Type, LangAS::opencl_private);
+}
+
+return InitializeTemporary(TypeInfo, Type);
   }
 
   /// Create the initialization entity for a temporary.

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 7627ed4f37d52..e76c4509d7888 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1453,7 +1453,8 @@ Sema::BuildCXXTypeConstructExpr(TypeSourceInfo *TInfo,
  "List initialization must have initializer list as expression.");
   SourceRange FullRange = SourceRange(TyBeginLoc, RParenOrBraceLoc);
 
-  InitializedEntity Entity = InitializedEntity::InitializeTemporary(TInfo);
+  InitializedEntity Entity =
+  InitializedEntity::InitializeTemporary(Context, TInfo);
   InitializationKind Kind =
   Exprs.size()
   ? ListInitialization
@@ -5290,7 +5291,8 @@ static bool evaluateTypeTrait(Sema &S, TypeTrait Kind, 
SourceLocation KWLoc,
 S, Sema::ExpressionEvaluationContext::Unevaluated);
 Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
 Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
-InitializedEntity To(InitializedEntity::InitializeTemporary(Args[0]));
+InitializedEntity To(
+InitializedEntity::InitializeTemporary(S.Context, Args[0]));
 InitializationKind InitKind(InitializationKind::CreateDirect(KWLoc, KWLoc,
  RParenLoc));
 InitializationSequence Init(S, To, InitKind, ArgExprs);

diff  --git a/clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp 
b/clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
index 0605a48d0fe91..1b97484767b1a 100644
--- a/clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
+++ b/clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
@@ -32,6 +32,8 @@ kernel void k() {
   __local X lx;
   __private X x;
 
+  __private X tx = X();
+
   __private Y py;
   __constant Y cy1; // expected-error{{variable in constant address space must 
be initialized}}
   __constant Y cy2(1); // expected-error{{no matching constructor for 
initialization of '__constant Y'}}

diff  --git a/clang/test/SemaOpenCLCXX/temporaries.clcpp 
b/clang/test/SemaOpenCLCXX/temporaries.clcpp
new file mode 100644
index 0..37d4f17183210
--- /dev/null
+++ b/clang/test/SemaOpenCLCXX/temporaries.clcpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -pedantic -ast-dump | FileCheck %s
+
+struct X {
+  X() __private = default;
+};
+
+// CHECK: VarDecl {{.*}} gx
+// CHECK: CXXTemporaryObjectExpr {{.*}} '__private X'
+__global X gx = X();
+
+void k() {
+  // CHECK: VarDecl {{.*}} x1
+  // CHECK: CXXTemporaryObjectExpr {{.*}} '__private X'
+  X x1 = X();
+
+  // CHECK: VarDecl {{.*}} x2
+  // CHECK: CXXConstructExpr {{.*}} 'const __private X'
+  const X x2;
+}



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


[PATCH] D107553: [C++4OpenCL] Initialize temporaries in the private address space

2021-09-13 Thread Ole Strohm via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8008009fd25b: [OpenCL] Initialize temporaries in the private 
address space (authored by olestrohm).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107553

Files:
  clang/include/clang/Sema/Initialization.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
  clang/test/SemaOpenCLCXX/temporaries.clcpp


Index: clang/test/SemaOpenCLCXX/temporaries.clcpp
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/temporaries.clcpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -pedantic -ast-dump | FileCheck %s
+
+struct X {
+  X() __private = default;
+};
+
+// CHECK: VarDecl {{.*}} gx
+// CHECK: CXXTemporaryObjectExpr {{.*}} '__private X'
+__global X gx = X();
+
+void k() {
+  // CHECK: VarDecl {{.*}} x1
+  // CHECK: CXXTemporaryObjectExpr {{.*}} '__private X'
+  X x1 = X();
+
+  // CHECK: VarDecl {{.*}} x2
+  // CHECK: CXXConstructExpr {{.*}} 'const __private X'
+  const X x2;
+}
Index: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
===
--- clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
+++ clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
@@ -32,6 +32,8 @@
   __local X lx;
   __private X x;
 
+  __private X tx = X();
+
   __private Y py;
   __constant Y cy1; // expected-error{{variable in constant address space must 
be initialized}}
   __constant Y cy2(1); // expected-error{{no matching constructor for 
initialization of '__constant Y'}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1453,7 +1453,8 @@
  "List initialization must have initializer list as expression.");
   SourceRange FullRange = SourceRange(TyBeginLoc, RParenOrBraceLoc);
 
-  InitializedEntity Entity = InitializedEntity::InitializeTemporary(TInfo);
+  InitializedEntity Entity =
+  InitializedEntity::InitializeTemporary(Context, TInfo);
   InitializationKind Kind =
   Exprs.size()
   ? ListInitialization
@@ -5290,7 +5291,8 @@
 S, Sema::ExpressionEvaluationContext::Unevaluated);
 Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/true);
 Sema::ContextRAII TUContext(S, S.Context.getTranslationUnitDecl());
-InitializedEntity To(InitializedEntity::InitializeTemporary(Args[0]));
+InitializedEntity To(
+InitializedEntity::InitializeTemporary(S.Context, Args[0]));
 InitializationKind InitKind(InitializationKind::CreateDirect(KWLoc, KWLoc,
  RParenLoc));
 InitializationSequence Init(S, To, InitKind, ArgExprs);
Index: clang/include/clang/Sema/Initialization.h
===
--- clang/include/clang/Sema/Initialization.h
+++ clang/include/clang/Sema/Initialization.h
@@ -335,8 +335,15 @@
   }
 
   /// Create the initialization entity for a temporary.
-  static InitializedEntity InitializeTemporary(TypeSourceInfo *TypeInfo) {
-return InitializeTemporary(TypeInfo, TypeInfo->getType());
+  static InitializedEntity InitializeTemporary(ASTContext &Context,
+   TypeSourceInfo *TypeInfo) {
+QualType Type = TypeInfo->getType();
+if (Context.getLangOpts().OpenCLCPlusPlus) {
+  assert(!Type.hasAddressSpace() && "Temporary already has address 
space!");
+  Type = Context.getAddrSpaceQualType(Type, LangAS::opencl_private);
+}
+
+return InitializeTemporary(TypeInfo, Type);
   }
 
   /// Create the initialization entity for a temporary.


Index: clang/test/SemaOpenCLCXX/temporaries.clcpp
===
--- /dev/null
+++ clang/test/SemaOpenCLCXX/temporaries.clcpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 %s -pedantic -ast-dump | FileCheck %s
+
+struct X {
+  X() __private = default;
+};
+
+// CHECK: VarDecl {{.*}} gx
+// CHECK: CXXTemporaryObjectExpr {{.*}} '__private X'
+__global X gx = X();
+
+void k() {
+  // CHECK: VarDecl {{.*}} x1
+  // CHECK: CXXTemporaryObjectExpr {{.*}} '__private X'
+  X x1 = X();
+
+  // CHECK: VarDecl {{.*}} x2
+  // CHECK: CXXConstructExpr {{.*}} 'const __private X'
+  const X x2;
+}
Index: clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
===
--- clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
+++ clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp
@@ -32,6 +32,8 @@
   __local X lx;
   __private X x;
 
+  __private X tx = X();
+
   __private Y py;
   __constant Y cy1; // expected-error{{variable in constant address space must be initialized}}
   __constant Y cy2(1); // expected-error{{no matching 

[clang] ec6c847 - [clang] Check unsupported types in expressions

2021-09-13 Thread Andrew Savonichev via cfe-commits

Author: Andrew Savonichev
Date: 2021-09-13T14:59:37+03:00
New Revision: ec6c847179fd019acae4d97a18f9e7d3961a6fdf

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

LOG: [clang] Check unsupported types in expressions

The patch adds missing diagnostics for cases like:

  float F3 = ((__float128)F1 * (__float128)F2) / 2.0f;

Sema::checkDeviceDecl (renamed to checkTypeSupport) is changed to work
with a type without the corresponding ValueDecl. It is also refactored
so that host diagnostics for unsupported types can be added here as
well.

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/CodeGen/ibm128-unsupported.c
clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
clang/test/SemaSYCL/float128.cpp
clang/test/SemaSYCL/int128.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 824d9bf469360..9c739c3c1f250 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10688,8 +10688,8 @@ def err_omp_invariant_or_linear_dependency : Error<
   "expected loop invariant expression or ' * %0 + ' 
kind of expression">;
 def err_omp_wrong_dependency_iterator_type : Error<
   "expected an integer or a pointer type of the outer loop counter '%0' for 
non-rectangular nests">;
-def err_device_unsupported_type
-: Error<"%0 requires %select{|%2 bit size}1 %3 type support, but device "
+def err_target_unsupported_type
+: Error<"%0 requires %select{|%2 bit size}1 %3 type support, but target "
 "'%4' does not support it">;
 def err_omp_lambda_capture_in_declare_target_not_to : Error<
   "variable captured in declare target region must appear in a to clause">;

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 710abeb1ea514..9e4175b6de811 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -12160,9 +12160,9 @@ class Sema final {
 return targetDiag(Loc, PD.getDiagID(), FD) << PD;
   }
 
-  /// Check if the expression is allowed to be used in expressions for the
-  /// offloading devices.
-  void checkDeviceDecl(ValueDecl *D, SourceLocation Loc);
+  /// Check if the type is allowed to be used for the current target.
+  void checkTypeSupport(QualType Ty, SourceLocation Loc,
+ValueDecl *D = nullptr);
 
   enum CUDAFunctionTarget {
 CFT_Device,

diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index a9867697a4c31..b23d92d101074 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1852,7 +1852,10 @@ Sema::SemaDiagnosticBuilder Sema::Diag(SourceLocation 
Loc, unsigned DiagID,
   return DB;
 }
 
-void Sema::checkDeviceDecl(ValueDecl *D, SourceLocation Loc) {
+void Sema::checkTypeSupport(QualType Ty, SourceLocation Loc, ValueDecl *D) {
+  if (!LangOpts.SYCLIsDevice && !(LangOpts.OpenMP && LangOpts.OpenMPIsDevice))
+return;
+
   if (isUnevaluatedContext())
 return;
 
@@ -1872,16 +1875,22 @@ void Sema::checkDeviceDecl(ValueDecl *D, SourceLocation 
Loc) {
 
   // Try to associate errors with the lexical context, if that is a function, 
or
   // the value declaration otherwise.
-  FunctionDecl *FD =
-  isa(C) ? cast(C) : dyn_cast(D);
+  FunctionDecl *FD = isa(C) ? cast(C)
+  : dyn_cast_or_null(D);
+
   auto CheckType = [&](QualType Ty) {
 if (Ty->isDependentType())
   return;
 
 if (Ty->isExtIntType()) {
   if (!Context.getTargetInfo().hasExtIntType()) {
-targetDiag(Loc, diag::err_device_unsupported_type, FD)
-<< D << false /*show bit size*/ << 0 /*bitsize*/
+PartialDiagnostic PD = PDiag(diag::err_target_unsupported_type);
+if (D)
+  PD << D;
+else
+  PD << "expression";
+targetDiag(Loc, PD, FD)
+<< false /*show bit size*/ << 0 /*bitsize*/
 << Ty << Context.getTargetInfo().getTriple().str();
   }
   return;
@@ -1903,16 +1912,24 @@ void Sema::checkDeviceDecl(ValueDecl *D, SourceLocation 
Loc) {
 (Ty->isIntegerType() && Context.getTypeSize(Ty) == 128 &&
  !Context.getTargetInfo().hasInt128Type()) ||
 LongDoubleMismatched) {
-  if (targetDiag(Loc, diag::err_device_unsupported_type, FD)
-  << D << true /*show bit size*/
+  PartialDiagnostic PD = PDiag(diag::err_target_unsupported_type);
+  if (D)
+PD << D;
+  else
+PD << "expression";
+
+  if (targetDiag(Loc, 

[PATCH] D109315: [clang] Check unsupported types in expressions

2021-09-13 Thread Andrew Savonichev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGec6c847179fd: [clang] Check unsupported types in expressions 
(authored by asavonic).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109315

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/ibm128-unsupported.c
  clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
  clang/test/SemaSYCL/float128.cpp
  clang/test/SemaSYCL/int128.cpp

Index: clang/test/SemaSYCL/int128.cpp
===
--- clang/test/SemaSYCL/int128.cpp
+++ clang/test/SemaSYCL/int128.cpp
@@ -26,19 +26,22 @@
   // expected-note@+1 3{{'A' defined here}}
   __int128 A;
   Z<__int128> C;
-  // expected-error@+2 {{'A' requires 128 bit size '__int128' type support, but device 'spir64' does not support it}}
-  // expected-error@+1 {{'field1' requires 128 bit size '__int128' type support, but device 'spir64' does not support it}}
+  // expected-error@+3 2{{expression requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
+  // expected-error@+2 {{'A' requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
+  // expected-error@+1 {{'field1' requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
   C.field1 = A;
-  // expected-error@+1 {{'bigfield' requires 128 bit size 'Z::BIGTYPE' (aka '__int128') type support, but device 'spir64' does not support it}}
+  // expected-error@+2 {{expression requires 128 bit size 'Z::BIGTYPE' (aka '__int128') type support, but target 'spir64' does not support it}}
+  // expected-error@+1 {{'bigfield' requires 128 bit size 'Z::BIGTYPE' (aka '__int128') type support, but target 'spir64' does not support it}}
   C.bigfield += 1.0;
 
-  // expected-error@+1 {{'A' requires 128 bit size '__int128' type support, but device 'spir64' does not support it}}
+  // expected-error@+1 {{'A' requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
   auto foo1 = [=]() {
 __int128 AA;
 // expected-note@+2 {{'BB' defined here}}
-// expected-error@+1 {{'A' requires 128 bit size '__int128' type support, but device 'spir64' does not support it}}
+// expected-error@+1 {{'A' requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
 auto BB = A;
-// expected-error@+1 {{'BB' requires 128 bit size '__int128' type support, but device 'spir64' does not support it}}
+// expected-error@+2 {{expression requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
+// expected-error@+1 {{'BB' requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
 BB += 1;
   };
 
@@ -50,7 +53,7 @@
 void foo2(){};
 
 // expected-note@+3 {{'P' defined here}}
-// expected-error@+2 {{'P' requires 128 bit size '__int128' type support, but device 'spir64' does not support it}}
+// expected-error@+2 {{'P' requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
 // expected-note@+1 2{{'foo' defined here}}
 __int128 foo(__int128 P) { return P; }
 
@@ -58,7 +61,8 @@
   // expected-note@+1 {{'operator __int128' defined here}}
   struct X { operator  __int128() const; } x;
   bool a = false;
-  // expected-error@+1 {{'operator __int128' requires 128 bit size '__int128' type support, but device 'spir64' does not support it}}
+  // expected-error@+2 2{{expression requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
+  // expected-error@+1 {{'operator __int128' requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
   a = x == __int128(0);
 }
 
@@ -74,12 +78,14 @@
   host_ok();
   kernel([=]() {
 decltype(CapturedToDevice) D;
-// expected-error@+1 {{'CapturedToDevice' requires 128 bit size '__int128' type support, but device 'spir64' does not support it}}
+// expected-error@+1 {{'CapturedToDevice' requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
 auto C = CapturedToDevice;
 Z<__int128> S;
-// expected-error@+1 {{'field1' requires 128 bit size '__int128' type support, but device 'spir64' does not support it}}
+// expected-error@+2 {{expression requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
+// expected-error@+1 {{'field1' requires 128 bit size '__int128' type support, but target 'spir64' does not support it}}
 S.field1 += 1;
-// expected-error@+1 {{'field' requires 128 bit size '__int128' type support, but device 'spir64' does not support it}}
+// expected-error@+2 {{expression requires 128 bit size '__int128' type support,

[PATCH] D109344: [AMDGPU][OpenMP] Use complex definitions from complex_cmath.h

2021-09-13 Thread Pushpinder Singh via Phabricator via cfe-commits
pdhaliwal added a comment.

Hey, Jon, sorry for late reply. I cannot reproduce this issue on nvptx so it 
seems to occur only on amdgcn. Will it be better if instead the name mangling 
issue is fixed? Or for the meantime, I could add #ifdef around as a temporary 
fix. Suggestions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109344

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


[clang] 6377426 - Revert "[clang] Check unsupported types in expressions"

2021-09-13 Thread Andrew Savonichev via cfe-commits

Author: Andrew Savonichev
Date: 2021-09-13T15:34:21+03:00
New Revision: 6377426b4a326b52733065609a5d811afd2b8b1b

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

LOG: Revert "[clang] Check unsupported types in expressions"

This reverts commit ec6c847179fd019acae4d97a18f9e7d3961a6fdf.

Fails on check-openmp:

/b/1/openmp-clang-x86_64-linux-debian/llvm.build/projects/openmp/runtime/test/lock/Output/omp_init_lock.c.tmp
--
Exit Code: -11

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/CodeGen/ibm128-unsupported.c
clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
clang/test/SemaSYCL/float128.cpp
clang/test/SemaSYCL/int128.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9c739c3c1f250..824d9bf469360 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10688,8 +10688,8 @@ def err_omp_invariant_or_linear_dependency : Error<
   "expected loop invariant expression or ' * %0 + ' 
kind of expression">;
 def err_omp_wrong_dependency_iterator_type : Error<
   "expected an integer or a pointer type of the outer loop counter '%0' for 
non-rectangular nests">;
-def err_target_unsupported_type
-: Error<"%0 requires %select{|%2 bit size}1 %3 type support, but target "
+def err_device_unsupported_type
+: Error<"%0 requires %select{|%2 bit size}1 %3 type support, but device "
 "'%4' does not support it">;
 def err_omp_lambda_capture_in_declare_target_not_to : Error<
   "variable captured in declare target region must appear in a to clause">;

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9e4175b6de811..710abeb1ea514 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -12160,9 +12160,9 @@ class Sema final {
 return targetDiag(Loc, PD.getDiagID(), FD) << PD;
   }
 
-  /// Check if the type is allowed to be used for the current target.
-  void checkTypeSupport(QualType Ty, SourceLocation Loc,
-ValueDecl *D = nullptr);
+  /// Check if the expression is allowed to be used in expressions for the
+  /// offloading devices.
+  void checkDeviceDecl(ValueDecl *D, SourceLocation Loc);
 
   enum CUDAFunctionTarget {
 CFT_Device,

diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index b23d92d101074..a9867697a4c31 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1852,10 +1852,7 @@ Sema::SemaDiagnosticBuilder Sema::Diag(SourceLocation 
Loc, unsigned DiagID,
   return DB;
 }
 
-void Sema::checkTypeSupport(QualType Ty, SourceLocation Loc, ValueDecl *D) {
-  if (!LangOpts.SYCLIsDevice && !(LangOpts.OpenMP && LangOpts.OpenMPIsDevice))
-return;
-
+void Sema::checkDeviceDecl(ValueDecl *D, SourceLocation Loc) {
   if (isUnevaluatedContext())
 return;
 
@@ -1875,22 +1872,16 @@ void Sema::checkTypeSupport(QualType Ty, SourceLocation 
Loc, ValueDecl *D) {
 
   // Try to associate errors with the lexical context, if that is a function, 
or
   // the value declaration otherwise.
-  FunctionDecl *FD = isa(C) ? cast(C)
-  : dyn_cast_or_null(D);
-
+  FunctionDecl *FD =
+  isa(C) ? cast(C) : dyn_cast(D);
   auto CheckType = [&](QualType Ty) {
 if (Ty->isDependentType())
   return;
 
 if (Ty->isExtIntType()) {
   if (!Context.getTargetInfo().hasExtIntType()) {
-PartialDiagnostic PD = PDiag(diag::err_target_unsupported_type);
-if (D)
-  PD << D;
-else
-  PD << "expression";
-targetDiag(Loc, PD, FD)
-<< false /*show bit size*/ << 0 /*bitsize*/
+targetDiag(Loc, diag::err_device_unsupported_type, FD)
+<< D << false /*show bit size*/ << 0 /*bitsize*/
 << Ty << Context.getTargetInfo().getTriple().str();
   }
   return;
@@ -1912,24 +1903,16 @@ void Sema::checkTypeSupport(QualType Ty, SourceLocation 
Loc, ValueDecl *D) {
 (Ty->isIntegerType() && Context.getTypeSize(Ty) == 128 &&
  !Context.getTargetInfo().hasInt128Type()) ||
 LongDoubleMismatched) {
-  PartialDiagnostic PD = PDiag(diag::err_target_unsupported_type);
-  if (D)
-PD << D;
-  else
-PD << "expression";
-
-  if (targetDiag(Loc, PD, FD)
-  << true /*show bit size*/
+  if (targetDiag(Loc, diag::err_device_unsupported_type, FD)
+  << D << true /*show bit size*/
   << static_cast(Context.getTypeSize(Ty)) << Ty
-  << Context.getTa

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

2021-09-13 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Can you just set `CLANG_TIDY_ENABLE_STATIC_ANALYZER=OFF` too if you care about 
this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109611

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


[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

2021-09-13 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added a comment.

In D109611#2997236 , @thakis wrote:

> Can you just set `CLANG_TIDY_ENABLE_STATIC_ANALYZER=OFF` too if you care 
> about this?

That doesn't have any effect on the libraries linked into clang-shlib. I am not 
building clang-tools-extra, so that does not make any difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109611

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


[clang] 648feab - [clang] Make the driver not diagnose errors on nonexistent linker inputs

2021-09-13 Thread Nico Weber via cfe-commits

Author: Nico Weber
Date: 2021-09-13T08:57:38-04:00
New Revision: 648feabc65d8ec20e5d39ac88e019d310955a6e6

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

LOG: [clang] Make the driver not diagnose errors on nonexistent linker inputs

When nonexistent linker inputs are passed to the driver, the linker
now errors out, instead of the compiler. If the linker does not run,
clang now emits a "warning: linker input unused" instead of an error
for nonexistent files.

The motivation for this change is that I noticed that
`clang-cl /winsysroot sysroot main.cc ole32.lib` emitted a
"ole32.lib not found" error, even though the linker finds it just fine when
I run `clang-cl /winsysroot sysroot main.cc /link ole32.lib`.

The same problem occurs if running `clang-cl main.cc ole32.lib` in a
non-MSVC shell.

The problem is that DiagnoseInputExistence() only looked for libs in %LIB%,
but MSVCToolChain uses much more involved techniques.

For this particular problem, we could make DiagnoseInputExistence() ask
the toolchain to see if it can find a .lib file, but in general the
driver can't know what the linker will do to find files, so it shouldn't
try. For example, if we implement PR24616, lld-link will look in the
registry to determine a good default for %LIB% if it isn't set.

This is less or a problem for the gcc driver, since .a paths there are
either passed via -l flags (which honor -L), or via a qualified path
(that doesn't honor -L) -- but for example ld.lld's --chroot flag
can also trigger this problem. Without this patch,
`clang -fuse-ld=lld -Wl,--chroot,some/dir /file.o` will complain that
`/file.o` doesn't exist, even though
`clang -fuse-ld=lld -Wl,--chroot,some/dir -Wl,/file.o` succeeds just fine.

This implements rnk's suggestion on the old bug PR27234.

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/test/Driver/cl-inputs.c
clang/test/Driver/cl-link.c
clang/test/Driver/unknown-arg.c
flang/test/Driver/missing-input.f90

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 7dea484267dcd..50791ec479397 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2130,19 +2130,6 @@ bool Driver::DiagnoseInputExistence(const DerivedArgList 
&Args, StringRef Value,
   if (getVFS().exists(Value))
 return true;
 
-  if (IsCLMode()) {
-if (!llvm::sys::path::is_absolute(Twine(Value)) &&
-llvm::sys::Process::FindInEnvPath("LIB", Value, ';'))
-  return true;
-
-if (Args.hasArg(options::OPT__SLASH_link) && Ty == types::TY_Object) {
-  // Arguments to the /link flag might cause the linker to search for 
object
-  // and library files in paths we don't know about. Don't error in such
-  // cases.
-  return true;
-}
-  }
-
   if (TypoCorrect) {
 // Check if the filename is a typo for an option flag. OptTable thinks
 // that all args that are not known options and that start with / are
@@ -2162,6 +2149,34 @@ bool Driver::DiagnoseInputExistence(const DerivedArgList 
&Args, StringRef Value,
 }
   }
 
+  // Don't error on apparently non-existent linker inputs, because they
+  // can be influenced by linker flags the clang driver might not understand.
+  // Examples:
+  // - `clang -fuse-ld=lld -Wl,--chroot,some/dir /file.o` will make lld look
+  //   for some/dir/file.o
+  // - `clang-cl main.cc ole32.lib` in a a non-MSVC shell will make the driver
+  //   module look for an MSVC installation in the registry. (We could ask
+  //   the MSVCToolChain object if it can find `ole32.lib`, but the logic to
+  //   look in the registry might move into lld-link in the future so that
+  //   lld-link invocations in non-MSVC shells just work too.)
+  // - `clang-cl ... /link ...` can pass arbitrary flags to the linker,
+  //   including /libpath:, which is used to find .lib and .obj files.
+  // So do not diagnose this on the driver level. Rely on the linker diagnosing
+  // it. (If we don't end up invoking the linker, this means we'll emit a
+  // "'linker' input unused [-Wunused-command-line-argument]" warning instead
+  // of an error.)
+  //
+  // Only do this skip after the typo correction step above. `/Brepo` is 
treated
+  // as TY_Object, but it's clearly a typo for `/Brepro`. It seems fine to emit
+  // an error if we have a flag that's within an edit distance of 1 from a
+  // flag. (Users can use `-Wl,` or `/linker` to launder the flag past the
+  // driver in the unlikely case they run into this.)
+  //
+  // Don't do this skip in clang-cl mode for inputs that start with a '/' --
+  // else we'd pass options like /libpath: through to the linker silently.
+  if (Ty == types::TY_Object && !(IsCLMode() && Value.star

[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

2021-09-13 Thread Nico Weber via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG648feabc65d8: [clang] Make the driver not diagnose errors on 
nonexistent linker inputs (authored by thakis).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109624

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/cl-inputs.c
  clang/test/Driver/cl-link.c
  clang/test/Driver/unknown-arg.c
  flang/test/Driver/missing-input.f90

Index: flang/test/Driver/missing-input.f90
===
--- flang/test/Driver/missing-input.f90
+++ flang/test/Driver/missing-input.f90
@@ -9,12 +9,12 @@
 ! FLANG DRIVER (flang-new)
 !--
 ! RUN: not %flang  2>&1 | FileCheck %s --check-prefix=FLANG-NO-FILE
-! RUN: not %flang %t 2>&1 | FileCheck %s --check-prefix=FLANG-NONEXISTENT-FILE
+! RUN: not %flang %t.f90 2>&1 | FileCheck %s --check-prefix=FLANG-NONEXISTENT-FILE
 
 !-
 ! FLANG FRONTEND DRIVER (flang-new -fc1)
 !-
-! RUN: not %flang_fc1 %t 2>&1  | FileCheck %s --check-prefix=FLANG-FC1-NONEXISTENT-FILE
+! RUN: not %flang_fc1 %t.f90 2>&1  | FileCheck %s --check-prefix=FLANG-FC1-NONEXISTENT-FILE
 ! RUN: not %flang_fc1 %S 2>&1  | FileCheck %s --check-prefix=FLANG-FC1-DIR
 
 !---
Index: clang/test/Driver/unknown-arg.c
===
--- clang/test/Driver/unknown-arg.c
+++ clang/test/Driver/unknown-arg.c
@@ -10,6 +10,8 @@
 // RUN: FileCheck %s --check-prefix=CL-DID-YOU-MEAN
 // RUN: %clang_cl /Brepo -### -- %s 2>&1 | \
 // RUN: FileCheck %s --check-prefix=CL-DID-YOU-MEAN-SLASH
+// RUN: %clang_cl /Brepo -### /Tc%s /link 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CL-DID-YOU-MEAN-SLASH
 // RUN: not %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -c -Werror=unknown-argument -### -- %s 2>&1 | \
 // RUN: FileCheck %s --check-prefix=CL-ERROR
 // RUN: not %clang_cl -helo -Werror=unknown-argument -### -- %s 2>&1 | \
Index: clang/test/Driver/cl-link.c
===
--- clang/test/Driver/cl-link.c
+++ clang/test/Driver/cl-link.c
@@ -47,12 +47,20 @@
 // DEBUG: link.exe
 // DEBUG: "-debug"
 
+// Don't pass through /libpath: if it's not after a /link flag:
+// RUN: %clang_cl /Tc%s /libpath:foo -fuse-ld=link -### /link /libpath:bar 2>&1 | FileCheck --check-prefix=LIBPATH %s
+// LIBPATH: error: no such file or directory: '/libpath:foo'
+// LIBPATH: libpath:bar
+
 // PR27234
 // RUN: %clang_cl /Tc%s nonexistent.obj -fuse-ld=link -### /link /libpath:somepath 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
 // RUN: %clang_cl /Tc%s nonexistent.lib -fuse-ld=link -### /link /libpath:somepath 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
+// RUN: %clang_cl /Tc%s nonexistent.obj -fuse-ld=link -### /winsysroot somepath 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
+// RUN: %clang_cl /Tc%s nonexistent.lib -fuse-ld=link -### /winsysroot somepath 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
+// RUN: %clang_cl /Tc%s nonexistent.obj -fuse-ld=link -### 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
+// RUN: %clang_cl /Tc%s nonexistent.lib -fuse-ld=link -### 2>&1 | FileCheck --check-prefix=NONEXISTENT %s
 // NONEXISTENT-NOT: no such file
 // NONEXISTENT: link.exe
-// NONEXISTENT: "/libpath:somepath"
 // NONEXISTENT: nonexistent
 
 // RUN: %clang_cl /Tc%s -fuse-ld=lld -### 2>&1 | FileCheck --check-prefix=USE_LLD %s
Index: clang/test/Driver/cl-inputs.c
===
--- clang/test/Driver/cl-inputs.c
+++ clang/test/Driver/cl-inputs.c
@@ -55,9 +55,9 @@
 // LIBINPUT: "cl-test.lib"
 
 // RUN: env LIB=%S/Inputs/cl-libs %clang_cl -fuse-ld=link -### -- %s cl-test2.lib 2>&1 | FileCheck -check-prefix=LIBINPUT2 %s
-// LIBINPUT2: error: no such file or directory: 'cl-test2.lib'
+// LIBINPUT2-NOT: error: no such file or directory: 'cl-test2.lib'
 // LIBINPUT2: link.exe"
-// LIBINPUT2-NOT: "cl-test2.lib"
+// LIBINPUT2: "cl-test2.lib"
 
 // RUN: %clang_cl -fuse-ld=link -### -- %s /nonexisting.lib 2>&1 | FileCheck -check-prefix=LIBINPUT3 %s
 // LIBINPUT3: error: no such file or directory: '/nonexisting.lib'
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2130,19 +2130,6 @@
   if (getVFS().exists(Value))
 return true;
 
-  if (IsCLMode()) {
-if (!llvm::sys::path::is_absolute(Twine(Value)) &&
-llvm::sys::Process::FindInEnvPath("LIB", Value, ';'))
-  return true;
-
-if (Args.hasArg(options::OPT__SLASH_link) && Ty == types::TY_Object) {
-  // Arguments to the /link flag might cau

[clang] 9d359f6 - [analyzer] MallocChecker: Add notes from NoOwnershipChangeVisitor only when a function "intents", but doesn't change ownership, enable by default

2021-09-13 Thread Kristóf Umann via cfe-commits

Author: Kristóf Umann
Date: 2021-09-13T15:01:20+02:00
New Revision: 9d359f6c738632c6973e9f5328b10bf39b3df55a

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

LOG: [analyzer] MallocChecker: Add notes from NoOwnershipChangeVisitor only 
when a function "intents", but doesn't change ownership, enable by default

D105819 Added NoOwnershipChangeVisitor, but it is only registered when an
off-by-default, hidden checker option was enabled. The reason behind this was
that it grossly overestimated the set of functions that really needed a note:

std::string getTrainName(const Train *T) {
  return T->name;
} // note: Retuning without changing the ownership of or deallocating memory
// Umm... I mean duh? Nor would I expect this function to do anything like 
that...

void foo() {
  Train *T = new Train("Land Plane");
  print(getTrainName(T)); // note: calling getTrainName / returning from 
getTrainName
} // warn: Memory leak

This patch adds a heuristic that guesses that any function that has an explicit
operator delete call could have be responsible for deallocating the memory that
ended up leaking. This is wy too conservative (see the TODOs in the new
function), but it safer to err on the side of too little than too much, and
would allow us to enable the option by default *now*, and add refinements
one-by-one.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/NewDeleteLeaks.cpp
clang/test/Analysis/analyzer-config.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 125ef859d1ebb..86513a3e541be 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -493,8 +493,8 @@ def DynamicMemoryModeling: Checker<"DynamicMemoryModeling">,
   "that neither deallocated it, or have taken responsibility "
   "of the ownership are noted, similarly to "
   "NoStoreFuncVisitor.",
-  "false",
-  InAlpha,
+  "true",
+  Released,
   Hide>
   ]>,
   Dependencies<[CStringModeling]>,

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index d97e8c33ce254..9899298b348a0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -52,6 +52,8 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ParentMap.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
@@ -791,9 +793,28 @@ class NoOwnershipChangeVisitor final : public 
NoStateChangeFuncVisitor {
 return "";
   }
 
+  bool doesFnIntendToHandleOwnership(const Decl *Callee, ASTContext &ACtx) {
+using namespace clang::ast_matchers;
+const FunctionDecl *FD = dyn_cast(Callee);
+if (!FD)
+  return false;
+// TODO: Operator delete is hardly the only deallocator -- Can we reuse
+// isFreeingCall() or something thats already here?
+auto Deallocations = match(
+stmt(hasDescendant(cxxDeleteExpr().bind("delete"))
+ ), *FD->getBody(), ACtx);
+// TODO: Ownership my change with an attempt to store the allocated memory.
+return !Deallocations.empty();
+  }
+
   virtual bool
   wasModifiedInFunction(const ExplodedNode *CallEnterN,
 const ExplodedNode *CallExitEndN) override {
+if (!doesFnIntendToHandleOwnership(
+CallExitEndN->getFirstPred()->getLocationContext()->getDecl(),
+CallExitEndN->getState()->getAnalysisManager().getASTContext()))
+  return true;
+
 if (CallEnterN->getState()->get(Sym) !=
 CallExitEndN->getState()->get(Sym))
   return true;

diff  --git a/clang/test/Analysis/NewDeleteLeaks.cpp 
b/clang/test/Analysis/NewDeleteLeaks.cpp
index 28040d9d0d36b..57c7e57c98e32 100644
--- a/clang/test/Analysis/NewDeleteLeaks.cpp
+++ b/clang/test/Analysis/NewDeleteLeaks.cpp
@@ -20,15 +20,16 @@
 
 bool coin();
 
+// TODO: AST analysis of sink would reveal that it doesn't intent to free the
+// allocated memory, but in this instance, its also the only function with
+// the ability to do so, we should see a note here.
 namespace memory_allocated_in_fn_call {
 
 void sink(int *P) {
-} // ownership-note {{Returning without deallocating memory or storing the 
pointer for la

[PATCH] D108753: [analyzer] MallocChecker: Add notes from NoOwnershipChangeVisitor only when a function "intents", but doesn't change ownership, enable by default

2021-09-13 Thread Kristóf Umann 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 rG9d359f6c7386: [analyzer] MallocChecker: Add notes from 
NoOwnershipChangeVisitor only when a… (authored by Szelethus).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108753

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/NewDeleteLeaks.cpp
  clang/test/Analysis/analyzer-config.c

Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -117,7 +117,7 @@
 // CHECK-NEXT: suppress-null-return-paths = true
 // CHECK-NEXT: track-conditions = true
 // CHECK-NEXT: track-conditions-debug = false
-// CHECK-NEXT: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes = false
+// CHECK-NEXT: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes = true
 // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: verbose-report-filename = false
Index: clang/test/Analysis/NewDeleteLeaks.cpp
===
--- clang/test/Analysis/NewDeleteLeaks.cpp
+++ clang/test/Analysis/NewDeleteLeaks.cpp
@@ -20,15 +20,16 @@
 
 bool coin();
 
+// TODO: AST analysis of sink would reveal that it doesn't intent to free the
+// allocated memory, but in this instance, its also the only function with
+// the ability to do so, we should see a note here.
 namespace memory_allocated_in_fn_call {
 
 void sink(int *P) {
-} // ownership-note {{Returning without deallocating memory or storing the pointer for later deallocation}}
+}
 
 void foo() {
   sink(new int(5)); // expected-note {{Memory is allocated}}
-// ownership-note@-1 {{Calling 'sink'}}
-// ownership-note@-2 {{Returning from 'sink'}}
 } // expected-warning {{Potential memory leak [cplusplus.NewDeleteLeaks]}}
 // expected-note@-1 {{Potential memory leak}}
 
@@ -109,17 +110,14 @@
 
 } // namespace memory_shared_with_ptr_of_same_lifetime
 
-// TODO: We don't want a note here. sink() doesn't seem like a function that
-// even attempts to take care of any memory ownership problems.
 namespace memory_passed_into_fn_that_doesnt_intend_to_free {
 
 void sink(int *P) {
-} // ownership-note {{Returning without deallocating memory or storing the pointer for later deallocation}}
+}
 
 void foo() {
   int *ptr = new int(5); // expected-note {{Memory is allocated}}
-  sink(ptr); // ownership-note {{Calling 'sink'}}
- // ownership-note@-1 {{Returning from 'sink'}}
+  sink(ptr);
 } // expected-warning {{Potential leak of memory pointed to by 'ptr' [cplusplus.NewDeleteLeaks]}}
 // expected-note@-1 {{Potential leak}}
 
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -52,6 +52,8 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ParentMap.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
@@ -791,9 +793,28 @@
 return "";
   }
 
+  bool doesFnIntendToHandleOwnership(const Decl *Callee, ASTContext &ACtx) {
+using namespace clang::ast_matchers;
+const FunctionDecl *FD = dyn_cast(Callee);
+if (!FD)
+  return false;
+// TODO: Operator delete is hardly the only deallocator -- Can we reuse
+// isFreeingCall() or something thats already here?
+auto Deallocations = match(
+stmt(hasDescendant(cxxDeleteExpr().bind("delete"))
+ ), *FD->getBody(), ACtx);
+// TODO: Ownership my change with an attempt to store the allocated memory.
+return !Deallocations.empty();
+  }
+
   virtual bool
   wasModifiedInFunction(const ExplodedNode *CallEnterN,
 const ExplodedNode *CallExitEndN) override {
+if (!doesFnIntendToHandleOwnership(
+CallExitEndN->getFirstPred()->getLocationContext()->getDecl(),
+CallExitEndN->getState()->getAnalysisManager().getASTContext()))
+  return true;
+
 if (CallEnterN->getState()->get(Sym) !=
 CallExitEndN->getState()->get(Sym))
   return true;
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -493,8 +493,8 @@
   "that neither deallocated it, o

[PATCH] D108912: [release][analyzer] Add 13.0.0 release notes

2021-09-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Gonna land this in a day or two, regardless of whether its accepted! Please 
take a look if you have anything to object to!


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

https://reviews.llvm.org/D108912

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


[PATCH] D107450: [clang-tidy] Fix wrong FIxIt in performance-move-const-arg

2021-09-13 Thread gehry via Phabricator via cfe-commits
Sockke added a comment.

kindly ping.


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

https://reviews.llvm.org/D107450

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


[PATCH] D105191: [Clang][OpenMP] Add support for Static Device Libraries

2021-09-13 Thread Ye Luo via Phabricator via cfe-commits
ye-luo added a comment.

@saiislam did you turn on offload? 
https://github.com/ye-luo/openmp-target/wiki/OpenMP-offload-compilers#llvm-clang
On NVIDIA, it fails at CMake step. On AMD, make step stops because of unrelated 
issue.

Please make the exact reproducer 1 working. Right now I got

  $ clang++ -fopenmp -fopenmp-targets=nvptx64 -Xopenmp-target=nvptx64 
-march=sm_80 modf.o
  nvlink fatal   : Could not open input file '/tmp/modf-88b730.cubin'
  /soft/llvm/main-patched/bin/clang-nvlink-wrapper: error: 'nvlink' failed
  clang-14: error: nvlink command failed with exit code 1 (use -v to see 
invocation)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105191

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


[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

2021-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:56
+hasType(Container),
+hasType(pointsTo(Container)),
+hasType(references(Container))

Is this `pointsTo()` correct? IIRC, that would be trying to capture something 
like:
```
std::vector *container;
&container[0];
```
but this wouldn't call the `operator[]` overload, it'd be using the builtin 
array subscripting, so I'm not certain this would match anything.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:63
+hasType(Container),
+hasType(pointsTo(Container)),
+hasType(references(Container))

Same question here about `pointsTo()`.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:75
+  hasType(Container),
+  hasType(pointsTo(Container)),
+  hasType(references(Container))

Here too.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:88
+  hasType(Container),
+  hasType(pointsTo(Container)),
+  hasType(references(Container))

And here.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:102-105
+  std::string ReplacementText;
+  ReplacementText = std::string(Lexer::getSourceText(
+  CharSourceRange::getTokenRange(DRE->getSourceRange()),
+  *Result.SourceManager, getLangOpts()));





Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h:23
+/// memory access when the container is empty.  Cases where the constant is not
+/// explictly zero can be addressed through teh clang static analyzer, and 
those
+/// which cannot be statically identified can be caught using UBSan.





Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h:45
+
+#endif




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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


[PATCH] D109686: Check supported architectures in sseXYZ/avxXYZ headers

2021-09-13 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision.
serge-sans-paille added reviewers: craig.topper, RKSimon.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It doesn't make sense to include those headers on the wrong architecture,
provide an explicit error message in that case.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109686

Files:
  clang/lib/Headers/ammintrin.h
  clang/lib/Headers/emmintrin.h
  clang/lib/Headers/immintrin.h
  clang/lib/Headers/mmintrin.h
  clang/lib/Headers/nmmintrin.h
  clang/lib/Headers/pmmintrin.h
  clang/lib/Headers/smmintrin.h
  clang/lib/Headers/tmmintrin.h
  clang/lib/Headers/wmmintrin.h
  clang/lib/Headers/xmmintrin.h
  clang/test/Headers/xmmintrin-unsupported.c

Index: clang/test/Headers/xmmintrin-unsupported.c
===
--- /dev/null
+++ clang/test/Headers/xmmintrin-unsupported.c
@@ -0,0 +1,5 @@
+// RUN: not %clang_cc1 %s -triple aarch64-eabi -fsyntax-only 2>&1 | FileCheck %s
+//
+// REQUIRES: x86-registered-target
+// CHECK: This header is only meant to be used on x86 and x64 architecture
+#include 
Index: clang/lib/Headers/xmmintrin.h
===
--- clang/lib/Headers/xmmintrin.h
+++ clang/lib/Headers/xmmintrin.h
@@ -10,6 +10,10 @@
 #ifndef __XMMINTRIN_H
 #define __XMMINTRIN_H
 
+#if !defined(__i386__) && !defined(__x86_64__)
+#error "This header is only meant to be used on x86 and x64 architecture"
+#endif
+
 #include 
 
 typedef int __v4si __attribute__((__vector_size__(16)));
Index: clang/lib/Headers/wmmintrin.h
===
--- clang/lib/Headers/wmmintrin.h
+++ clang/lib/Headers/wmmintrin.h
@@ -10,6 +10,10 @@
 #ifndef __WMMINTRIN_H
 #define __WMMINTRIN_H
 
+#if !defined(__i386__) && !defined(__x86_64__)
+#error "This header is only meant to be used on x86 and x64 architecture"
+#endif
+
 #include 
 
 #include <__wmmintrin_aes.h>
Index: clang/lib/Headers/tmmintrin.h
===
--- clang/lib/Headers/tmmintrin.h
+++ clang/lib/Headers/tmmintrin.h
@@ -10,6 +10,10 @@
 #ifndef __TMMINTRIN_H
 #define __TMMINTRIN_H
 
+#if !defined(__i386__) && !defined(__x86_64__)
+#error "This header is only meant to be used on x86 and x64 architecture"
+#endif
+
 #include 
 
 /* Define the default attributes for the functions in this file. */
Index: clang/lib/Headers/smmintrin.h
===
--- clang/lib/Headers/smmintrin.h
+++ clang/lib/Headers/smmintrin.h
@@ -10,6 +10,10 @@
 #ifndef __SMMINTRIN_H
 #define __SMMINTRIN_H
 
+#if !defined(__i386__) && !defined(__x86_64__)
+#error "This header is only meant to be used on x86 and x64 architecture"
+#endif
+
 #include 
 
 /* Define the default attributes for the functions in this file. */
Index: clang/lib/Headers/pmmintrin.h
===
--- clang/lib/Headers/pmmintrin.h
+++ clang/lib/Headers/pmmintrin.h
@@ -10,6 +10,10 @@
 #ifndef __PMMINTRIN_H
 #define __PMMINTRIN_H
 
+#if !defined(__i386__) && !defined(__x86_64__)
+#error "This header is only meant to be used on x86 and x64 architecture"
+#endif
+
 #include 
 
 /* Define the default attributes for the functions in this file. */
Index: clang/lib/Headers/nmmintrin.h
===
--- clang/lib/Headers/nmmintrin.h
+++ clang/lib/Headers/nmmintrin.h
@@ -10,6 +10,10 @@
 #ifndef __NMMINTRIN_H
 #define __NMMINTRIN_H
 
+#if !defined(__i386__) && !defined(__x86_64__)
+#error "This header is only meant to be used on x86 and x64 architecture"
+#endif
+
 /* To match expectations of gcc we put the sse4.2 definitions into smmintrin.h,
just include it now then.  */
 #include 
Index: clang/lib/Headers/mmintrin.h
===
--- clang/lib/Headers/mmintrin.h
+++ clang/lib/Headers/mmintrin.h
@@ -10,6 +10,10 @@
 #ifndef __MMINTRIN_H
 #define __MMINTRIN_H
 
+#if !defined(__i386__) && !defined(__x86_64__)
+#error "This header is only meant to be used on x86 and x64 architecture"
+#endif
+
 typedef long long __m64 __attribute__((__vector_size__(8), __aligned__(8)));
 
 typedef long long __v1di __attribute__((__vector_size__(8)));
Index: clang/lib/Headers/immintrin.h
===
--- clang/lib/Headers/immintrin.h
+++ clang/lib/Headers/immintrin.h
@@ -10,6 +10,10 @@
 #ifndef __IMMINTRIN_H
 #define __IMMINTRIN_H
 
+#if !defined(__i386__) && !defined(__x86_64__)
+#error "This header is only meant to be used on x86 and x64 architecture"
+#endif
+
 #include 
 
 #if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||  \
Index: clang/lib/Headers/emmintrin.h
=

[PATCH] D97699: [analyzer] Add InvalidPtrChecker

2021-09-13 Thread Zurab Tsinadze via Phabricator via cfe-commits
zukatsinadze updated this revision to Diff 372237.
zukatsinadze added a comment.

Thanks for the review @martong

I've fixed all the suggestions.


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

https://reviews.llvm.org/D97699

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
  clang/test/Analysis/cert/env31-c.c
  clang/test/Analysis/cert/env34-c-cert-examples.c
  clang/test/Analysis/cert/env34-c.c

Index: clang/test/Analysis/cert/env34-c.c
===
--- /dev/null
+++ clang/test/Analysis/cert/env34-c.c
@@ -0,0 +1,331 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.env.InvalidPtr\
+// RUN:  -analyzer-output=text -verify -Wno-unused %s
+
+#include "../Inputs/system-header-simulator.h"
+char *getenv(const char *name);
+char *setlocale(int category, const char *locale);
+char *strerror(int errnum);
+
+typedef struct {
+  char * field;
+} lconv;
+lconv *localeconv(void);
+
+typedef struct {
+} tm;
+char *asctime(const tm *timeptr);
+
+int strcmp(const char*, const char*);
+extern void foo(char *e);
+extern char* bar();
+
+
+void getenv_test1() {
+  char *p;
+
+  p = getenv("VAR");
+  *p; // no-warning
+
+  p = getenv("VAR2");
+  *p; // no-warning, getenv result was assigned to the same pointer
+}
+
+void getenv_test2() {
+  char *p, *p2;
+
+  p = getenv("VAR");
+  // expected-note@-1{{previous function call was here}}
+  *p; // no-warning
+
+  p2 = getenv("VAR2");
+  // expected-note@-1{{'getenv' call may invalidate the the result of the previous 'getenv'}}
+
+  *p;
+  // expected-warning@-1{{dereferencing an invalid pointer}}
+  // expected-note@-2{{dereferencing an invalid pointer}}
+}
+
+void getenv_test3() {
+  char *p, *p2, *p3;
+
+  p = getenv("VAR");
+  *p; // no-warning
+
+  p = getenv("VAR2");
+  // expected-note@-1{{previous function call was here}}
+  p2 = getenv("VAR2");
+  // expected-note@-1{{'getenv' call may invalidate the the result of the previous 'getenv'}}
+
+  p3 = getenv("VAR3");
+
+  *p;
+  // expected-warning@-1{{dereferencing an invalid pointer}}
+  // expected-note@-2{{dereferencing an invalid pointer}}
+}
+
+void getenv_test4() {
+  char *p, *p2, *p3;
+
+  p = getenv("VAR");
+  // expected-note@-1{{previous function call was here}}
+  p2 = getenv("VAR2");
+  // expected-note@-1{{'getenv' call may invalidate the the result of the previous 'getenv'}}
+  p3 = getenv("VAR3");
+
+  *p;
+  // expected-warning@-1{{dereferencing an invalid pointer}}
+  // expected-note@-2{{dereferencing an invalid pointer}}
+}
+
+void getenv_test5() {
+  char *p, *p2, *p3;
+
+  p = getenv("VAR");
+  p2 = getenv("VAR2");
+  // expected-note@-1{{previous function call was here}}
+  p3 = getenv("VAR3");
+  // expected-note@-1{{'getenv' call may invalidate the the result of the previous 'getenv'}}
+
+  *p2;
+  // expected-warning@-1{{dereferencing an invalid pointer}}
+  // expected-note@-2{{dereferencing an invalid pointer}}
+}
+
+void getenv_test6() {
+  char *p, *p2;
+  p = getenv("VAR");
+  *p; // no-warning
+
+  p = getenv("VAR2");
+  // expected-note@-1{{previous function call was here}}
+  *p; // no-warning
+
+  p2 = getenv("VAR3");
+  // expected-note@-1{{previous function call was here}}
+  // expected-note@-2{{'getenv' call may invalidate the the result of the previous 'getenv'}}
+
+  *p;
+  // expected-warning@-1{{dereferencing an invalid pointer}}
+  // expected-note@-2{{dereferencing an invalid pointer}}
+
+  *p2; // no-warning
+
+  p = getenv("VAR4");
+  // expected-note@-1{{'getenv' call may invalidate the the result of the previous 'getenv'}}
+
+  *p; // no-warning
+  *p2;
+  // expected-warning@-1{{dereferencing an invalid pointer}}
+  // expected-note@-2{{dereferencing an invalid pointer}}
+}
+
+void getenv_test7() {
+  char *p, *p2;
+  p = getenv("VAR");
+  // expected-note@-1{{previous function call was here}}
+  *p; // no-warning
+
+  p2 = getenv("VAR2");
+  // expected-note@-1{{'getenv' call may invalidate the the result of the previous 'getenv'}}
+
+  foo(p);
+  // expected-warning@-1{{use of invalidated pointer 'p' in a function call}}
+  // expected-note@-2{{use of invalidated pointer 'p' in a function call}}
+}
+
+void getenv_test8() {
+  static const char *array[] = {
+ 0,
+ 0,
+ "/var/tmp",
+ "/usr/tmp",
+ "/tmp",
+ "."
+  };
+
+  if( !array[0] )
+  // expected-note@-1{{Taking true branch}}
+array[0] = getenv("TEMPDIR");
+// expected-note@-1{{previous function call was here}}
+
+  if( !array[1] )
+  // expected-note@-1{{Taking true branch}}
+array[1] = getenv("TMPDIR");
+// expected-note@-1{{'getenv' call may invalidate the the result of the previous 'getenv'}}
+
+  *array[0];
+  // expected-warning@-1{{dereferencing an invalid pointer}}
+  // expected-note@-2{{dereferencing a

[PATCH] D97699: [analyzer] Add InvalidPtrChecker

2021-09-13 Thread Zurab Tsinadze via Phabricator via cfe-commits
zukatsinadze marked 4 inline comments as done.
zukatsinadze added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:163
+// memory region returned by previous call of this function
+REGISTER_MAP_WITH_PROGRAMSTATE(PreviousCallResultMap, const char *,
+   const MemRegion *)

martong wrote:
> I think we could use the canonical `FunctionDecl*` as the key instead of 
> `const char *`. Then all the `eval` functions like `evalGetenv` and alike 
> could be substituted with one simple function.
Thanks! Those functions annoyed me a lot.


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

https://reviews.llvm.org/D97699

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


[PATCH] D106713: Thread safety analysis: Warn when demoting locks on back edges

2021-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Accepting the review -- if you don't hear back from @delesley in the next few 
days, I think it's fine to land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106713

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


[PATCH] D109437: [PowerPC] FP compare and test XL compat builtins.

2021-09-13 Thread Quinn Pham via Phabricator via cfe-commits
quinnp updated this revision to Diff 372243.
quinnp added a comment.

Adding SemaChecking for the first argument of __builitn_ppc_test_data_class so
the llvm_unreachable in CGBuiltin is actually unreachable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109437

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-error.c
  clang/test/CodeGen/builtins-ppc-xlcompat-test.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-test.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-test.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-test.ll
@@ -0,0 +1,99 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   -mcpu=pwr9 < %s | FileCheck %s
+
+define i32 @test_builtin_ppc_compare_exp_eq(double %d) {
+; CHECK-LABEL: test_builtin_ppc_compare_exp_eq:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xscmpexpdp 0, 1, 1
+; CHECK-NEXT:li 3, 0
+; CHECK-NEXT:li 4, 1
+; CHECK-NEXT:iseleq 3, 4, 3
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call i32 @llvm.ppc.compare.exp.eq(double %d, double %d)
+  ret i32 %0
+}
+
+declare i32 @llvm.ppc.compare.exp.eq(double, double)
+
+define i32 @test_builtin_ppc_compare_exp_lt(double %d) {
+; CHECK-LABEL: test_builtin_ppc_compare_exp_lt:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xscmpexpdp 0, 1, 1
+; CHECK-NEXT:li 3, 0
+; CHECK-NEXT:li 4, 1
+; CHECK-NEXT:isellt 3, 4, 3
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call i32 @llvm.ppc.compare.exp.lt(double %d, double %d)
+  ret i32 %0
+}
+
+declare i32 @llvm.ppc.compare.exp.lt(double, double)
+
+define i32 @test_builtin_ppc_compare_exp_gt(double %d) {
+; CHECK-LABEL: test_builtin_ppc_compare_exp_gt:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xscmpexpdp 0, 1, 1
+; CHECK-NEXT:li 3, 0
+; CHECK-NEXT:li 4, 1
+; CHECK-NEXT:iselgt 3, 4, 3
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call i32 @llvm.ppc.compare.exp.gt(double %d, double %d)
+  ret i32 %0
+}
+
+declare i32 @llvm.ppc.compare.exp.gt(double, double)
+
+define i32 @test_builtin_ppc_compare_exp_uo(double %d) {
+; CHECK-LABEL: test_builtin_ppc_compare_exp_uo:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xscmpexpdp 0, 1, 1
+; CHECK-NEXT:li 3, 0
+; CHECK-NEXT:li 4, 1
+; CHECK-NEXT:isel 3, 4, 3, 3
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call i32 @llvm.ppc.compare.exp.uo(double %d, double %d)
+  ret i32 %0
+}
+
+declare i32 @llvm.ppc.compare.exp.uo(double, double)
+
+define i32 @test_builtin_ppc_test_data_class_d(double %d) {
+; CHECK-LABEL: test_builtin_ppc_test_data_class_d:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xststdcdp 0, 1, 0
+; CHECK-NEXT:li 3, 0
+; CHECK-NEXT:li 4, 1
+; CHECK-NEXT:iseleq 3, 4, 3
+; CHECK-NEXT:blr
+entry:
+  %test_data_class = tail call i32 @llvm.ppc.test.data.class.d(double %d, i32 0)
+  ret i32 %test_data_class
+}
+
+declare i32 @llvm.ppc.test.data.class.d(double, i32 immarg)
+
+define i32 @test_builtin_ppc_test_data_class_f(float %f) {
+; CHECK-LABEL: test_builtin_ppc_test_data_class_f:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:xststdcsp 0, 1, 127
+; CHECK-NEXT:li 3, 0
+; CHECK-NEXT:li 4, 1
+; CHECK-NEXT:iseleq 3, 4, 3
+; CHECK-NEXT:blr
+entry:
+  %test_data_class = tail call i32 @llvm.ppc.test.data.class.f(float %f, i32 127)
+  ret i32 %test_data_class
+}
+
+declare i32 @llvm.ppc.test.data.class.f(float, i32 immarg)
Index: llvm/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -10382,6 +10382,50 @@
 }
 return DAG.getMergeValues(RetOps, dl);
   }
+  case Intrinsic::ppc_compare_exp_lt:
+  case Intrinsic::ppc_compare_exp_gt:
+  case Intrinsic::ppc_compare_exp_eq:
+  case Intrinsic::ppc_compare_exp_uo:
+  case Intrinsic::ppc_test_data_class_d:
+  case Intrinsic::ppc_test_data_class_f: {
+unsigned CmprOpc = PPC::XSCMPEXPDP;
+SDValue Op1 = Op.getOperand(1);
+SDValue Op2 = Op.getOperand(2);
+unsigned Pred;
+switch (IntrinsicID) {
+default

[PATCH] D109658: [X86][FP16] Change the order of the operands in complex FMA intrinsics to allow swap between the mul operands.

2021-09-13 Thread Pengfei Wang via Phabricator via cfe-commits
pengfei added a comment.

In D109658#2996767 , @craig.topper 
wrote:

> In D109658#2996714 , @pengfei wrote:
>
>> In D109658#2996412 , @craig.topper 
>> wrote:
>>
>>> Does gcc use the same builtin name? Our general policy is to have the same 
>>> interface as gcc if we have a builtin. So if gcc has these builtins the 
>>> should work the same way.
>>
>> No. We don't sync with GCC on the builtin name during the development. We 
>> had a disscussion and decided to not keep them aligned due to 1) target 
>> specific builtins are compiler private names that no need to keep it 
>> compatible with other compilers; and 2) we already differentiate the target 
>> builtins with GCC long ago on the naming, masking etc. Currently, regardless 
>> the name, GCC uses the same C, A, B order with our existing implementation. 
>> https://gitlab.com/x86-gcc/gcc/-/blob/users/intel/liuhongt/independentfp16_wip/gcc/config/i386/avx512fp16intrin.h#L6672
>
> I thought we were pretty consistent on names with gcc for most of sse and avx 
> and most of avx512. The names aren't completely private occasionally users 
> due try to use them. If we happen to have the same name we should have the 
> same behavior to avoid confusion.

I'm not so optimistic. I had a coarse-grained statistics on the use of x86 
builtins in Clang and GCC. It shows Clang only defines 2/3 of GCC's builtins 
and 1/4 of Clang builtins have different names with GCC's. Command below:

  cat gcc/config/i386/*.h | grep -o "\b__builtin_ia32_\w\+" |sort|uniq|tee 
gcc.txt|wc -l
  2788
  
  ls clang/lib/Headers/*.h |grep -v fp16 |xargs cat |grep -o 
"\b__builtin_ia32_\w\+" |sort|uniq|tee clang.txt|wc -l
  1808
  
  comm -12 gcc.txt clang.txt |wc -l
  1347

Regarding this case, we already have a different name with GCC, I think it 
worthwhile to use a different order for the swapping optimization.
With a bit research on AVX512IFMA, I found:

1. The use of C, A, B order in GCC is not consistent on its AVX512IFMA 
builtins. It supposes GCC should change to A, B, C order if considering 
consistency;
2. We aren't consistent on AVX512IFMA builtins with GCC either due to the use 
of select.

By the way, GCC folks told me GCC has ability to specify arbitrary operands 
that can be commutative. But I found both SDNode and MI only have ability on 
the first 2 operands, which is insufficient for instruction like CFMA. Do you 
know if we have other mechanism for commutable operands?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109658

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


[PATCH] D109688: [AMDGPU] ASan Patches for OpenMP

2021-09-13 Thread Amit Kumar Pandey via Phabricator via cfe-commits
ampandey-AMD created this revision.
Herald added subscribers: kerbowa, guansong, hiraditya, t-tye, tpr, dstuttard, 
yaxunl, mgorny, nhaehnle, jvesely, kzhuravl.
ampandey-AMD requested review of this revision.
Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, sstefan1, 
wdng.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, OpenMP, LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109688

Files:
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  openmp/libomptarget/hostrpc/services/CMakeLists.txt
  openmp/libomptarget/hostrpc/services/hostcall.cpp
  openmp/libomptarget/hostrpc/services/hostrpc_execute_service.c
  openmp/libomptarget/hostrpc/services/hostrpc_internal.h
  openmp/libomptarget/hostrpc/src/hostrpc.cpp
  openmp/libomptarget/hostrpc/src/hostrpc.h
  openmp/libomptarget/src/rtl.cpp

Index: openmp/libomptarget/src/rtl.cpp
===
--- openmp/libomptarget/src/rtl.cpp
+++ openmp/libomptarget/src/rtl.cpp
@@ -306,6 +306,7 @@
 Device.HasPendingGlobals = true;
 for (__tgt_offload_entry *entry = img->EntriesBegin;
  entry != img->EntriesEnd; ++entry) {
+			 entry.size=32;
   if (entry->flags & OMP_DECLARE_TARGET_CTOR) {
 DP("Adding ctor " DPxMOD " to the pending list.\n",
DPxPTR(entry->addr));
Index: openmp/libomptarget/hostrpc/src/hostrpc.h
===
--- openmp/libomptarget/hostrpc/src/hostrpc.h
+++ openmp/libomptarget/hostrpc/src/hostrpc.h
@@ -111,6 +111,8 @@
   HOSTRPC_SERVICE_VARFNDOUBLE,
   HOSTRPC_SERVICE_FPRINTF,
   HOSTRPC_SERVICE_FTNASSIGN,
+  HOSTRPC_SERVICE_DEVMEM,
+  HOSTRPC_SERVICE_SANITIZER,
 };
 typedef enum hostcall_service_id hostcall_service_id_t;
 
Index: openmp/libomptarget/hostrpc/src/hostrpc.cpp
===
--- openmp/libomptarget/hostrpc/src/hostrpc.cpp
+++ openmp/libomptarget/hostrpc/src/hostrpc.cpp
@@ -156,6 +156,22 @@
   return unionarg.dval;
 }
 
+EXTERN void __ockl_sanitizer_report(uint64_t addr, uint64_t pc, uint64_t wgidx,
+uint64_t wgidy, uint64_t wgidz,
+uint64_t wave_id, uint64_t is_read,
+uint64_t access_size) {
+  hostrpc_result_t value =
+  hostrpc_invoke(PACK_VERS(HOSTRPC_SERVICE_SANITIZER), addr, pc, wgidx,
+ wgidy, wgidz, wave_id, is_read, access_size);
+  return (void)value.arg0;
+}
+
+EXTERN uint64_t __hostrpc_devmem_request(uint64_t addr, uint64_t size) {
+  hostrpc_result_t result = hostrpc_invoke(PACK_VERS(HOSTRPC_SERVICE_DEVMEM),
+   addr, size, 0, 0, 0, 0, 0, 0);
+  return (uint64_t)result.arg0;
+}
+
 // -
 //
 // vector_product_zeros: Example stub to demonstrate hostrpc services
Index: openmp/libomptarget/hostrpc/services/hostrpc_internal.h
===
--- openmp/libomptarget/hostrpc/services/hostrpc_internal.h
+++ openmp/libomptarget/hostrpc/services/hostrpc_internal.h
@@ -9,6 +9,25 @@
 extern "C" {
 #endif
 
+// Error codes for hostrpc functions.
+typedef enum hostrpc_status_t {
+  HOSTRPC_SUCCESS = 0,
+  HOSTRPC_STATUS_UNKNOWN = 1,
+  HOSTRPC_STATUS_ERROR = 2,
+  HOSTRPC_STATUS_TERMINATE = 3,
+  HOSTRPC_DATA_USED_ERROR = 4,
+  HOSTRPC_ADDINT_ERROR = 5,
+  HOSTRPC_ADDFLOAT_ERROR = 6,
+  HOSTRPC_ADDSTRING_ERROR = 7,
+  HOSTRPC_UNSUPPORTED_ID_ERROR = 8,
+  HOSTRPC_INVALID_ID_ERROR = 9,
+  HOSTRPC_ERROR_INVALID_REQUEST = 10,
+  HOSTRPC_EXCEED_MAXVARGS_ERROR = 11,
+  HOSTRPC_WRONGVERSION_ERROR = 12,
+  HOSTRPC_OLDHOSTVERSIONMOD_ERROR = 13,
+  HOSTRPC_INVALIDSERVICE_ERROR = 14,
+} hostrpc_status_t;
+
 /** \file Support library for invoking host services from the device.
  *
  *  The hostcall consumer defined here is used by the language runtime
Index: openmp/libomptarget/hostrpc/services/hostrpc_execute_service.c
===
--- openmp/libomptarget/hostrpc/services/hostrpc_execute_service.c
+++ openmp/libomptarget/hostrpc/services/hostrpc_execute_service.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 
+#if 0
 // Error codes for service handler functions used in this file
 // Some error codes may be returned to device stub functions.
 typedef enum hostrpc_status_t {
@@ -59,6 +60,7 @@
   HOSTRPC_OLDHOSTVERSIONMOD_ERROR = 13,
   HOSTRPC_INVALIDSERVICE_ERROR = 14,
 } hostrpc_status_t;
+#endif
 
 // MAXVARGS is more than a static array size.
 // It is for user vargs functions only.
@@ -263,6 +265,7 @@
   payload[1] = (uint64_t)num_zeros;
 }
 
+#if 0
 // FIXME: Clean up this diagnostic and die properly
 static bool hostrpc_version_checked;
 static hostrpc_status_t hostrpc_version_check(unsigned int 

[PATCH] D109461: [clang][darwin] Add support for --emit-static-lib

2021-09-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:748
+
+  // ar tool command "llvm-ar   ".
+  ArgStringList CmdArgs;

libtool ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109461

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


[PATCH] D109658: [X86][FP16] Change the order of the operands in complex FMA intrinsics to allow swap between the mul operands.

2021-09-13 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added a comment.

> Regarding this case, we already have a different name with GCC, I think it 
> worthwhile to use a different order for the swapping optimization.
> With a bit research on AVX512IFMA, I found:
>
> 1. The use of C, A, B order in GCC is not consistent on its AVX512IFMA 
> builtins. It supposes GCC should change to A, B, C order if considering 
> consistency;
> 2. We aren't consistent on AVX512IFMA builtins with GCC either due to the use 
> of select.

Do we have any builtins with the same name as gcc but different 
operands/behaviours? Those are the only ones that I'd be worried about.

> By the way, GCC folks told me GCC has ability to specify arbitrary operands 
> that can be commutative. But I found both SDNode and MI only have ability on 
> the first 2 operands, which is insufficient for instruction like CFMA. Do you 
> know if we have other mechanism for commutable operands?

Doesn't X86InstrInfo::findCommutedOpIndices handle these cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109658

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


[PATCH] D109688: [AMDGPU] ASan Patches for OpenMP

2021-09-13 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert requested changes to this revision.
jdoerfert added a comment.
This revision now requires changes to proceed.

This doesn't seem to be based on LLVM upstream, among other issues. Blocking it 
for now w/o looking at the actual patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109688

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


[PATCH] D109658: [X86][FP16] Change the order of the operands in complex FMA intrinsics to allow swap between the mul operands.

2021-09-13 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D109658#2997395 , @pengfei wrote:

> In D109658#2996767 , @craig.topper 
> wrote:
>
>> In D109658#2996714 , @pengfei 
>> wrote:
>>
>>> In D109658#2996412 , 
>>> @craig.topper wrote:
>>>
 Does gcc use the same builtin name? Our general policy is to have the same 
 interface as gcc if we have a builtin. So if gcc has these builtins the 
 should work the same way.
>>>
>>> No. We don't sync with GCC on the builtin name during the development. We 
>>> had a disscussion and decided to not keep them aligned due to 1) target 
>>> specific builtins are compiler private names that no need to keep it 
>>> compatible with other compilers; and 2) we already differentiate the target 
>>> builtins with GCC long ago on the naming, masking etc. Currently, 
>>> regardless the name, GCC uses the same C, A, B order with our existing 
>>> implementation. 
>>> https://gitlab.com/x86-gcc/gcc/-/blob/users/intel/liuhongt/independentfp16_wip/gcc/config/i386/avx512fp16intrin.h#L6672
>>
>> I thought we were pretty consistent on names with gcc for most of sse and 
>> avx and most of avx512. The names aren't completely private occasionally 
>> users due try to use them. If we happen to have the same name we should have 
>> the same behavior to avoid confusion.
>
> I'm not so optimistic. I had a coarse-grained statistics on the use of x86 
> builtins in Clang and GCC. It shows Clang only defines 2/3 of GCC's builtins 
> and 1/4 of Clang builtins have different names with GCC's. Command below:
>
>   cat gcc/config/i386/*.h | grep -o "\b__builtin_ia32_\w\+" |sort|uniq|tee 
> gcc.txt|wc -l
>   2788
>   
>   ls clang/lib/Headers/*.h |grep -v fp16 |xargs cat |grep -o 
> "\b__builtin_ia32_\w\+" |sort|uniq|tee clang.txt|wc -l
>   1808
>   
>   comm -12 gcc.txt clang.txt |wc -l
>   1347

Not implementing something that gcc does at least gives a compile error if some 
tries to use the gcc name so I’m fine with that.

Using a different name in clang for something gcc also has is kinda silly. But 
I guess we’re worse at this then I thought.

Using the same name and having different behavior should be avoided if the it 
won’t give a compile error.

> Regarding this case, we already have a different name with GCC, I think it 
> worthwhile to use a different order for the swapping optimization.
> With a bit research on AVX512IFMA, I found:
>
> 1. The use of C, A, B order in GCC is not consistent on its AVX512IFMA 
> builtins. It supposes GCC should change to A, B, C order if considering 
> consistency;
> 2. We aren't consistent on AVX512IFMA builtins with GCC either due to the use 
> of select.
>
> By the way, GCC folks told me GCC has ability to specify arbitrary operands 
> that can be commutative. But I found both SDNode and MI only have ability on 
> the first 2 operands, which is insufficient for instruction like CFMA. Do you 
> know if we have other mechanism for commutable operands?

That’s true for SDNode, but you can always manually add more isel patterns. We 
do this for FMA3.

MI uses two target specific hooks in X86InstrInfo.cpp. findCommutedOpIndices 
and commuteInstructionImpl are the names if I remember right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109658

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


[PATCH] D109344: [AMDGPU][OpenMP] Use complex definitions from complex_cmath.h

2021-09-13 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

Let's change to the uglier #ifdef for now, and add it to the short list of 
things that aren't quite right in declare variant


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109344

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


[PATCH] D109607: [X86] Refactor GetSSETypeAtOffset to fix pr51813

2021-09-13 Thread Pengfei Wang via Phabricator via cfe-commits
pengfei updated this revision to Diff 372254.
pengfei marked an inline comment as done.
pengfei added a comment.

Add more comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109607

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/X86/avx512fp16-abi.c

Index: clang/test/CodeGen/X86/avx512fp16-abi.c
===
--- clang/test/CodeGen/X86/avx512fp16-abi.c
+++ clang/test/CodeGen/X86/avx512fp16-abi.c
@@ -1,11 +1,12 @@
-// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm  -target-feature +avx512fp16 < %s | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm  -target-feature +avx512fp16 < %s | FileCheck %s --check-prefixes=CHECK,CHECK-C
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm  -target-feature +avx512fp16 -x c++ -std=c++11 < %s | FileCheck %s --check-prefixes=CHECK,CHECK-CPP
 
 struct half1 {
   _Float16 a;
 };
 
 struct half1 h1(_Float16 a) {
-  // CHECK: define{{.*}}half @h1
+  // CHECK: define{{.*}}half @
   struct half1 x;
   x.a = a;
   return x;
@@ -17,7 +18,7 @@
 };
 
 struct half2 h2(_Float16 a, _Float16 b) {
-  // CHECK: define{{.*}}<2 x half> @h2
+  // CHECK: define{{.*}}<2 x half> @
   struct half2 x;
   x.a = a;
   x.b = b;
@@ -31,7 +32,7 @@
 };
 
 struct half3 h3(_Float16 a, _Float16 b, _Float16 c) {
-  // CHECK: define{{.*}}<4 x half> @h3
+  // CHECK: define{{.*}}<4 x half> @
   struct half3 x;
   x.a = a;
   x.b = b;
@@ -47,7 +48,7 @@
 };
 
 struct half4 h4(_Float16 a, _Float16 b, _Float16 c, _Float16 d) {
-  // CHECK: define{{.*}}<4 x half> @h4
+  // CHECK: define{{.*}}<4 x half> @
   struct half4 x;
   x.a = a;
   x.b = b;
@@ -62,7 +63,7 @@
 };
 
 struct floathalf fh(float a, _Float16 b) {
-  // CHECK: define{{.*}}<4 x half> @fh
+  // CHECK: define{{.*}}<4 x half> @
   struct floathalf x;
   x.a = a;
   x.b = b;
@@ -76,7 +77,7 @@
 };
 
 struct floathalf2 fh2(float a, _Float16 b, _Float16 c) {
-  // CHECK: define{{.*}}<4 x half> @fh2
+  // CHECK: define{{.*}}<4 x half> @
   struct floathalf2 x;
   x.a = a;
   x.b = b;
@@ -90,7 +91,7 @@
 };
 
 struct halffloat hf(_Float16 a, float b) {
-  // CHECK: define{{.*}}<4 x half> @hf
+  // CHECK: define{{.*}}<4 x half> @
   struct halffloat x;
   x.a = a;
   x.b = b;
@@ -104,7 +105,7 @@
 };
 
 struct half2float h2f(_Float16 a, _Float16 b, float c) {
-  // CHECK: define{{.*}}<4 x half> @h2f
+  // CHECK: define{{.*}}<4 x half> @
   struct half2float x;
   x.a = a;
   x.b = b;
@@ -120,7 +121,7 @@
 };
 
 struct floathalf3 fh3(float a, _Float16 b, _Float16 c, _Float16 d) {
-  // CHECK: define{{.*}}{ <4 x half>, half } @fh3
+  // CHECK: define{{.*}}{ <4 x half>, half } @
   struct floathalf3 x;
   x.a = a;
   x.b = b;
@@ -138,7 +139,7 @@
 };
 
 struct half5 h5(_Float16 a, _Float16 b, _Float16 c, _Float16 d, _Float16 e) {
-  // CHECK: define{{.*}}{ <4 x half>, half } @h5
+  // CHECK: define{{.*}}{ <4 x half>, half } @
   struct half5 x;
   x.a = a;
   x.b = b;
@@ -147,3 +148,27 @@
   x.e = e;
   return x;
 }
+
+struct float2 {
+  struct {} s;
+  float a;
+  float b;
+};
+
+float pr51813(struct float2 s) {
+  // CHECK-C: define{{.*}} @pr51813(<2 x float>
+  // CHECK-CPP: define{{.*}} @_Z7pr518136float2(double {{.*}}, float
+  return s.a;
+}
+
+struct float3 {
+  float a;
+  struct {} s;
+  float b;
+};
+
+float pr51813_2(struct float3 s) {
+  // CHECK-C: define{{.*}} @pr51813_2(<2 x float>
+  // CHECK-CPP: define{{.*}} @_Z9pr51813_26float3(double {{.*}}, float
+  return s.a;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -3377,52 +3377,18 @@
   return false;
 }
 
-/// ContainsFloatAtOffset - Return true if the specified LLVM IR type has a
-/// float member at the specified offset.  For example, {int,{float}} has a
-/// float at offset 4.  It is conservatively correct for this routine to return
-/// false.
-static bool ContainsFloatAtOffset(llvm::Type *IRType, unsigned IROffset,
-  const llvm::DataLayout &TD) {
-  // Base case if we find a float.
-  if (IROffset == 0 && IRType->isFloatTy())
-return true;
-
-  // If this is a struct, recurse into the field at the specified offset.
-  if (llvm::StructType *STy = dyn_cast(IRType)) {
-const llvm::StructLayout *SL = TD.getStructLayout(STy);
-unsigned Elt = SL->getElementContainingOffset(IROffset);
-IROffset -= SL->getElementOffset(Elt);
-return ContainsFloatAtOffset(STy->getElementType(Elt), IROffset, TD);
-  }
-
-  // If this is an array, recurse into the field at the specified offset.
-  if (llvm::ArrayType *ATy = dyn_cast(IRType)) {
-llvm::Type *EltTy = ATy->getElementType();
-unsigned EltSize = TD.getTypeAllocSize(EltTy);
-IROffset -= IROffset/EltSize*EltSize;
-return ContainsFloatAtOffset(EltTy, IROffset,

[PATCH] D109607: [X86] Refactor GetSSETypeAtOffset to fix pr51813

2021-09-13 Thread Pengfei Wang via Phabricator via cfe-commits
pengfei added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:3454
-
-return llvm::Type::getHalfTy(getVMContext());
-  }

LuoYuanke wrote:
> Is this the major change?
No, this logic is not changed. When `T1` is nullptr, it means `IRType` is 
simply a `half` or `float`. So we return `T0` that equals to the code here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109607

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


[PATCH] D109344: [AMDGPU][OpenMP] Use complex definitions from complex_cmath.h

2021-09-13 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D109344#2997454 , @JonChesterfield 
wrote:

> Let's change to the uglier #ifdef for now, and add it to the short list of 
> things that aren't quite right in declare variant

Can you please file a bug with a reproducer, I'm not sure I follow your problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109344

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


[PATCH] D97699: [analyzer] Add InvalidPtrChecker

2021-09-13 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

Excellent! Thanks!


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

https://reviews.llvm.org/D97699

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


[PATCH] D109693: Driver: Remove major release version detection for RHEL

2021-09-13 Thread Tom Stellard via Phabricator via cfe-commits
tstellar created this revision.
tstellar added a reviewer: serge-sans-paille.
tstellar requested review of this revision.
Herald added a project: clang.

We treat all release of RHEL the same in the driver, so we don't need to
detect the RHEL major release version.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109693

Files:
  clang/include/clang/Driver/Distro.h
  clang/lib/Driver/Distro.cpp
  clang/unittests/Driver/DistroTest.cpp


Index: clang/unittests/Driver/DistroTest.cpp
===
--- clang/unittests/Driver/DistroTest.cpp
+++ clang/unittests/Driver/DistroTest.cpp
@@ -152,7 +152,7 @@

"REDHAT_SUPPORT_PRODUCT_VERSION=\"7\"\n"));
 
   Distro CentOS7{CentOS7FileSystem, llvm::Triple("unknown-pc-linux")};
-  ASSERT_EQ(Distro(Distro::RHEL7), CentOS7);
+  ASSERT_EQ(Distro(Distro::RHEL), CentOS7);
   ASSERT_FALSE(CentOS7.IsUbuntu());
   ASSERT_TRUE(CentOS7.IsRedhat());
   ASSERT_FALSE(CentOS7.IsOpenSUSE());
Index: clang/lib/Driver/Distro.cpp
===
--- clang/lib/Driver/Distro.cpp
+++ clang/lib/Driver/Distro.cpp
@@ -118,12 +118,7 @@
   return Distro::Fedora;
 if (Data.startswith("Red Hat Enterprise Linux") ||
 Data.startswith("CentOS") || Data.startswith("Scientific Linux")) {
-  if (Data.find("release 7") != StringRef::npos)
-return Distro::RHEL7;
-  else if (Data.find("release 6") != StringRef::npos)
-return Distro::RHEL6;
-  else if (Data.find("release 5") != StringRef::npos)
-return Distro::RHEL5;
+return Distro::RHEL;
 }
 return Distro::UnknownDistro;
   }
Index: clang/include/clang/Driver/Distro.h
===
--- clang/include/clang/Driver/Distro.h
+++ clang/include/clang/Driver/Distro.h
@@ -39,9 +39,7 @@
 DebianBullseye,
 DebianBookworm,
 Exherbo,
-RHEL5,
-RHEL6,
-RHEL7,
+RHEL,
 Fedora,
 Gentoo,
 OpenSUSE,
@@ -114,7 +112,7 @@
   /// @{
 
   bool IsRedhat() const {
-return DistroVal == Fedora || (DistroVal >= RHEL5 && DistroVal <= RHEL7);
+return DistroVal == Fedora || DistroVal == RHEL;
   }
 
   bool IsOpenSUSE() const { return DistroVal == OpenSUSE; }


Index: clang/unittests/Driver/DistroTest.cpp
===
--- clang/unittests/Driver/DistroTest.cpp
+++ clang/unittests/Driver/DistroTest.cpp
@@ -152,7 +152,7 @@
"REDHAT_SUPPORT_PRODUCT_VERSION=\"7\"\n"));
 
   Distro CentOS7{CentOS7FileSystem, llvm::Triple("unknown-pc-linux")};
-  ASSERT_EQ(Distro(Distro::RHEL7), CentOS7);
+  ASSERT_EQ(Distro(Distro::RHEL), CentOS7);
   ASSERT_FALSE(CentOS7.IsUbuntu());
   ASSERT_TRUE(CentOS7.IsRedhat());
   ASSERT_FALSE(CentOS7.IsOpenSUSE());
Index: clang/lib/Driver/Distro.cpp
===
--- clang/lib/Driver/Distro.cpp
+++ clang/lib/Driver/Distro.cpp
@@ -118,12 +118,7 @@
   return Distro::Fedora;
 if (Data.startswith("Red Hat Enterprise Linux") ||
 Data.startswith("CentOS") || Data.startswith("Scientific Linux")) {
-  if (Data.find("release 7") != StringRef::npos)
-return Distro::RHEL7;
-  else if (Data.find("release 6") != StringRef::npos)
-return Distro::RHEL6;
-  else if (Data.find("release 5") != StringRef::npos)
-return Distro::RHEL5;
+return Distro::RHEL;
 }
 return Distro::UnknownDistro;
   }
Index: clang/include/clang/Driver/Distro.h
===
--- clang/include/clang/Driver/Distro.h
+++ clang/include/clang/Driver/Distro.h
@@ -39,9 +39,7 @@
 DebianBullseye,
 DebianBookworm,
 Exherbo,
-RHEL5,
-RHEL6,
-RHEL7,
+RHEL,
 Fedora,
 Gentoo,
 OpenSUSE,
@@ -114,7 +112,7 @@
   /// @{
 
   bool IsRedhat() const {
-return DistroVal == Fedora || (DistroVal >= RHEL5 && DistroVal <= RHEL7);
+return DistroVal == Fedora || DistroVal == RHEL;
   }
 
   bool IsOpenSUSE() const { return DistroVal == OpenSUSE; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D109694: Driver: Add preferred gcc triples for Red Hat operating systems

2021-09-13 Thread Tom Stellard via Phabricator via cfe-commits
tstellar created this revision.
tstellar added reviewers: serge-sans-paille, MaskRay, mgorny.
Herald added a subscriber: pengfei.
tstellar requested review of this revision.
Herald added a project: clang.

The GCCInstallationDetector has a list of gcc triples to search
for when trying to find a valid gcc install on the system.  When
all else is equal, clang selects the first triple that appears in
the list.

This causes a problem on Red Hat operating systems, since there
is a cross-compiler gcc package that uses the triple x86_64-linux-gnu
and since this is listed in the triple list before the system gcc
install (x86_64-redhat-linux), the cross-compiler is always selected
by clang.

To solve this, we make it possible for a distro to provide a list of
preferred triples that will be selected before any triples in the
generic triple list.

Another solution to this problem would be to reorder the generic triple
lists so that distro supplied triples come first.  However, I don't
have confidence that I would be able to order the list so that it
is correct for every distro.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109694

Files:
  clang/include/clang/Driver/Distro.h
  clang/lib/Driver/Distro.cpp
  clang/lib/Driver/ToolChains/Linux.cpp


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -177,7 +177,9 @@
 
 Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
 : Generic_ELF(D, Triple, Args) {
-  GCCInstallation.init(Triple, Args);
+
+  Distro Distro(D.getVFS(), Triple);
+  GCCInstallation.init(Triple, Args, 
Distro.getDefaultGCCTriples(Triple.getArch()));
   Multilibs = GCCInstallation.getMultilibs();
   SelectedMultilib = GCCInstallation.getMultilib();
   llvm::Triple::ArchType Arch = Triple.getArch();
@@ -186,7 +188,6 @@
 
   Generic_GCC::PushPPaths(PPaths);
 
-  Distro Distro(D.getVFS(), Triple);
 
   if (Distro.IsAlpineLinux() || Triple.isAndroid()) {
 ExtraOpts.push_back("-z");
Index: clang/lib/Driver/Distro.cpp
===
--- clang/lib/Driver/Distro.cpp
+++ clang/lib/Driver/Distro.cpp
@@ -225,3 +225,53 @@
 
 Distro::Distro(llvm::vfs::FileSystem &VFS, const llvm::Triple &TargetOrHost)
 : DistroVal(GetDistro(VFS, TargetOrHost)) {}
+
+/// @return An array of triples that should be used first when searching for
+/// a candidate gcc install on the system.
+llvm::ArrayRef 
Distro::getDefaultGCCTriples(llvm::Triple::ArchType Arch) {
+  if (!IsRedhat())
+return None;
+
+  static const std::array RedHatAArch64Triples = {{
+"aarch64-redhat-linux"
+  }};
+
+  static const std::array RedHatArmTriples = {{
+"armv7hl-redhat-linux-gnueabi"
+  }};
+
+  static const std::array RedHatPPC64LETriples = {{
+"ppc64le-redhat-linux"
+  }};
+
+  static const std::array RedHatSystemZTriples = {{
+"s390x-redhat-linux"
+  }};
+
+  static const std::array RedHatX86Triples = {{
+"i686-redhat-linux"
+  }};
+
+  static const std::array RedHatX86_64Triples = {{
+"x86_64-redhat-linux"
+  }};
+
+  switch (Arch) {
+  default:
+break;
+  case llvm::Triple::aarch64:
+return RedHatAArch64Triples;
+  case llvm::Triple::arm:
+return RedHatArmTriples;
+  case llvm::Triple::ppc64le:
+return  RedHatPPC64LETriples;
+  case llvm::Triple::systemz:
+return RedHatSystemZTriples;
+  case llvm::Triple::x86:
+return RedHatX86Triples;
+  case llvm::Triple::x86_64:
+return RedHatX86_64Triples;
+  }
+
+  return None;
+}
Index: clang/include/clang/Driver/Distro.h
===
--- clang/include/clang/Driver/Distro.h
+++ clang/include/clang/Driver/Distro.h
@@ -91,6 +91,8 @@
   /// Detects the distribution using specified VFS.
   explicit Distro(llvm::vfs::FileSystem &VFS, const llvm::Triple 
&TargetOrHost);
 
+  llvm::ArrayRef getDefaultGCCTriples(llvm::Triple::ArchType 
Arch);
+
   bool operator==(const Distro &Other) const {
 return DistroVal == Other.DistroVal;
   }


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -177,7 +177,9 @@
 
 Linux::Linux(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
 : Generic_ELF(D, Triple, Args) {
-  GCCInstallation.init(Triple, Args);
+
+  Distro Distro(D.getVFS(), Triple);
+  GCCInstallation.init(Triple, Args, Distro.getDefaultGCCTriples(Triple.getArch()));
   Multilibs = GCCInstallation.getMultilibs();
   SelectedMultilib = GCCInstallation.getMultilib();
   llvm::Triple::ArchType Arch = Triple.getArch();
@@ -186,7 +188,6 @@
 
   Generic_GCC::PushPPaths(PPaths);
 
-  Distro Distro(D.getVFS(), Triple);
 
   if (Distro.IsAlpineLinux() || Triple.isAndroid()) {
 ExtraOpts.push_back

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

2021-09-13 Thread David Goldman via Phabricator via cfe-commits
dgoldman marked an inline comment as done.
dgoldman added a comment.

Friendly ping, PTAL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108556

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


[PATCH] D109128: [VFS] Use original path when falling back to external FS

2021-09-13 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere added a comment.

Keith and I discussed this offline. My suggestion was to do the following:

1. Check the overlay for the canonicalized path
2. Check the fall-through for the canonicalized path
3. Check the fall-through for the original path

If I understand correctly, this patch does that, but swaps 2 and 3. What's the 
motivation for trying the non-canonical path first? If we treat the current 
working directory as a property directory (which is what the canonicalization 
is all about), then it seems like we should exhaust those options first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128

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


[PATCH] D109608: [clang][ASTImporter] Generic attribute import handling (first step).

2021-09-13 Thread Gabor Marton via Phabricator via cfe-commits
martong added a reviewer: teemperor.
martong added a comment.

Thanks, nice work! I like the direction, however, I'd like to ask comments from 
@teemperor .




Comment at: clang/lib/AST/ASTImporter.cpp:656-671
+// Helper for chaining together multiple imports. If an error is detected,
+// subsequent imports will return default constructed nodes, so that 
failure
+// can be detected with a single conditional branch after a sequence of
+// imports.
+template  T importChecked(Error &Err, const T &From) {
+  // Don't attempt to import nodes if we hit an error earlier.
+  if (Err)

What's the reason of moving this hunk?



Comment at: clang/lib/AST/ASTImporter.cpp:8473-8474
+
+ToAttrName = Importer.Import(FromAttr->getAttrName());
+ToScopeName = Importer.Import(FromAttr->getScopeName());
+ToAttrRange = NImporter.importChecked(Err, FromAttr->getRange());

Why can't we use `importChecked` here?



Comment at: clang/lib/AST/ASTImporter.cpp:8485-8486
+T *ToAttr = T::Create(Importer.getToContext(), ImportedArg..., ToI);
+// T *ToAttr = new (Importer.getToContext()) T(Importer.getToContext(), 
ToI,
+// ImportedArg...);
+

Could you please remove this comment?



Comment at: clang/lib/AST/ASTImporter.cpp:8547-8548
+Expected ToAttrOrErr = AI.createImportedAttr(
+From, AI.importArrayArg(From->args(), From->args_size()).value(),
+From->args_size());
+if (ToAttrOrErr)

Could we hide these arguments?
I mean we probably need a higher abstraction, something like
```
Expected ToAttrOrErr = AI.createImportedAttr(From);
```
should be sufficient, isn't it. We do want to import all arguments of all kind 
of attributes, don't we?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109608

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


[PATCH] D109344: [AMDGPU][OpenMP] Use complex definitions from complex_cmath.h

2021-09-13 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added a comment.

In D109344#2997495 , @jdoerfert wrote:

> Can you please file a bug with a reproducer, I'm not sure I follow your 
> problem.

Should be a case of trying to compile complex test cases for ovo for amdgpu 
immediately crashes in the name mangler, I'll see if I can check that's still 
the case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109344

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


[PATCH] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

2021-09-13 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment.

Probably it is better to make a big comment at the start of the file that 
explains how the checker works, like in FuchsiaHandleChecker. This comes in a 
separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106262

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


[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D69764#2996365 , @steveire wrote:

> FYI - there's nothing "anti-inclusive" about East/West.

While I'm certain that there can be confusion around why terms are or are not 
troublesome to some people and there needs to be space for those discussions, 
I'm also certain that telling someone that there is "nothing" to their concerns 
is not a good way to reach an understanding. Let's please be a bit more 
respectful when discussing concerns about using inclusive terminology 
(https://llvm.org/docs/CodeOfConduct.html#when-we-disagree-try-to-understand-why).


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

https://reviews.llvm.org/D69764

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


[PATCH] D109642: -Wunused-but-set-parameter/-Wunused-but-set-variable Add to the release notes

2021-09-13 Thread Michael Benfield via Phabricator via cfe-commits
mbenfield added a comment.

I'd request small changes for grammar:

`Wunused-but-set-parameter` and `-Wunused-but-set-variable` emit warnings
when a parameter or a variable is set but not used.

Otherwise LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109642

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


[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Philippe Antoine via Phabricator via cfe-commits
catenacyber added a comment.

This commit has broken oss-fuzz workflow for rust projects.
I do not know if the fix is 
https://reviews.llvm.org/rG66e2772e4285588ccc3bcdb5f392c8326debbd7d

Scenario is

  export RUSTFLAGS="--cfg fuzzing -Cdebuginfo=1 -Cforce-frame-pointers 
-Zinstrument-coverage -C link-dead-code -C link-arg=-lc++"
  git clone https://github.com/ctz/rustls && cd rustls
  cargo fuzz build -O
  ./fuzz/target/x86_64-unknown-linux-gnu/release/client 
./fuzz/target/x86_64-unknown-linux-gnu/release/client
  llvm-profdata merge -sparse default.profraw -o profdata

Current output is

  warning: default.profraw: malformed instrumentation profile data
  error: no profile can be merged

Scenario does not output error for in 13.0.0-rc2

Error does not happen on commit 69cdadddecaf97f572c311aa07044f79a5b8329a 
 just 
previous this integration, if we add `git cherry-pick 
a6c14fba70e170a279f7e77f068368f09d8c5eaf` to fix the other pending bug which 
was fixed in 13.0.0-rc2
Error happens on commit a1532ed27582038e2d9588108ba0fe8237f01844 
 even if 
we `git cherry-pick a6c14fba70e170a279f7e77f068368f09d8c5eaf` and fix the 
conflict about only llvm/test/tools/llvm-profdata/Inputs/c-general.profraw


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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


[PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.

2021-09-13 Thread Chris Lattner via Phabricator via cfe-commits
lattner added a comment.

> What is a "keep constructor"?

Good question, I'm not sure.  I think I meant to say "key constructors".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109483

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


[PATCH] D109531: [CSSPGO] Enable pseudo probe instrumentation in O0 mode.

2021-09-13 Thread Hongtao Yu via Phabricator via cfe-commits
hoy added inline comments.



Comment at: llvm/lib/Passes/PassBuilder.cpp:1930
+  // mixed with an O0 prelink and an O2 postlink. Loading a sample profile in
+  // the postlink will require pseudo probe instrumentation in the prelink.
+  if (PGOOpt && PGOOpt->PseudoProbeForProfiling)

wenlei wrote:
> > Loading a sample profile in the postlink will require pseudo probe 
> > instrumentation in the prelink. 
> 
> Even with this change, is it still possible that prelink compile for some 
> module actually doesn't have `-fpseudo-probe-for-profiling`, and it's on for 
> LTO postlink? We could contrive such case, and it could happen in reality 
> too, right? Would we have the same problem when trying to load profile for 
> functions from modules without pseudo-probe in prelink? 
Yes, it could happen. The compiler will stop working too with the current 
implementation. We could change the error reporting to be a warning to make 
that pass. It is an error because we want to remind user if that's intentional.

I think it's mostly user's responsibility to be clear if pseudo probe 
instrumentation is needed or not, especially when passing linker flags 
separately. The change being made here is to ensure it works when all flags are 
passed via CXX_FLAGS, such as

clang -flto 1.cpp -v -fuse-ld=lld -fpseudo-probe-for-profiling 
-fprofile-sample-use=


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109531

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


[PATCH] D97699: [analyzer] Add InvalidPtrChecker

2021-09-13 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt:42
   DynamicTypeChecker.cpp
+  cert/InvalidPtrChecker.cpp
   EnumCastOutOfRangeChecker.cpp

Please, insert this in its sorted place.


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

https://reviews.llvm.org/D97699

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


[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

2021-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D104556#2997674 , @catenacyber 
wrote:

> This commit has broken oss-fuzz workflow for rust projects.
> I do not know if the fix is 
> https://reviews.llvm.org/rG66e2772e4285588ccc3bcdb5f392c8326debbd7d
>
> Scenario is
>
>   export RUSTFLAGS="--cfg fuzzing -Cdebuginfo=1 -Cforce-frame-pointers 
> -Zinstrument-coverage -C link-dead-code -C link-arg=-lc++"
>   git clone https://github.com/ctz/rustls && cd rustls
>   cargo fuzz build -O
>   ./fuzz/target/x86_64-unknown-linux-gnu/release/client 
> ./fuzz/target/x86_64-unknown-linux-gnu/release/client
>   llvm-profdata merge -sparse default.profraw -o profdata
>
> Current output is
>
>   warning: default.profraw: malformed instrumentation profile data
>   error: no profile can be merged
>
> Scenario does not output error for in 13.0.0-rc2
>
> Error does not happen on commit 69cdadddecaf97f572c311aa07044f79a5b8329a 
>  just 
> previous this integration, if we add `git cherry-pick 
> a6c14fba70e170a279f7e77f068368f09d8c5eaf` to fix the other pending bug which 
> was fixed in 13.0.0-rc2
> Error happens on commit a1532ed27582038e2d9588108ba0fe8237f01844 
>  even if 
> we `git cherry-pick a6c14fba70e170a279f7e77f068368f09d8c5eaf` and fix the 
> conflict about only llvm/test/tools/llvm-profdata/Inputs/c-general.profraw

I don't think this change is responsible. This has been used by many C++ 
downstream groups.

Rust should use upgrade its lib/ProfileData and compiler-rt/lib/profile.
The binary ID change has caused a bit of churn to ProfileData but it is 
unrelated to this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104556

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


[PATCH] D109694: Driver: Add preferred gcc triples for Red Hat operating systems

2021-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Haven't looked closely at this particular commit, but I think hard coding the 
detected triples is almost always a bad idea.

See https://reviews.llvm.org/D63497#2993662 for a longer comment.

The proper way is (1) making sure CMake `LLVM_DEFAULT_TARGET_TRIPLE` is correct 
at configure time (2) the user should specify the complete --target=.

Inferring random target triple from an incomplete `--target=x86_64` really 
should not be supported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109694

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


[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

2021-09-13 Thread Vaibhav Yenamandra via Phabricator via cfe-commits
vaibhav.y created this revision.
vaibhav.y added a reviewer: lattner.
Herald added subscribers: dexonsmith, wenlei, mgorny.
vaibhav.y edited the summary of this revision.
vaibhav.y added a project: clang.
vaibhav.y added subscribers: aaron.ballman, lebedev.ri.
vaibhav.y added a comment.
vaibhav.y edited the summary of this revision.
vaibhav.y edited the summary of this revision.
vaibhav.y edited the summary of this revision.
vaibhav.y published this revision for review.
Herald added a subscriber: cfe-commits.

I've added Roman Lebedev and Aaron Ballman as subscribers from the original RFC 
thread




Comment at: clang/lib/Basic/Sarif.cpp:27
+
+StringRef getFileName(const FileEntry &FE) {
+  StringRef Filename = FE.tryGetRealPathName();

A lot of these are copied from [[ 
https://github.com/llvm/llvm-project/blob/181d18ef53db1e5810bf6b905fbafc91da9b5baa/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp#L64
 | SarifDiagnostics.cpp ]]


Create an interface for writing SARIF documents from within `clang`:

The primary intent of this change is to introduce the interface 
`clang::SarifDocumentWriter`, which allows incrementally adding diagnostic data 
to a JSON backed document. The proposed interface is not yet connected to the 
compiler internals, which will be covered in future work. As such this change 
will not change the input/output interface of `clang`.

This change also introduces the `clang::FullSourceRange` type that is modeled 
after `clang::SourceRange` + `clang::FullSourceLoc`, this is useful for 
packaging a pair of `clang::SourceLocation` objects with their corresponding 
`SourceManager`s.

Previous discussions:

- RFC for this change: 
https://lists.llvm.org/pipermail/cfe-dev/2021-March/067907.html
- https://lists.llvm.org/pipermail/cfe-dev/2021-July/068480.html

SARIF Standard (2.1.0):

- https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109701

Files:
  clang/include/clang/Basic/Sarif.h
  clang/include/clang/Basic/SourceLocation.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Sarif.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/unittests/Basic/CMakeLists.txt
  clang/unittests/Basic/SarifTest.cpp

Index: clang/unittests/Basic/SarifTest.cpp
===
--- /dev/null
+++ clang/unittests/Basic/SarifTest.cpp
@@ -0,0 +1,153 @@
+//===- unittests/Basic/SarifTest.cpp - Test writing SARIF documents ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Basic/Sarif.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+#include 
+
+#include "gmock/gmock.h"
+#include "gtest/gtest-death-test.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace llvm;
+
+namespace {
+
+TEST(SarifDocumentWriterTest, createEmptyDocument) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+
+  // WHEN:
+  const json::Object &emptyDocument = writer.createDocument();
+  std::vector keys(emptyDocument.size());
+  std::transform(emptyDocument.begin(), emptyDocument.end(), keys.begin(),
+ [](auto item) { return item.getFirst(); });
+
+  // THEN:
+  ASSERT_THAT(keys, testing::UnorderedElementsAre("$schema", "version"));
+}
+
+// Test that a newly inserted run will associate correct tool names
+TEST(SarifDocumentWriterTest, documentWithARun) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+  const char *shortName = "sariftest";
+  const char *longName = "sarif writer test";
+
+  // WHEN:
+  writer.createRun(shortName, longName);
+  writer.endRun();
+  const json::Object &document = writer.createDocument();
+  const json::Array *runs = document.getArray("runs");
+
+  // THEN:
+  // A run was created
+  ASSERT_THAT(runs, testing::NotNull());
+
+  // It is the only run
+  ASSERT_EQ(runs->size(), 1UL);
+
+  // The tool associated with the run was the tool
+  const json::Object *driver =
+  runs->begin()->getAsObject()->getObject("tool")->getObject("driver");
+  ASSERT_THAT(driver, testing::NotNull());
+
+  ASSERT_TRUE(driver->getString("name").hasValue());
+  ASSERT_TRUE(driver->getString("fullName").hasValue());
+  ASSERT_TRUE(driver->getString("language").hasValue());
+
+  EXPECT_EQ(driver->getString("name").getValue(), shortName);
+  EXPECT_EQ(driver->getString("fullName").getValue(), longName);
+  EXPECT_EQ(driver->getString("language").getValue(), "en-US");
+}
+
+// Test adding result without a run causes a crash
+TEST(SarifDocumentWriterTest, addingResultsWillCrashIfThereIsNoRun) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+  SarifResult &&emptyResult = SarifResult::create();
+
+  // WHEN:
+  //  A SarifDocumentWriter::createRun(...) was no

[PATCH] D109128: [VFS] Use original path when falling back to external FS

2021-09-13 Thread Keith Smiley via Phabricator via cfe-commits
keith added a comment.

In D109128#2997588 , @JDevlieghere 
wrote:

> If I understand correctly, this patch does that, but swaps 2 and 3. What's 
> the motivation for trying the non-canonical path first? If we treat the 
> current working directory as a property directory (which is what the 
> canonicalization is all about), then it seems like we should exhaust those 
> options first.

Using the canonical path first is the root of the problem, using that first the 
behavior to return the absolute path virtually always, for example in my new 
test these assertions

  EXPECT_EQ("a", Name.get());
  EXPECT_EQ("a", OpenedS->getName());

fail and would have to be converted to:

  EXPECT_EQ("//root/foo/a", Name.get());
  EXPECT_EQ("//root/foo/a", OpenedS->getName());


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128

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


[PATCH] D109703: [DebugInfo] Fix scope for local static variables

2021-09-13 Thread Kristina Bessonova via Phabricator via cfe-commits
krisb created this revision.
krisb added reviewers: dblaikie, probinson.
Herald added a subscriber: hiraditya.
krisb requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This fixes https://bugs.llvm.org/show_bug.cgi?id=44695 (and likely
https://bugs.llvm.org/show_bug.cgi?id=30637).

On clang side we need to place static locals defined within a bracketed
block into a correct scope. Since getContextDescriptor() does no access lexical
blocks, just pick the innermost lexical scope early.

On llvm side the patch proposes to delay emission of static locals until their
parent scopes are created (just like normal local variables).
In case of inlined function, static locals are created only for abstract
entities (as per suggestions from https://bugs.llvm.org/show_bug.cgi?id=30637).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109703

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-static.c
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/test/DebugInfo/Generic/static-locals.ll
  llvm/test/DebugInfo/X86/gnu-public-names.ll

Index: llvm/test/DebugInfo/X86/gnu-public-names.ll
===
--- llvm/test/DebugInfo/X86/gnu-public-names.ll
+++ llvm/test/DebugInfo/X86/gnu-public-names.ll
@@ -137,19 +137,6 @@
 ; CHECK-NOT: {{DW_TAG|NULL}}
 ; CHECK: DW_AT_name {{.*}} "global_namespace_function"
 
-; CHECK: DW_TAG_subprogram
-; CHECK-NOT: {{DW_TAG|NULL}}
-; CHECK:   DW_AT_name {{.*}} "f3"
-; CHECK-NOT: {{DW_TAG|NULL}}
-; CHECK: [[F3_Z:.*]]:   DW_TAG_variable
-; CHECK-NOT: DW_TAG
-; CHECK: DW_AT_name {{.*}} "z"
-; CHECK-NOT: {{DW_TAG|NULL}}
-; CHECK: DW_AT_location
-; CHECK-NOT: {{DW_TAG|NULL}}
-; CHECK:   NULL
-; CHECK-NOT: {{DW_TAG|NULL}}
-
 ; CHECK: [[ANON:.*]]: DW_TAG_namespace
 ; CHECK-NOT:   DW_AT_name
 ; CHECK: [[ANON_I:.*]]: DW_TAG_variable
@@ -237,6 +224,19 @@
 ; CHECK: DW_AT_linkage_name
 ; CHECK-NOT: {{DW_TAG|NULL}}
 ; CHECK: DW_AT_name {{.*}} "global_function"
+; CHECK-NOT: {{DW_TAG|NULL}}
+
+; CHECK: DW_TAG_subprogram
+; CHECK-NOT: {{DW_TAG|NULL}}
+; CHECK:   DW_AT_name {{.*}} "f3"
+; CHECK-NOT: {{DW_TAG|NULL}}
+; CHECK: [[F3_Z:.*]]:   DW_TAG_variable
+; CHECK-NOT: DW_TAG
+; CHECK: DW_AT_name {{.*}} "z"
+; CHECK-NOT: {{DW_TAG|NULL}}
+; CHECK: DW_AT_location
+; CHECK-NOT: {{DW_TAG|NULL}}
+; CHECK:   NULL
 
 ; CHECK-LABEL: .debug_gnu_pubnames contents:
 ; CHECK-NEXT: length = {{.*}}, version = 0x0002, unit_offset = 0x, unit_size = {{.*}}
@@ -250,8 +250,8 @@
 ; comes out naturally from LLVM's implementation, so I'm OK with it for now. If
 ; it's demonstrated that this is a major size concern or degrades debug info
 ; consumer behavior, feel free to change it.
-; CHECK-NEXT:  [[F3_Z]] STATIC VARIABLE "f3::z"
 ; CHECK-NEXT:  [[ANON]] EXTERNAL TYPE "(anonymous namespace)"
+; CHECK-NEXT:  [[F3_Z]] STATIC VARIABLE "f3::z"
 ; CHECK-NEXT:  [[OUTER_ANON]] EXTERNAL TYPE "outer::(anonymous namespace)"
 ; CHECK-NEXT:  [[ANON_INNER_B]] STATIC VARIABLE "(anonymous namespace)::inner::b"
 ; CHECK-NEXT:  [[OUTER]] EXTERNAL TYPE "outer"
Index: llvm/test/DebugInfo/Generic/static-locals.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/Generic/static-locals.ll
@@ -0,0 +1,204 @@
+; RUN: %llc_dwarf -O0 -filetype=obj < %s \
+; RUN:   | llvm-dwarfdump -debug-info -  \
+; RUN:   | FileCheck --implicit-check-not "{{DW_TAG|NULL}}" %s
+
+; Generated from:
+
+; static int global = 42;
+;
+; inline __attribute__((always_inline))
+; int inlined() {
+;   static int inlined_a = 1;
+;   if (global > 100) {
+; static int inlined_b = 2;
+; return inlined_b;
+;   }
+;   {
+; static int inlined_c = 3;
+; inlined_a = inlined_c;
+;   }
+;   return inlined_a;
+; }
+;
+; int foo() {
+;   static int a = 1;
+;   if (global < 100) {
+; static int b = 2;
+; return b;
+;   }
+;   {
+; static int c = 3;
+; a = c;
+;   }
+;   return a;
+; }
+;
+; int far() {
+;   return inlined();
+; }
+
+; CHECK: DW_TAG_compile_unit
+; CHECK: DW_TAG_variable
+; CHECK: DW_AT_name	("global")
+; CHECK: DW_TAG_base_type
+
+; foo().
+; CHECK: DW_TAG_subprogram
+; CHECK:   DW_AT_name	("foo")
+; CHECK:   DW_TAG_variable
+; CHECK: DW_AT_name	("a")
+; CHECK:   DW_TAG_lexical_block
+; CHECK: DW_TAG_variable
+; CHECK:   DW_AT_name	("b")
+; CHECK: NULL
+; CHECK:   DW_TAG_lexical_block
+; CHECK: DW_TAG_variable
+; CHECK:   DW_AT_name	("c")
+; CHECK:   NULL
+; CHECK: NULL
+
+; inlined().
+; CHECK: DW_TAG_subprogram
+; CHECK:   DW_AT_name  ("inlined")
+; CHECK:   DW_AT_inline  (DW_INL_inlined)
+; CHECK:   DW_TAG_variable
+; CHECK: DW_AT_name	("inlined_a")
+; CHECK:   DW_TAG_lexical_block
+; CHECK: DW_TAG_variable
+; CHECK:   DW_AT_name	("inlined_b")
+; CHECK: NULL
+; C

[PATCH] D109608: [clang][ASTImporter] Generic attribute import handling (first step).

2021-09-13 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:711
+
+/*template
+friend struct AttrArgImporter;

Looks like left over unused code?



Comment at: clang/lib/AST/ASTImporter.cpp:8448
+
+class AttrImporter {
+  Error Err = Error::success();

@martong the `ASTImporter` file is quite large as it is and I think it makes 
sense to have `AttrImporter` but perhaps it would help long-term 
maintainability to split it out into its own source file? What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109608

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


[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

2021-09-13 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

`Clang :: Driver/cl.c` started failing on our Windows bots after this change 
landed. Here 

 is the error output is, would it be possible to take a look and revert the 
change if needed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109624

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


[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

2021-09-13 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd marked 7 inline comments as done.
compnerd added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:63
+hasType(Container),
+hasType(pointsTo(Container)),
+hasType(references(Container))

aaron.ballman wrote:
> Same question here about `pointsTo()`.
This is actually meant to catch cases such as references to UnOp(DRE).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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


[PATCH] D109461: [clang][darwin] Add support for --emit-static-lib

2021-09-13 Thread Keith Smiley via Phabricator via cfe-commits
keith updated this revision to Diff 372286.
keith marked an inline comment as done.
keith added a comment.

Fix formatting and comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109461

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/bindings.c
  clang/test/Driver/darwin-static-lib.c

Index: clang/test/Driver/darwin-static-lib.c
===
--- /dev/null
+++ clang/test/Driver/darwin-static-lib.c
@@ -0,0 +1,5 @@
+// RUN: %clang -target i386-apple-darwin9 %s -### --emit-static-lib 2>&1 | FileCheck %s
+// CHECK: "{{.*}}libtool" "-static" "-D" "-no_warning_for_no_symbols" "-o" "a.out" "{{.*o}}"
+
+// RUN: %clang -target i386-apple-darwin9 %s -### --emit-static-lib -o libfoo.a 2>&1 | FileCheck %s --check-prefix=OUTPUT
+// OUTPUT: "{{.*}}libtool" "-static" "-D" "-no_warning_for_no_symbols" "-o" "libfoo.a" "{{.*o}}"
Index: clang/test/Driver/bindings.c
===
--- clang/test/Driver/bindings.c
+++ clang/test/Driver/bindings.c
@@ -27,3 +27,7 @@
 // GNU StaticLibTool binding
 // RUN: %clang -target x86_64-linux-gnu -ccc-print-bindings --emit-static-lib %s 2>&1 | FileCheck %s --check-prefix=CHECK15
 // CHECK15: "x86_64-unknown-linux-gnu" - "GNU::StaticLibTool", inputs: ["{{.*}}.o"], output: "a.out"
+
+// Darwin StaticLibTool binding
+// RUN: %clang -target i386-apple-darwin9 -ccc-print-bindings --emit-static-lib %s 2>&1 | FileCheck %s --check-prefix=CHECK16
+// CHECK16: "i386-apple-darwin9" - "darwin::StaticLibTool", inputs: ["{{.*}}.o"], output: "a.out"
Index: clang/lib/Driver/ToolChains/Darwin.h
===
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -78,6 +78,20 @@
 const char *LinkingOutput) const override;
 };
 
+class LLVM_LIBRARY_VISIBILITY StaticLibTool : public MachOTool {
+public:
+  StaticLibTool(const ToolChain &TC)
+  : MachOTool("darwin::StaticLibTool", "static-lib-linker", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return true; }
+
+  void ConstructJob(Compilation &C, const JobAction &JA,
+const InputInfo &Output, const InputInfoList &Inputs,
+const llvm::opt::ArgList &TCArgs,
+const char *LinkingOutput) const override;
+};
+
 class LLVM_LIBRARY_VISIBILITY Lipo : public MachOTool {
 public:
   Lipo(const ToolChain &TC) : MachOTool("darwin::Lipo", "lipo", TC) {}
@@ -125,6 +139,7 @@
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
+  Tool *buildStaticLibTool() const override;
   Tool *getTool(Action::ActionClass AC) const override;
 
 private:
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -196,8 +196,8 @@
   return true;
 if (A->getOption().matches(options::OPT_O))
   return llvm::StringSwitch(A->getValue())
-.Case("1", true)
-.Default(false);
+  .Case("1", true)
+  .Default(false);
 return false; // OPT_Ofast & OPT_O4
   }
 
@@ -296,8 +296,8 @@
 if ((A = Args.getLastArg(options::OPT_compatibility__version)) ||
 (A = Args.getLastArg(options::OPT_current__version)) ||
 (A = Args.getLastArg(options::OPT_install__name)))
-  D.Diag(diag::err_drv_argument_only_allowed_with) << A->getAsString(Args)
-   << "-dynamiclib";
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "-dynamiclib";
 
 Args.AddLastArg(CmdArgs, options::OPT_force__flat__namespace);
 Args.AddLastArg(CmdArgs, options::OPT_keep__private__externs);
@@ -312,8 +312,8 @@
 (A = Args.getLastArg(options::OPT_force__flat__namespace)) ||
 (A = Args.getLastArg(options::OPT_keep__private__externs)) ||
 (A = Args.getLastArg(options::OPT_private__bundle)))
-  D.Diag(diag::err_drv_argument_not_allowed_with) << A->getAsString(Args)
-  << "-dynamiclib";
+  D.Diag(diag::err_drv_argument_not_allowed_with)
+  << A->getAsString(Args) << "-dynamiclib";
 
 Args.AddAllArgsTranslated(CmdArgs, options::OPT_compatibility__version,
   "-dylib_compatibility_version");
@@ -729,6 +729,54 @@
   C.addCommand(std::move(Cmd));
 }
 
+void darwin::StaticLibTool::ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output,
+ const InputInfoLi

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

2021-09-13 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd updated this revision to Diff 372285.
compnerd added a comment.

Address review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s readability-container-data-pointer %t
+
+typedef __SIZE_TYPE__ size_t;
+
+namespace std {
+template 
+struct vector {
+  using size_type = size_t;
+
+  vector();
+  explicit vector(size_type);
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+};
+
+template 
+struct basic_string {
+  using size_type = size_t;
+
+  basic_string();
+
+  T *data();
+  const T *data() const;
+
+  T &operator[](size_t);
+  const T &operator[](size_type) const;
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+
+template 
+struct is_integral;
+
+template <>
+struct is_integral {
+  static const bool value = true;
+};
+
+template 
+struct enable_if { };
+
+template 
+struct enable_if {
+  typedef T type;
+};
+}
+
+template 
+void f(const T *);
+
+#define z (0)
+
+void g(size_t s) {
+  std::vector b(s);
+  f(&((b)[(z)]));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+void h() {
+  std::string s;
+  f(&((s).operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(s.data());{{$}}
+
+  std::wstring w;
+  f(&((&(w))->operator[]((z;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(w.data());{{$}}
+}
+
+template ::value>::type>
+void i(U s) {
+  std::vector b(s);
+  f(&b[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(b.data());{{$}}
+}
+
+template void i(size_t);
+
+void j(std::vector * const v) {
+  f(&(*v)[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v->data());{{$}}
+}
+
+void k(const std::vector &v) {
+  f(&v[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}f(v.data());{{$}}
+}
+
+void l() {
+  unsigned char b[32];
+  f(&b[0]);
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-data-pointer.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - readability-data-pointer
+
+readability-data-pointer
+
+
+Finds cases where code could use ``data()`` rather than the address of the
+element at index 0 in a container.  This pattern is commonly used to materialize
+a pointer to the backing data of a container.  ``std::vector`` and
+``std::string`` provide a ``data()`` accessor to retrieve the data pointer which
+should be preferred.
+
+This also ensures that in the case that the container is empty, the data pointer
+access does not perform an errant memory access.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -91,6 +91,11 @@
   variables and function parameters only.
 
 
+- New :doc:`readability-dat

[PATCH] D109557: Adds an AlignCloseBracket option

2021-09-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

Please resubmit the patch with full context


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

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


[PATCH] D109461: [clang][darwin] Add support for --emit-static-lib

2021-09-13 Thread Keith Smiley via Phabricator via cfe-commits
keith updated this revision to Diff 372287.
keith added a comment.

Revert unrelated formatting fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109461

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/test/Driver/bindings.c
  clang/test/Driver/darwin-static-lib.c

Index: clang/test/Driver/darwin-static-lib.c
===
--- /dev/null
+++ clang/test/Driver/darwin-static-lib.c
@@ -0,0 +1,5 @@
+// RUN: %clang -target i386-apple-darwin9 %s -### --emit-static-lib 2>&1 | FileCheck %s
+// CHECK: "{{.*}}libtool" "-static" "-D" "-no_warning_for_no_symbols" "-o" "a.out" "{{.*o}}"
+
+// RUN: %clang -target i386-apple-darwin9 %s -### --emit-static-lib -o libfoo.a 2>&1 | FileCheck %s --check-prefix=OUTPUT
+// OUTPUT: "{{.*}}libtool" "-static" "-D" "-no_warning_for_no_symbols" "-o" "libfoo.a" "{{.*o}}"
Index: clang/test/Driver/bindings.c
===
--- clang/test/Driver/bindings.c
+++ clang/test/Driver/bindings.c
@@ -27,3 +27,7 @@
 // GNU StaticLibTool binding
 // RUN: %clang -target x86_64-linux-gnu -ccc-print-bindings --emit-static-lib %s 2>&1 | FileCheck %s --check-prefix=CHECK15
 // CHECK15: "x86_64-unknown-linux-gnu" - "GNU::StaticLibTool", inputs: ["{{.*}}.o"], output: "a.out"
+
+// Darwin StaticLibTool binding
+// RUN: %clang -target i386-apple-darwin9 -ccc-print-bindings --emit-static-lib %s 2>&1 | FileCheck %s --check-prefix=CHECK16
+// CHECK16: "i386-apple-darwin9" - "darwin::StaticLibTool", inputs: ["{{.*}}.o"], output: "a.out"
Index: clang/lib/Driver/ToolChains/Darwin.h
===
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -78,6 +78,20 @@
 const char *LinkingOutput) const override;
 };
 
+class LLVM_LIBRARY_VISIBILITY StaticLibTool : public MachOTool {
+public:
+  StaticLibTool(const ToolChain &TC)
+  : MachOTool("darwin::StaticLibTool", "static-lib-linker", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return true; }
+
+  void ConstructJob(Compilation &C, const JobAction &JA,
+const InputInfo &Output, const InputInfoList &Inputs,
+const llvm::opt::ArgList &TCArgs,
+const char *LinkingOutput) const override;
+};
+
 class LLVM_LIBRARY_VISIBILITY Lipo : public MachOTool {
 public:
   Lipo(const ToolChain &TC) : MachOTool("darwin::Lipo", "lipo", TC) {}
@@ -125,6 +139,7 @@
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
+  Tool *buildStaticLibTool() const override;
   Tool *getTool(Action::ActionClass AC) const override;
 
 private:
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -729,6 +729,54 @@
   C.addCommand(std::move(Cmd));
 }
 
+void darwin::StaticLibTool::ConstructJob(Compilation &C, const JobAction &JA,
+ const InputInfo &Output,
+ const InputInfoList &Inputs,
+ const ArgList &Args,
+ const char *LinkingOutput) const {
+  const Driver &D = getToolChain().getDriver();
+
+  // Silence warning for "clang -g foo.o -o foo"
+  Args.ClaimAllArgs(options::OPT_g_Group);
+  // and "clang -emit-llvm foo.o -o foo"
+  Args.ClaimAllArgs(options::OPT_emit_llvm);
+  // and for "clang -w foo.o -o foo". Other warning options are already
+  // handled somewhere else.
+  Args.ClaimAllArgs(options::OPT_w);
+  // Silence warnings when linking C code with a C++ '-stdlib' argument.
+  Args.ClaimAllArgs(options::OPT_stdlib_EQ);
+
+  // libtool   
+  ArgStringList CmdArgs;
+  // Create and insert file members with a deterministic index.
+  CmdArgs.push_back("-static");
+  CmdArgs.push_back("-D");
+  CmdArgs.push_back("-no_warning_for_no_symbols");
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Output.getFilename());
+
+  for (const auto &II : Inputs) {
+if (II.isFilename()) {
+  CmdArgs.push_back(II.getFilename());
+}
+  }
+
+  // Delete old output archive file if it already exists before generating a new
+  // archive file.
+  const auto *OutputFileName = Output.getFilename();
+  if (Output.isFilename() && llvm::sys::fs::exists(OutputFileName)) {
+if (std::error_code EC = llvm::sys::fs::remove(OutputFileName)) {
+  D.Diag(diag::err_drv_unable_to_remove_file) << EC.message();
+  return;
+}
+  }
+
+  const char *Exec = Args.MakeArgString(getToolChain().GetStaticLibToolPath());
+  C.addCommand(std::make_unique(JA, *this,
+ 

[PATCH] D109461: [clang][darwin] Add support for --emit-static-lib

2021-09-13 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/D109461/new/

https://reviews.llvm.org/D109461

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


[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

2021-09-13 Thread Vaibhav Yenamandra via Phabricator via cfe-commits
vaibhav.y updated this revision to Diff 372289.
vaibhav.y added a comment.

[clangBasic] Fixup header guard


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

Files:
  clang/include/clang/Basic/Sarif.h
  clang/include/clang/Basic/SourceLocation.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Sarif.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/unittests/Basic/CMakeLists.txt
  clang/unittests/Basic/SarifTest.cpp

Index: clang/unittests/Basic/SarifTest.cpp
===
--- /dev/null
+++ clang/unittests/Basic/SarifTest.cpp
@@ -0,0 +1,153 @@
+//===- unittests/Basic/SarifTest.cpp - Test writing SARIF documents ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Basic/Sarif.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+#include 
+
+#include "gmock/gmock.h"
+#include "gtest/gtest-death-test.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace llvm;
+
+namespace {
+
+TEST(SarifDocumentWriterTest, createEmptyDocument) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+
+  // WHEN:
+  const json::Object &emptyDocument = writer.createDocument();
+  std::vector keys(emptyDocument.size());
+  std::transform(emptyDocument.begin(), emptyDocument.end(), keys.begin(),
+ [](auto item) { return item.getFirst(); });
+
+  // THEN:
+  ASSERT_THAT(keys, testing::UnorderedElementsAre("$schema", "version"));
+}
+
+// Test that a newly inserted run will associate correct tool names
+TEST(SarifDocumentWriterTest, documentWithARun) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+  const char *shortName = "sariftest";
+  const char *longName = "sarif writer test";
+
+  // WHEN:
+  writer.createRun(shortName, longName);
+  writer.endRun();
+  const json::Object &document = writer.createDocument();
+  const json::Array *runs = document.getArray("runs");
+
+  // THEN:
+  // A run was created
+  ASSERT_THAT(runs, testing::NotNull());
+
+  // It is the only run
+  ASSERT_EQ(runs->size(), 1UL);
+
+  // The tool associated with the run was the tool
+  const json::Object *driver =
+  runs->begin()->getAsObject()->getObject("tool")->getObject("driver");
+  ASSERT_THAT(driver, testing::NotNull());
+
+  ASSERT_TRUE(driver->getString("name").hasValue());
+  ASSERT_TRUE(driver->getString("fullName").hasValue());
+  ASSERT_TRUE(driver->getString("language").hasValue());
+
+  EXPECT_EQ(driver->getString("name").getValue(), shortName);
+  EXPECT_EQ(driver->getString("fullName").getValue(), longName);
+  EXPECT_EQ(driver->getString("language").getValue(), "en-US");
+}
+
+// Test adding result without a run causes a crash
+TEST(SarifDocumentWriterTest, addingResultsWillCrashIfThereIsNoRun) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+  SarifResult &&emptyResult = SarifResult::create();
+
+  // WHEN:
+  //  A SarifDocumentWriter::createRun(...) was not called prior to
+  //  SarifDocumentWriter::appendResult(...)
+  // But a rule exists
+  auto ruleIdx = writer.createRule(SarifRule::create());
+
+  // THEN:
+  ASSERT_DEATH({ writer.appendResult(ruleIdx, emptyResult); },
+   ".*create a run first.*");
+}
+
+// Test adding result for invalid ruleIdx causes a crash
+TEST(SarifDocumentWriterTest, addingResultsWithoutRuleWillCrash) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+  SarifResult &&emptyResult = SarifResult::create();
+
+  // WHEN:
+  writer.createRun("sarif test", "sarif test runner");
+  // But caller forgot to create a rule for this run:
+
+  // THEN:
+  ASSERT_DEATH({ writer.appendResult(0, emptyResult); },
+   "Trying to reference a rule that doesn't exist");
+}
+
+// Test adding rule and result shows up in the final document
+TEST(SarifDocumentWriterTest, addResultWIthValidRuleIsOk) {
+  // GIVEN:
+  SarifDocumentWriter writer;
+  const SarifResult &result = SarifResult::create();
+  const SarifRule &rule =
+  SarifRule::create()
+  .setRuleId("clang.unittest")
+  .setDescription("Example rule created during unit tests")
+  .setName("clang unit test");
+
+  // WHEN:
+  writer.createRun("sarif test", "sarif test runner");
+  unsigned ruleIdx = writer.createRule(rule);
+  writer.appendResult(ruleIdx, result);
+  const json::Object &document = writer.createDocument();
+
+  // THEN:
+  // A document with a valid schema and version exists
+  ASSERT_THAT(document.get("$schema"), ::testing::NotNull());
+  ASSERT_THAT(document.get("version"), ::testing::NotNull());
+  const json::Array *runs = document.getArray("runs");
+
+  // A run exists on this document
+  ASSERT_THAT(runs, ::testing::NotNull());
+  ASSERT_EQ(runs->s

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

2021-09-13 Thread Vaibhav Yenamandra via Phabricator via cfe-commits
vaibhav.y added inline comments.



Comment at: clang/include/clang/Basic/Sarif.h:69
+/// Reference:
+/// 1. https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317427";>artifactLocation
 object
+/// 2. \ref SarifArtifact

I'm not sure how to deal with overlength links in docs directly, turning clang 
format off & on on comments also seems counter-productive. Would it be okay to 
add an alias in the doxyfile for the root page of SARIF docs?

E.g.:

```
ALIASES = 
sarifDocs="https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html";
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

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


[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

2021-09-13 Thread Chris Lattner via Phabricator via cfe-commits
lattner resigned from this revision.
lattner edited reviewers, added: rsmith; removed: lattner.
lattner added a comment.

I'm not sure who the best person is to review this, but it isn't me anymore 
sadly.  Richard, can you recommend someone?




Comment at: clang/lib/Basic/Sarif.cpp:1
+#include "clang/Basic/Sarif.h"
+#include "clang/Basic/LangOptions.h"

THis nees the standard header boilerplate per the coding standards doc



Comment at: clang/lib/Basic/Sarif.cpp:27
+
+StringRef getFileName(const FileEntry &FE) {
+  StringRef Filename = FE.tryGetRealPathName();

vaibhav.y wrote:
> A lot of these are copied from [[ 
> https://github.com/llvm/llvm-project/blob/181d18ef53db1e5810bf6b905fbafc91da9b5baa/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp#L64
>  | SarifDiagnostics.cpp ]]
Please use static methods instead of anonymous namespaces, per the coding 
standards doc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

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


[PATCH] D109557: Adds an AlignCloseBracket option

2021-09-13 Thread Cameron Mulhern via Phabricator via cfe-commits
csmulhern added a comment.

I've added more information to my original message. Please let me know if 
further context is desired.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557

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


[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

I think the relevance of Left/Right  East/West as a setting is less important, 
as its more about an ordering, allowing some tokens to go to the Left and some 
to the Right of the "type"

  QualifierAlignment: Custom
  QualifierOrder: [ inline, static, type, const, volatile, restrict ]

While I currently include the Left/Right for some form of completeness I sort 
of feel i should kill them before we landing this, as I think even my defaults 
are a little subjective. (unless we can get concencus), hence I don't feel the 
need to bring back up the Left/Right/East/West debate. If its still a problem 
later we can add them.

  if (Style.QualifierAlignment == FormatStyle::QAS_Right) {
Style.QualifierOrder.clear();
Style.QualifierOrder.push_back("type");
Style.QualifierOrder.push_back("const");
Style.QualifierOrder.push_back("volatile");
  } else if (Style.QualifierAlignment == FormatStyle::QAS_Left) {
Style.QualifierOrder.clear();
Style.QualifierOrder.push_back("const");
Style.QualifierOrder.push_back("volatile");
Style.QualifierOrder.push_back("type");
  }


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

https://reviews.llvm.org/D69764

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


[PATCH] D108243: Put code that avoids heapifying local blocks behind a flag

2021-09-13 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

In D108243#2995898 , @waltl wrote:

> Added driver flags, and tests for them

@ahatanak did you intend to ask Walt to add a driver flag for this?

I think we should not have one, since this isn't something we should be telling 
users to specify, or that we want to promise stability for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108243

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


[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

2021-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman edited reviewers, added: aaron.ballman; removed: rsmith.
aaron.ballman added a subscriber: lattner.
aaron.ballman added a comment.

In D109701#2997892 , @lattner wrote:

> I'm not sure who the best person is to review this, but it isn't me anymore 
> sadly.  Richard, can you recommend someone?

It's likely me -- I added the original SARIF implementation. It'll be a bit 
before I have the chance to give this a thorough review though, in case others 
who know SARIF would like to chime in.

Btw, is the intent for this functionality to replace what's in 
SarifDiagnostics.cpp 
(https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp)
 fairly immediately? Or are we going to carry both implementations? (Having put 
very little consideration into it, it seems like the static analysis output 
would be a good test of how well your changes work in practice as there are 
tools out there already consuming that output.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

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


[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 372292.
MyDeveloperDay added a comment.

QualifierAlignmentFixer need to process all the Left/Right passes internally 
before return the fixes on the original code. So now QualifierAlignmentFixer 
has its own inner set of passes which will be processed, and then "no op" fixes 
(replacements which make no change to the original code are removed (this was a 
problem before, as multiple passes causes changes that changed one way then 
changed it back  "static const int a;" -> "const static int a;" -> "static 
const a;"

Move QualifierAlignmentFixer tests out to its own unit test file so Ultimately 
I can add more "Unit tests" on the functions

Still WIP but getting much closer.


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

https://reviews.llvm.org/D69764

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/Format.cpp
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/QualifierAlignmentFixer.h
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -0,0 +1,643 @@
+//===- unittest/Format/QualifierFixerTest.cpp - Formatting unit tests -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Format/Format.h"
+
+#include "FormatTestUtils.h"
+#include "TestLexer.h"
+#include "gtest/gtest.h"
+
+#include "../../lib/Format/QualifierAlignmentFixer.h"
+
+#define DEBUG_TYPE "format-qualifier-fixer-test"
+
+using testing::ScopedTrace;
+
+namespace clang {
+namespace format {
+namespace {
+
+#define CHECK_PARSE(TEXT, FIELD, VALUE)\
+  EXPECT_NE(VALUE, Style.FIELD) << "Initial value already the same!";  \
+  EXPECT_EQ(0, parseConfiguration(TEXT, &Style).value());  \
+  EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
+
+#define FAIL_PARSE(TEXT, FIELD, VALUE) \
+  EXPECT_NE(0, parseConfiguration(TEXT, &Style).value());  \
+  EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
+
+class QualifierFixerTest : public ::testing::Test {
+protected:
+  enum StatusCheck { SC_ExpectComplete, SC_ExpectIncomplete, SC_DoNotCheck };
+
+  std::string format(llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle(),
+ StatusCheck CheckComplete = SC_ExpectComplete) {
+LLVM_DEBUG(llvm::errs() << "---\n");
+LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+std::vector Ranges(1, tooling::Range(0, Code.size()));
+FormattingAttemptStatus Status;
+tooling::Replacements Replaces =
+reformat(Style, Code, Ranges, "", &Status);
+if (CheckComplete != SC_DoNotCheck) {
+  bool ExpectedCompleteFormat = CheckComplete == SC_ExpectComplete;
+  EXPECT_EQ(ExpectedCompleteFormat, Status.FormatComplete)
+  << Code << "\n\n";
+}
+ReplacementCount = Replaces.size();
+auto Result = applyAllReplacements(Code, Replaces);
+EXPECT_TRUE(static_cast(Result));
+LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+return *Result;
+  }
+
+  FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) {
+Style.ColumnLimit = ColumnLimit;
+return Style;
+  }
+
+  FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
+return getStyleWithColumns(getLLVMStyle(), ColumnLimit);
+  }
+
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Expected,
+ llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+ScopedTrace t(File, Line, ::testing::Message() << Code.str());
+EXPECT_EQ(Expected.str(), format(Expected, Style))
+<< "Expected code is not stable";
+EXPECT_EQ(Expected.str(), format(Code, Style));
+if (Style.Language == FormatStyle::LK_Cpp) {
+  // Objective-C++ is a superset of C++, so everything checked for C++
+  // needs to be checked for Objective-C++ as well.
+  FormatStyle ObjCStyle = Style;
+  ObjCStyle.Language = FormatStyle::LK_ObjC;
+  EXPECT_EQ(Expected.str(), format(test::messUp(Code), ObjCStyle));
+}
+  }
+
+  void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+_verifyFormat(File, Line, Code, test::messUp(Cod

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D69764#2997910 , @MyDeveloperDay 
wrote:

> I think the relevance of Left/Right  East/West as a setting is less 
> important, as its more about an ordering, allowing some tokens to go to the 
> Left and some to the Right of the "type"
>
>   QualifierAlignment: Custom
>   QualifierOrder: [ inline, static, type, const, volatile, restrict ]
>
> While I currently include the Left/Right for some form of completeness I sort 
> of feel i should kill them before we landing this, as I think even my 
> defaults are a little subjective. (unless we can get concencus), hence I 
> don't feel the need to bring back up the Left/Right/East/West debate. If its 
> still a problem later we can add them.

+1, I think this style of configuration is easier to reason about than 
left/right, east/west, before/after, etc because it's an explicit ordering.


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

https://reviews.llvm.org/D69764

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


[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

2021-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:63
+hasType(Container),
+hasType(pointsTo(Container)),
+hasType(references(Container))

compnerd wrote:
> aaron.ballman wrote:
> > Same question here about `pointsTo()`.
> This is actually meant to catch cases such as references to UnOp(DRE).
Good catch and thank you for all the off-list discussion on this point!



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp:104
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'data' should be used for 
accessing the data pointer instead of taking the address of the 0-th element 
[readability-container-data-pointer]
+}

Can you add one more test for how this behaves on uninstantiated templates?
```
template 
void func(const std::vector &Vec) {
  const Ty *Ptr = &Vec[0]; // Should diagnose and provide a fix-it despite not 
instantiating func()
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108893

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


[PATCH] D109624: [clang] Make the driver not diagnose errors on nonexistent linker inputs

2021-09-13 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Hm, it passes on my win box and on the one running the presubmit test. Do you 
know if there's anything special about your bot? Any idea how I could reproduce 
this? The output is `error: command failed with exit status: True`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109624

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


[PATCH] D109710: [PowerPC] Add range checks for P10 Vector Builtins

2021-09-13 Thread Quinn Pham via Phabricator via cfe-commits
quinnp created this revision.
Herald added subscribers: shchenz, kbarton, nemanjai.
quinnp requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds range checking for some Power10 altivec builtins and
changes the signature of a builtin to match documentation. For `vec_cntm`,
range checking is done via SemaChecking. For `vec_splati_ins`, the second
argument is masked to extract the 0th bit so that we always receive either a `0`
or a `1`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109710

Files:
  clang/lib/Headers/altivec.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-ppc-p10vector-error.c
  clang/test/CodeGen/builtins-ppc-p10vector.c

Index: clang/test/CodeGen/builtins-ppc-p10vector.c
===
--- clang/test/CodeGen/builtins-ppc-p10vector.c
+++ clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -1370,10 +1370,12 @@
 }
 
 vector signed int test_vec_vec_splati_ins_si(void) {
+  // CHECK-BE: [[T0:%.+]]] = and %{{.+}}, i32 1
   // CHECK-BE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 %{{.+}}
   // CHECK-BE:  [[T1:%.+]] = add i32 2, %{{.+}}
   // CHECK-BE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T1]]
   // CHECK-BE: ret <4 x i32>
+  // CHECK-LE: [[T0:%.+]]] = and %{{.+}}, i32 1
   // CHECK-LE:  [[T1:%.+]] = sub i32 1, %{{.+}}
   // CHECK-LE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T1]]
   // CHECK-LE:  [[T2:%.+]] = sub i32 3, %{{.+}}
@@ -1383,10 +1385,12 @@
 }
 
 vector unsigned int test_vec_vec_splati_ins_ui(void) {
+  // CHECK-BE: [[T0:%.+]]] = and %{{.+}}, i32 1
   // CHECK-BE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 %{{.+}}
   // CHECK-BE:  [[T1:%.+]] = add i32 2, %{{.+}}
   // CHECK-BE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T1]]
   // CHECK-BE: ret <4 x i32>
+  // CHECK-LE: [[T0:%.+]]] = and %{{.+}}, i32 1
   // CHECK-LE:  [[T1:%.+]] = sub i32 1, %{{.+}}
   // CHECK-LE: insertelement <4 x i32> %{{.+}}, i32 %{{.+}}, i32 [[T1]]
   // CHECK-LE:  [[T2:%.+]] = sub i32 3, %{{.+}}
@@ -1396,10 +1400,12 @@
 }
 
 vector float test_vec_vec_splati_ins_f(void) {
+  // CHECK-BE: [[T0:%.+]]] = and %{{.+}}, i32 1
   // CHECK-BE: insertelement <4 x float> %{{.+}}, float %{{.+}}, i32 %{{.+}}
   // CHECK-BE:  [[T1:%.+]] = add i32 2, %{{.+}}
   // CHECK-BE: insertelement <4 x float> %{{.+}}, float %{{.+}}, i32 [[T1]]
   // CHECK-BE: ret <4 x float>
+  // CHECK-LE: [[T0:%.+]]] = and %{{.+}}, i32 1
   // CHECK-LE:  [[T1:%.+]] = sub i32 1, %{{.+}}
   // CHECK-LE: insertelement <4 x float> %{{.+}}, float %{{.+}}, i32 [[T1]]
   // CHECK-LE:  [[T2:%.+]] = sub i32 3, %{{.+}}
Index: clang/test/CodeGen/builtins-ppc-p10vector-error.c
===
--- /dev/null
+++ clang/test/CodeGen/builtins-ppc-p10vector-error.c
@@ -0,0 +1,32 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -target-cpu pwr10 \
+// RUN:   -fsyntax-only -Wall -Werror -verify %s
+// RUN: %clang_cc1 -triple powerpc64le-unknown-unknown -target-cpu pwr10 \
+// RUN:   -fsyntax-only -Wall -Werror -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-cpu pwr10 \
+// RUN:   -fsyntax-only -Wall -Werror -verify %s
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-cpu pwr10 \
+// RUN:   -fsyntax-only -Wall -Werror -verify %s
+
+#include 
+
+vector unsigned char vuca;
+vector unsigned short vusa;
+vector unsigned int vuia;
+vector unsigned long long vulla;
+
+unsigned long long test_vec_cntm_uc(void) {
+  return vec_cntm(vuca, -1); // expected-error 1+ {{argument value 255 is outside the valid range [0, 1]}}
+}
+
+unsigned long long test_vec_cntm_us(void) {
+  return vec_cntm(vusa, -1); // expected-error 1+ {{argument value 255 is outside the valid range [0, 1]}}
+}
+
+unsigned long long test_vec_cntm_ui(void) {
+  return vec_cntm(vuia, 2); // expected-error 1+ {{argument value 2 is outside the valid range [0, 1]}}
+}
+
+unsigned long long test_vec_cntm_ull(void) {
+  return vec_cntm(vulla, 2); // expected-error 1+  {{argument value 2 is outside the valid range [0, 1]}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -3473,6 +3473,11 @@
 return SemaFeatureCheck(*this, TheCall, "isa-v207-instructions",
 diag::err_ppc_builtin_only_on_arch, "8") ||
SemaBuiltinConstantArgRange(TheCall, 1, 1, 16);
+  case PPC::BI__builtin_altivec_vcntmbb:
+  case PPC::BI__builtin_altivec_vcntmbh:
+  case PPC::BI__builtin_altivec_vcntmbw:
+  case PPC::BI__builtin_altivec_vcntmbd:
+return SemaBuiltinConstantArgRange(TheCall, 1, 0, 1);
 #define CUSTOM_BUILTIN(Name, Intr, Types, Acc) \
   case PPC::BI__builtin_##Name: \
 return SemaBuiltinPPCMMACall(TheCall, Types);
Index: clang/lib/Hea

[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

2021-09-13 Thread Anshil Gandhi via Phabricator via cfe-commits
gandhi21299 created this revision.
gandhi21299 added reviewers: yaxunl, aeubanks.
Herald added subscribers: foad, kerbowa, hiraditya, tpr, nhaehnle, jvesely, 
arsenm.
gandhi21299 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

By default clang emits complete contructors as alias of base constructors if 
they are the same. The backend is supposed to emit symbols for the alias, 
otherwise it causes undefined symbols. @yaxunl observed that this issue is 
related to the llvm options `-amdgpu-early-inline-all=true` and 
`-amdgpu-function-calls=false`. Disabling the AlwaysInliner fixes this issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109707

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/amdgpu-alias-undef-symbols.hip
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp


Index: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -689,8 +689,6 @@
 if (InternalizeSymbols) {
   PM.addPass(GlobalDCEPass());
 }
-if (EarlyInlineAll && !EnableFunctionCalls)
-  PM.addPass(AMDGPUAlwaysInlinePass());
   });
 
   PB.registerCGSCCOptimizerLateEPCallback(
Index: clang/test/CodeGen/amdgpu-alias-undef-symbols.hip
===
--- /dev/null
+++ clang/test/CodeGen/amdgpu-alias-undef-symbols.hip
@@ -0,0 +1,14 @@
+// RUN: %clang -c --offload-arch=gfx906 --cuda-device-only -emit-llvm -S -o - 
%s
+//  -fgpu-rdc -O3 -mllvm -amdgpu-early-inline-all=true -mllvm 
-amdgpu-function-calls=false
+//  FileCheck %s
+
+// CHECK: %struct.B = type { i8 }
+struct B {
+
+  // CHECK: @_ZN1BC1Ei = hidden unnamed_addr alias void (%struct.B*, i32), 
void (%struct.B*, i32)* @_ZN1BC2Ei
+  __device__ B(int x);
+};
+
+__device__ B::B(int x) {
+
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5066,7 +5066,7 @@
   // Enable -mconstructor-aliases except on darwin, where we have to work 
around
   // a linker bug (see ), and CUDA/AMDGPU device code,
   // where aliases aren't supported.
-  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isAMDGPU())
+  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX())
 CmdArgs.push_back("-mconstructor-aliases");
 
   // Darwin's kernel doesn't support guard variables; just die if we


Index: llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
===
--- llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
+++ llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
@@ -689,8 +689,6 @@
 if (InternalizeSymbols) {
   PM.addPass(GlobalDCEPass());
 }
-if (EarlyInlineAll && !EnableFunctionCalls)
-  PM.addPass(AMDGPUAlwaysInlinePass());
   });
 
   PB.registerCGSCCOptimizerLateEPCallback(
Index: clang/test/CodeGen/amdgpu-alias-undef-symbols.hip
===
--- /dev/null
+++ clang/test/CodeGen/amdgpu-alias-undef-symbols.hip
@@ -0,0 +1,14 @@
+// RUN: %clang -c --offload-arch=gfx906 --cuda-device-only -emit-llvm -S -o - %s
+//  -fgpu-rdc -O3 -mllvm -amdgpu-early-inline-all=true -mllvm -amdgpu-function-calls=false
+//  FileCheck %s
+
+// CHECK: %struct.B = type { i8 }
+struct B {
+
+  // CHECK: @_ZN1BC1Ei = hidden unnamed_addr alias void (%struct.B*, i32), void (%struct.B*, i32)* @_ZN1BC2Ei
+  __device__ B(int x);
+};
+
+__device__ B::B(int x) {
+
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5066,7 +5066,7 @@
   // Enable -mconstructor-aliases except on darwin, where we have to work around
   // a linker bug (see ), and CUDA/AMDGPU device code,
   // where aliases aren't supported.
-  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isAMDGPU())
+  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX())
 CmdArgs.push_back("-mconstructor-aliases");
 
   // Darwin's kernel doesn't support guard variables; just die if we
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

2021-09-13 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

While I think the early inliner is largely obsolete, it should still handle 
aliases correctly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109707

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


[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

2021-09-13 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5069
   // where aliases aren't supported.
-  if (!RawTriple.isOSDarwin() && !RawTriple.isNVPTX() && !RawTriple.isAMDGPU())
 CmdArgs.push_back("-mconstructor-aliases");

This looks like an unrelated change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109707

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


[PATCH] D108243: Put code that avoids heapifying local blocks behind a flag

2021-09-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

I was just asking for test cases as I thought it was already a driver option, 
but it turns out it wasn't. I agree that this shouldn't be a driver option.

Sorry @waltl, please revert the changes that made it a driver option in the 
last patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108243

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


[PATCH] D109608: [clang][ASTImporter] Generic attribute import handling (first step).

2021-09-13 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:8448
+
+class AttrImporter {
+  Error Err = Error::success();

shafik wrote:
> @martong the `ASTImporter` file is quite large as it is and I think it makes 
> sense to have `AttrImporter` but perhaps it would help long-term 
> maintainability to split it out into its own source file? What do you think?
Yes, good idea, I agree. Although, I wouldn't insist to do that in this patch, 
a following patch is okay for me.

After all, it would make sense to split ASTNodeImporter into it's own 
implementation file. Or, even better,  we could put Visit*Type, Visit*Decl and 
Visit*Stmt into separate files. Other than that, the ASTImporterTest.cpp test 
file compiles painfully slow, so that is among my TODOs for a while to split 
that up as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109608

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


[PATCH] D109707: [HIP] [AlwaysInliner] Disable AlwaysInliner to eliminate undefined symbols

2021-09-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

We cannot disable early inline all since this will cause performance 
regressions. Instead, the early inline all pass should be fixed so it does not 
remove aliases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109707

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


  1   2   >