[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Teresa Johnson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG72bdb41a06a2: [Docs] Document --lto-whole-program-visibility 
(authored by tejohnson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655

Files:
  clang/docs/LTOVisibility.rst


Index: clang/docs/LTOVisibility.rst
===
--- clang/docs/LTOVisibility.rst
+++ clang/docs/LTOVisibility.rst
@@ -35,6 +35,16 @@
 (e.g. classes declared in unnamed namespaces) also receive hidden LTO
 visibility.
 
+During the LTO link, all classes with public LTO visibility will be refined
+to hidden LTO visibility when the ``--lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole-program-visibility`` for gold). This 
flag
+can be used to defer specifying whether classes have hidden LTO visibility 
until
+link time, to allow bitcode objects to be shared by different LTO links.
+Due to an implementation limitation, symbols associated with classes with 
hidden
+LTO visibility may still be exported from the binary when using this flag. It 
is
+unsafe to refer to these symbols, and their visibility may be relaxed to hidden
+in a future compiler release.
+
 A class defined in a translation unit built without LTO receives public
 LTO visibility regardless of its object file visibility, linkage or other
 attributes.


Index: clang/docs/LTOVisibility.rst
===
--- clang/docs/LTOVisibility.rst
+++ clang/docs/LTOVisibility.rst
@@ -35,6 +35,16 @@
 (e.g. classes declared in unnamed namespaces) also receive hidden LTO
 visibility.
 
+During the LTO link, all classes with public LTO visibility will be refined
+to hidden LTO visibility when the ``--lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole-program-visibility`` for gold). This flag
+can be used to defer specifying whether classes have hidden LTO visibility until
+link time, to allow bitcode objects to be shared by different LTO links.
+Due to an implementation limitation, symbols associated with classes with hidden
+LTO visibility may still be exported from the binary when using this flag. It is
+unsafe to refer to these symbols, and their visibility may be relaxed to hidden
+in a future compiler release.
+
 A class defined in a translation unit built without LTO receives public
 LTO visibility regardless of its object file visibility, linkage or other
 attributes.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 72bdb41 - [Docs] Document --lto-whole-program-visibility

[Teresa Johnson via cfe-commits]

Author: Teresa Johnson
Date: 2020-08-25T19:44:54-07:00
New Revision: 72bdb41a06a27b5453bf966a0ffecfa6f5fae1a6

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

LOG: [Docs] Document --lto-whole-program-visibility

Summary:
Documents interaction of linker option added in D71913 with LTO
visibility.

Reviewers: pcc

Subscribers: inglorion, hiraditya, steven_wu, dexonsmith, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/docs/LTOVisibility.rst

Removed: 




diff  --git a/clang/docs/LTOVisibility.rst b/clang/docs/LTOVisibility.rst
index 3a60f54e1b90..cdc0b9cc0e19 100644
--- a/clang/docs/LTOVisibility.rst
+++ b/clang/docs/LTOVisibility.rst
@@ -35,6 +35,16 @@ other classes receive hidden LTO visibility. Classes with 
internal linkage
 (e.g. classes declared in unnamed namespaces) also receive hidden LTO
 visibility.
 
+During the LTO link, all classes with public LTO visibility will be refined
+to hidden LTO visibility when the ``--lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole-program-visibility`` for gold). This 
flag
+can be used to defer specifying whether classes have hidden LTO visibility 
until
+link time, to allow bitcode objects to be shared by 
diff erent LTO links.
+Due to an implementation limitation, symbols associated with classes with 
hidden
+LTO visibility may still be exported from the binary when using this flag. It 
is
+unsafe to refer to these symbols, and their visibility may be relaxed to hidden
+in a future compiler release.
+
 A class defined in a translation unit built without LTO receives public
 LTO visibility regardless of its object file visibility, linkage or other
 attributes.



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

[PATCH] D86427: Fix some spelling errors

[YangZhihui via Phabricator via cfe-commits]

YangZhihui added a comment.

In D86427#2237668 , @plotfi wrote:

> In D86427#2235630 , @YangZhihui 
> wrote:
>
>> how can I add the Author property in the patch 
>> Thanks
>
> Is your email `yangzh.f...@cn.fujitsu.com` associated with a github account? 
> If it is, this is all I need.

Yes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86427

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

[PATCH] D86427: Fix some spelling errors

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D86427#2235630 , @YangZhihui wrote:

> how can I add the Author property in the patch 
> Thanks

Is your email `yangzh.f...@cn.fujitsu.com` associated with a github account? If 
it is, this is all I need.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86427

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

[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

[Ed Maste via Phabricator via cfe-commits]

emaste added inline comments.



Comment at: clang/lib/Basic/Targets.cpp:361-362
   return new LinuxTargetInfo(Triple, Opts);
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);
 case llvm::Triple::NetBSD:

List was previously in alpha order


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73425

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

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Peter Collingbourne via Phabricator via cfe-commits]

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

LGTM

Thanks and sorry for the delay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655

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

[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

[Fangrui Song via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73425

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

[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

[Brandon Bergren via Phabricator via cfe-commits]

Bdragon28 updated this revision to Diff 287811.
Bdragon28 added a comment.

Use correct target for FreeBSD driver test.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73425

Files:
  clang/lib/Basic/Targets.cpp
  clang/test/CodeGen/target-data.c
  clang/test/Driver/freebsd.c
  clang/test/Driver/ppc-abi.c
  clang/test/Preprocessor/init-ppc64.c


Index: clang/test/Preprocessor/init-ppc64.c
===
--- clang/test/Preprocessor/init-ppc64.c
+++ clang/test/Preprocessor/init-ppc64.c
@@ -1067,6 +1067,7 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-unknown-freebsd11 
-target-abi elfv1 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv1 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-unknown-freebsd12 
-target-abi elfv1 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv1 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-unknown-freebsd13 
-target-abi elfv2 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv2 %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64le-unknown-freebsd13 
-target-abi elfv2 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv2 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-unknown-openbsd 
-target-abi elfv2 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv2 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-linux-musl 
-target-abi elfv2 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv2 %s
 
@@ -1079,4 +1080,5 @@
 // PPC64LE-LINUX:#define _CALL_LINUX 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-unknown-freebsd < 
/dev/null | FileCheck -match-full-lines -check-prefix PPC64-FREEBSD %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64le-unknown-freebsd < 
/dev/null | FileCheck -match-full-lines -check-prefix PPC64-FREEBSD %s
 // PPC64-FREEBSD-NOT: #define __LONG_DOUBLE_128__ 1
Index: clang/test/Driver/ppc-abi.c
===
--- clang/test/Driver/ppc-abi.c
+++ clang/test/Driver/ppc-abi.c
@@ -20,6 +20,7 @@
 // RUN: %clang -target powerpc64-unknown-freebsd12 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv1 %s
 // RUN: %clang -target powerpc64-unknown-freebsd13 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE %s
 // RUN: %clang -target powerpc64-unknown-freebsd14 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE %s
+// RUN: %clang -target powerpc64le-unknown-freebsd13 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64-unknown-openbsd %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE-PIE %s
 // RUN: %clang -target powerpc64-linux-musl %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE-PIE %s
 
Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -21,7 +21,15 @@
 // CHECK-PPC64: "-cc1" "-triple" "powerpc64-pc-freebsd8"
 // CHECK-PPC64: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-PPC64: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" 
"a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" 
"-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"{{.*}}crtend.o" "{{.*}}crtn.o"
-//
+
+// RUN: %clang -no-canonical-prefixes \
+// RUN:   -target powerpc64le-unknown-freebsd13 %s \
+// RUN:   --sysroot=%S/Inputs/basic_freebsd64_tree -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-PPC64LE %s
+// CHECK-PPC64LE: "-cc1" "-triple" "powerpc64le-unknown-freebsd13"
+// CHECK-PPC64LE: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-PPC64LE: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" 
"a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" 
"-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"{{.*}}crtend.o" "{{.*}}crtn.o"
+
 //
 // Check that -m32 properly adjusts the toolchain flags.
 //
Index: clang/test/CodeGen/target-data.c
===
--- clang/test/CodeGen/target-data.c
+++ clang/test/CodeGen/target-data.c
@@ -130,6 +130,10 @@
 // RUN: FileCheck %s -check-prefix=PPC64-FREEBSD
 // PPC64-FREEBSD: target datalayout = "E-m:e-i64:64-n32:64"
 
+// RUN: %clang_cc1 -triple powerpc64le-freebsd -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=PPC64LE-FREEBSD
+// PPC64LE-FREEBSD: target datalayout = "e-m:e-i64:64-n32:64"
+
 // RUN: %clang_cc1 -triple powerpc64-linux -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=PPC64-LINUX
 // PPC64-LINUX: target datalayout = "E-m:e-i64:64-n32:64"
Index: clang/lib/Basic/Targets.cpp
===
--- 

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 287808.
tejohnson marked an inline comment as done.
tejohnson added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655

Files:
  clang/docs/LTOVisibility.rst


Index: clang/docs/LTOVisibility.rst
===
--- clang/docs/LTOVisibility.rst
+++ clang/docs/LTOVisibility.rst
@@ -35,6 +35,16 @@
 (e.g. classes declared in unnamed namespaces) also receive hidden LTO
 visibility.
 
+During the LTO link, all classes with public LTO visibility will be refined
+to hidden LTO visibility when the ``--lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole-program-visibility`` for gold). This 
flag
+can be used to defer specifying whether classes have hidden LTO visibility 
until
+link time, to allow bitcode objects to be shared by different LTO links.
+Due to an implementation limitation, symbols associated with classes with 
hidden
+LTO visibility may still be exported from the binary when using this flag. It 
is
+unsafe to refer to these symbols, and their visibility may be relaxed to hidden
+in a future compiler release.
+
 A class defined in a translation unit built without LTO receives public
 LTO visibility regardless of its object file visibility, linkage or other
 attributes.


Index: clang/docs/LTOVisibility.rst
===
--- clang/docs/LTOVisibility.rst
+++ clang/docs/LTOVisibility.rst
@@ -35,6 +35,16 @@
 (e.g. classes declared in unnamed namespaces) also receive hidden LTO
 visibility.
 
+During the LTO link, all classes with public LTO visibility will be refined
+to hidden LTO visibility when the ``--lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole-program-visibility`` for gold). This flag
+can be used to defer specifying whether classes have hidden LTO visibility until
+link time, to allow bitcode objects to be shared by different LTO links.
+Due to an implementation limitation, symbols associated with classes with hidden
+LTO visibility may still be exported from the binary when using this flag. It is
+unsafe to refer to these symbols, and their visibility may be relaxed to hidden
+in a future compiler release.
+
 A class defined in a translation unit built without LTO receives public
 LTO visibility regardless of its object file visibility, linkage or other
 attributes.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson marked an inline comment as done.
tejohnson added a comment.

In D75655#2237474 , @pcc wrote:

> I think that the part of the description beginning "This can be used when..." 
> is somewhat misleading since you could pretty much say the same thing about 
> specifying `-fvisibility=hidden -fwhole-program-vtables` at compile time.
>
> I think it would be better to focus on the specific capability that 
> `--lto-visibility-hidden` gives you over just compiling with 
> `-fvisibility=hidden`. Let's keep @evgeny777 's use case out of this because 
> I still have concerns about it being dangerous. Can you simply say:
>
> "This flag can be used to defer specifying whether classes have hidden LTO 
> visibility until link time."
>
> Then, in order to try to forbid inappropriate use of the symbols, you could 
> add:
>
> "Due to an implementation limitation, symbols associated with classes with 
> hidden LTO visibility may still be exported from the binary when using this 
> flag. It is unsafe to refer to these symbols, and their visibility may be 
> relaxed to hidden in a future compiler release."

Added these, but added in my proposed wording about why you would want to defer 
(to allow bitcode sharing).




Comment at: clang/docs/LTOVisibility.rst:39
+During the LTO link, all classes with public LTO visibility will be refined
+to hidden LTO visibility when the ``-lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole-program-visibility`` for gold). This

MaskRay wrote:
> This is now
> 
> ``--lto-whole-program-visibility``
> 
> (I am currently preparing some lld/ELF release notes for 11.0.0...)
Fixed here and in summary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655

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

[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

[Brandon Bergren via Phabricator via cfe-commits]

Bdragon28 added inline comments.



Comment at: clang/test/Driver/freebsd.c:32
+// CHECK-PPC64LE: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" 
"a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" 
"-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"{{.*}}crtend.o" "{{.*}}crtn.o"
+//
 //

MaskRay wrote:
> Drop `//` on otherwise empty lines.
Filewide? I was attempting to follow the style of the surrounding code.



Comment at: clang/test/Driver/ppc-abi.c:24
+// RUN: %clang -target powerpc64le-unknown-freebsd13 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2 %s
+// RUN: %clang -target powerpc64le-unknown-freebsd14 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64-unknown-openbsd %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE-PIE %s

MaskRay wrote:
> One freebsd13 should be sufficient?
> 
> Every RUN line makes the testsuite a bit slower.
Fine with that, will change. Was just matching what ELFv2-BE does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73425

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

[PATCH] D86581: [clang-format] Handle shifts within conditions

[Miguel Saldivar via Phabricator via cfe-commits]

Saldivarcher created this revision.
Saldivarcher added a reviewer: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Saldivarcher requested review of this revision.

In some situation shifts can be treated as a template, and is thus formatted as 
one. So, by doing a couple extra checks to assure that the condition doesn't 
contain a template, and is in fact a bit shift should solve this problem.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86581

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7540,6 +7540,9 @@
"  while (a < b && c > d) {\n"
"  }\n"
"}");
+  verifyFormat("int j = 10;\n"
+   "for (int i = 0; i < j >> 1; i++)\n"
+   "  ;");
   verifyFormat("template \n"
"typename enable_if<0 < sizeof...(Types)>::type Foo() {}");
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -56,6 +56,13 @@
  Left->Previous->MatchingParen->is(TT_LambdaLSquare);
 }
 
+/// Returns \c true if the token is followed by a boolean condition, \c false
+/// otherwise.
+static bool isKeywordWithCondition(const FormatToken ) {
+  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
+ tok::kw_constexpr, tok::kw_catch);
+}
+
 /// A parser that gathers additional information about tokens.
 ///
 /// The \c TokenAnnotator tries to match parenthesis and square brakets and
@@ -108,6 +115,12 @@
 
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
+// Try to do a better job at looking for ">>" within the condition of
+// a statement.
+if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
+Left->ParentBracket != tok::less &&
+isKeywordWithCondition(*Line.First))
+  return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // In TT_Proto, we must distignuish between:
@@ -2733,13 +2746,6 @@
   Right.ParameterCount > 0);
 }
 
-/// Returns \c true if the token is followed by a boolean condition, \c false
-/// otherwise.
-static bool isKeywordWithCondition(const FormatToken ) {
-  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
- tok::kw_constexpr, tok::kw_catch);
-}
-
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,
   const FormatToken ) {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7540,6 +7540,9 @@
"  while (a < b && c > d) {\n"
"  }\n"
"}");
+  verifyFormat("int j = 10;\n"
+   "for (int i = 0; i < j >> 1; i++)\n"
+   "  ;");
   verifyFormat("template \n"
"typename enable_if<0 < sizeof...(Types)>::type Foo() {}");
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -56,6 +56,13 @@
  Left->Previous->MatchingParen->is(TT_LambdaLSquare);
 }
 
+/// Returns \c true if the token is followed by a boolean condition, \c false
+/// otherwise.
+static bool isKeywordWithCondition(const FormatToken ) {
+  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
+ tok::kw_constexpr, tok::kw_catch);
+}
+
 /// A parser that gathers additional information about tokens.
 ///
 /// The \c TokenAnnotator tries to match parenthesis and square brakets and
@@ -108,6 +115,12 @@
 
 while (CurrentToken) {
   if (CurrentToken->is(tok::greater)) {
+// Try to do a better job at looking for ">>" within the condition of
+// a statement.
+if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
+Left->ParentBracket != tok::less &&
+isKeywordWithCondition(*Line.First))
+  return false;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // In TT_Proto, we must distignuish between:
@@ -2733,13 +2746,6 @@
   Right.ParameterCount > 0);
 }
 
-/// Returns \c true if the token is followed by a boolean condition, \c false
-/// otherwise.
-static bool isKeywordWithCondition(const FormatToken ) {
-  return Tok.isOneOf(tok::kw_if, tok::kw_for, 

[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/freebsd.c:32
+// CHECK-PPC64LE: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" 
"a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" 
"-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"{{.*}}crtend.o" "{{.*}}crtn.o"
+//
 //

Drop `//` on otherwise empty lines.



Comment at: clang/test/Driver/ppc-abi.c:24
+// RUN: %clang -target powerpc64le-unknown-freebsd13 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2 %s
+// RUN: %clang -target powerpc64le-unknown-freebsd14 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64-unknown-openbsd %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE-PIE %s

One freebsd13 should be sufficient?

Every RUN line makes the testsuite a bit slower.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73425

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

[PATCH] D82777: Clang Driver: Use Apple ld64's new @response-file support.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

BTW, to the apple folks here, it'd sure be awesome if this could be backported 
into XCode 12's clang. :) (c.f. FB7037642).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82777

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

[PATCH] D85735: [OpenMP] Context selector extensions for template functions

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 5 inline comments as done.
jdoerfert added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:10042
   void ActOnFinishedFunctionDefinitionInOpenMPDeclareVariantScope(
-  FunctionDecl *FD, FunctionDecl *BaseFD);
+  Decl *D, SmallVectorImpl );
 

ABataev wrote:
> JonChesterfield wrote:
> > SmallVectorImpl => SmallVector?
> It is a good practice to pass SmallVectors as SmallVectorImpl to make it 
> independent of concrete SmallVector template specialization.
We usually use the Impl versions for the APIs to avoid hardcoding the "size" 
everywhere. 



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5826
 
-FunctionDecl *
-Sema::ActOnStartOfFunctionDefinitionInOpenMPDeclareVariantScope(Scope *S,
-Declarator ) 
{
+void Sema::ActOnStartOfFunctionDefinitionInOpenMPDeclareVariantScope(
+Scope *S, Declarator , MultiTemplateParamsArg TemplateParamLists,

JonChesterfield wrote:
> Return SmallVector by value instead of via the reference? Makes 
> it clear that we aren't adding to an existing vector
Hm, I guess we can do that, it will cause some more copies. FWIW, we are adding 
to an existing vector if bases is not empty. I mean, if you provide an empty 
vector we add and if there is something in the vector we add.



Comment at: 
clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp:57
+
+// CHECK:  |-FunctionTemplateDecl [[ADDR_0:0x[a-z0-9]*]] <{{.*}}, 
line:7:1> line:5:5 also_before
+// CHECK-NEXT: | |-TemplateTypeParmDecl [[ADDR_1:0x[a-z0-9]*]]  col:20 referenced typename depth 0 index 0 T

JonChesterfield wrote:
> I'd like a different way for us to test this stuff. Realistically impossible 
> to spot errors in 200 lines of tree. Don't have a suggestion at this time.
I manually checked it, and now we can verify changes. At the end of the day 
checking this by hand is actually not too bad. The trick is that you can skip 
most of it. You go to the test function return value at the very end. Each call 
expression there should call a function that returns 0. So follow the callee 
and check the return there (nothing else anyway). I used this scheme for all 
tests and once you did verify one it is really easy to do the others. Assuming 
my input is proper ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85735

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

[PATCH] D85735: [OpenMP] Context selector extensions for template functions

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:10042
   void ActOnFinishedFunctionDefinitionInOpenMPDeclareVariantScope(
-  FunctionDecl *FD, FunctionDecl *BaseFD);
+  Decl *D, SmallVectorImpl );
 

JonChesterfield wrote:
> SmallVectorImpl => SmallVector?
It is a good practice to pass SmallVectors as SmallVectorImpl to make it 
independent of concrete SmallVector template specialization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85735

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

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D86562#2237399 , @kkwli0 wrote:

> `default(firstprivate)` was added in 
> https://reviews.llvm.org/rG78443666bc18a6957d279a0f58319c8a3e57771a

Yest, it was before clang 11 was branched, so need to add to the list of the 
changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86562

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

[PATCH] D85777: [OpenMP] Support std::complex math functions in target regions

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/lib/Headers/openmp_wrappers/complex_cmath.h:58
+
+template  __DEVICE__ _Tp norm(const std::complex<_Tp> &__c) {
+  if (std::isinf(__c.real()))

JonChesterfield wrote:
> Doesn't matter hugely given inlining, but I'm surprised to see std::complex 
> taken by const & instead of by value.
I took this from the libc++, I hope they know ;)



Comment at: clang/test/Headers/Inputs/include/type_traits:3
+
+#pragma once
+

JonChesterfield wrote:
> In order to run the tests without libc++/libstdc++ available?
yes. this allows to include the header in the test w/o any system dependence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85777

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

[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai marked an inline comment as done.
vsapsai added a comment.

Thanks for the review.




Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:700
+  "no module named '%0' %select{found|in '%2'}1, "
+  "expected parent module to be defined before the submodule">;
 def err_mmap_top_level_inferred_submodule : Error<

bruno wrote:
> vsapsai wrote:
> > Not sure about this second part of the message ("expected..."). Wanted to 
> > make it more actionable but not entirely happy with the wording. Any 
> > suggestions including removal are welcome.
> How about `, parent module must be defined before the submodule`? 
Thanks, I think it is more direct and slightly more concise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84458

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

[clang] 8839e27 - [Modules] Improve error message when cannot find parent module for submodule definition.

[Volodymyr Sapsai via cfe-commits]

Author: Volodymyr Sapsai
Date: 2020-08-25T16:31:27-07:00
New Revision: 8839e278ffcadc62b333423de07756488cae980f

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

LOG: [Modules] Improve error message when cannot find parent module for 
submodule definition.

Before the change the diagnostic for

module unknown.submodule {}

was "error: expected module name" which is incorrect and misleading
because both "unknown" and "submodule" are valid module names.

We already have a better error message when a parent module is a
submodule itself and is missing. Make the error for a missing top-level
module more like the one for a submodule.

rdar://problem/64424407

Reviewed By: bruno

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/lib/Lex/ModuleMap.cpp
clang/test/Modules/diagnostics.modulemap

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticLexKinds.td 
b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 9cb06cf5b5e1..77d2e26ba790 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -695,6 +695,9 @@ def err_mmap_missing_module_unqualified : Error<
   "no module named '%0' visible from '%1'">;
 def err_mmap_missing_module_qualified : Error<
   "no module named '%0' in '%1'">;
+def err_mmap_missing_parent_module: Error<
+  "no module named '%0' %select{found|in '%2'}1, "
+  "parent module must be defined before the submodule">;
 def err_mmap_top_level_inferred_submodule : Error<
   "only submodules and framework modules may be inferred with wildcard 
syntax">;
 def err_mmap_inferred_no_umbrella : Error<

diff  --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index bcdc5b8062a0..12da5a85da37 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -1903,18 +1903,16 @@ void ModuleMapParser::parseModuleDecl() {
 continue;
   }
 
-  if (ActiveModule) {
-Diags.Report(Id[I].second, diag::err_mmap_missing_module_qualified)
-  << Id[I].first
-  << ActiveModule->getTopLevelModule()->getFullModuleName();
-  } else {
-Diags.Report(Id[I].second, diag::err_mmap_expected_module_name);
-  }
+  Diags.Report(Id[I].second, diag::err_mmap_missing_parent_module)
+  << Id[I].first << (ActiveModule != nullptr)
+  << (ActiveModule
+  ? ActiveModule->getTopLevelModule()->getFullModuleName()
+  : "");
   HadError = true;
-  return;
 }
 
-if (ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
+if (TopLevelModule &&
+ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
   assert(ModuleMapFile != Map.getModuleMapFileForUniquing(TopLevelModule) 
&&
  "submodule defined in same file as 'module *' that allowed its "
  "top-level module");

diff  --git a/clang/test/Modules/diagnostics.modulemap 
b/clang/test/Modules/diagnostics.modulemap
index 01aa0b66a406..c12fef50c38e 100644
--- a/clang/test/Modules/diagnostics.modulemap
+++ b/clang/test/Modules/diagnostics.modulemap
@@ -28,3 +28,9 @@ module header_attr {
   header "quux.h" { size 1 mtime 2 }
   header "no_attrs.h" {}
 }
+
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:8: error: no module named 
'unknown' found, parent module must be defined before the submodule
+module unknown.submodule {}
+module known_top_level {}
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:24: error: no module named 
'unknown' in 'known_top_level', parent module must be defined before the 
submodule
+module known_top_level.unknown.submodule {}



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

[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8839e278ffca: [Modules] Improve error message when cannot 
find parent module for submodule… (authored by vsapsai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84458

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Modules/diagnostics.modulemap


Index: clang/test/Modules/diagnostics.modulemap
===
--- clang/test/Modules/diagnostics.modulemap
+++ clang/test/Modules/diagnostics.modulemap
@@ -28,3 +28,9 @@
   header "quux.h" { size 1 mtime 2 }
   header "no_attrs.h" {}
 }
+
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:8: error: no module named 
'unknown' found, parent module must be defined before the submodule
+module unknown.submodule {}
+module known_top_level {}
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:24: error: no module named 
'unknown' in 'known_top_level', parent module must be defined before the 
submodule
+module known_top_level.unknown.submodule {}
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1903,18 +1903,16 @@
 continue;
   }
 
-  if (ActiveModule) {
-Diags.Report(Id[I].second, diag::err_mmap_missing_module_qualified)
-  << Id[I].first
-  << ActiveModule->getTopLevelModule()->getFullModuleName();
-  } else {
-Diags.Report(Id[I].second, diag::err_mmap_expected_module_name);
-  }
+  Diags.Report(Id[I].second, diag::err_mmap_missing_parent_module)
+  << Id[I].first << (ActiveModule != nullptr)
+  << (ActiveModule
+  ? ActiveModule->getTopLevelModule()->getFullModuleName()
+  : "");
   HadError = true;
-  return;
 }
 
-if (ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
+if (TopLevelModule &&
+ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
   assert(ModuleMapFile != Map.getModuleMapFileForUniquing(TopLevelModule) 
&&
  "submodule defined in same file as 'module *' that allowed its "
  "top-level module");
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -695,6 +695,9 @@
   "no module named '%0' visible from '%1'">;
 def err_mmap_missing_module_qualified : Error<
   "no module named '%0' in '%1'">;
+def err_mmap_missing_parent_module: Error<
+  "no module named '%0' %select{found|in '%2'}1, "
+  "parent module must be defined before the submodule">;
 def err_mmap_top_level_inferred_submodule : Error<
   "only submodules and framework modules may be inferred with wildcard 
syntax">;
 def err_mmap_inferred_no_umbrella : Error<


Index: clang/test/Modules/diagnostics.modulemap
===
--- clang/test/Modules/diagnostics.modulemap
+++ clang/test/Modules/diagnostics.modulemap
@@ -28,3 +28,9 @@
   header "quux.h" { size 1 mtime 2 }
   header "no_attrs.h" {}
 }
+
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:8: error: no module named 'unknown' found, parent module must be defined before the submodule
+module unknown.submodule {}
+module known_top_level {}
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:24: error: no module named 'unknown' in 'known_top_level', parent module must be defined before the submodule
+module known_top_level.unknown.submodule {}
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1903,18 +1903,16 @@
 continue;
   }
 
-  if (ActiveModule) {
-Diags.Report(Id[I].second, diag::err_mmap_missing_module_qualified)
-  << Id[I].first
-  << ActiveModule->getTopLevelModule()->getFullModuleName();
-  } else {
-Diags.Report(Id[I].second, diag::err_mmap_expected_module_name);
-  }
+  Diags.Report(Id[I].second, diag::err_mmap_missing_parent_module)
+  << Id[I].first << (ActiveModule != nullptr)
+  << (ActiveModule
+  ? ActiveModule->getTopLevelModule()->getFullModuleName()
+  : "");
   HadError = true;
-  return;
 }
 
-if (ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
+if (TopLevelModule &&
+ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
   assert(ModuleMapFile != Map.getModuleMapFileForUniquing(TopLevelModule) &&
  "submodule defined in same file as 'module *' that allowed its "
  "top-level module");
Index: 

[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

[Brandon Bergren via Phabricator via cfe-commits]

Bdragon28 updated this revision to Diff 287795.
Bdragon28 added reviewers: MaskRay, dim.
Bdragon28 added a comment.
Herald added a subscriber: kbarton.

Add some tests for the new target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73425

Files:
  clang/lib/Basic/Targets.cpp
  clang/test/CodeGen/target-data.c
  clang/test/Driver/freebsd.c
  clang/test/Driver/ppc-abi.c
  clang/test/Preprocessor/init-ppc64.c


Index: clang/test/Preprocessor/init-ppc64.c
===
--- clang/test/Preprocessor/init-ppc64.c
+++ clang/test/Preprocessor/init-ppc64.c
@@ -1067,6 +1067,7 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-unknown-freebsd11 
-target-abi elfv1 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv1 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-unknown-freebsd12 
-target-abi elfv1 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv1 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-unknown-freebsd13 
-target-abi elfv2 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv2 %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64le-unknown-freebsd13 
-target-abi elfv2 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv2 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-unknown-openbsd 
-target-abi elfv2 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv2 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-linux-musl 
-target-abi elfv2 -xc /dev/null | FileCheck --check-prefix=PPC64-ELFv2 %s
 
@@ -1079,4 +1080,5 @@
 // PPC64LE-LINUX:#define _CALL_LINUX 1
 
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-unknown-freebsd < 
/dev/null | FileCheck -match-full-lines -check-prefix PPC64-FREEBSD %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64le-unknown-freebsd < 
/dev/null | FileCheck -match-full-lines -check-prefix PPC64-FREEBSD %s
 // PPC64-FREEBSD-NOT: #define __LONG_DOUBLE_128__ 1
Index: clang/test/Driver/ppc-abi.c
===
--- clang/test/Driver/ppc-abi.c
+++ clang/test/Driver/ppc-abi.c
@@ -20,6 +20,8 @@
 // RUN: %clang -target powerpc64-unknown-freebsd12 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv1 %s
 // RUN: %clang -target powerpc64-unknown-freebsd13 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE %s
 // RUN: %clang -target powerpc64-unknown-freebsd14 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE %s
+// RUN: %clang -target powerpc64le-unknown-freebsd13 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2 %s
+// RUN: %clang -target powerpc64le-unknown-freebsd14 %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2 %s
 // RUN: %clang -target powerpc64-unknown-openbsd %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE-PIE %s
 // RUN: %clang -target powerpc64-linux-musl %s -### 2>&1 | FileCheck 
--check-prefix=CHECK-ELFv2-BE-PIE %s
 
Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -22,6 +22,14 @@
 // CHECK-PPC64: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-PPC64: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" 
"a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" 
"-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"{{.*}}crtend.o" "{{.*}}crtn.o"
 //
+// RUN: %clang -no-canonical-prefixes \
+// RUN:   -target powerpc64-unknown-freebsd13 %s \
+// RUN:   --sysroot=%S/Inputs/basic_freebsd64_tree -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-PPC64LE %s
+// CHECK-PPC64LE: "-cc1" "-triple" "powerpc64-unknown-freebsd13"
+// CHECK-PPC64LE: ld{{.*}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-PPC64LE: "--eh-frame-hdr" "-dynamic-linker" "{{.*}}ld-elf{{.*}}" "-o" 
"a.out" "{{.*}}crt1.o" "{{.*}}crti.o" "{{.*}}crtbegin.o" 
"-L[[SYSROOT]]/usr/lib" "{{.*}}.o" "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"{{.*}}crtend.o" "{{.*}}crtn.o"
+//
 //
 // Check that -m32 properly adjusts the toolchain flags.
 //
Index: clang/test/CodeGen/target-data.c
===
--- clang/test/CodeGen/target-data.c
+++ clang/test/CodeGen/target-data.c
@@ -130,6 +130,10 @@
 // RUN: FileCheck %s -check-prefix=PPC64-FREEBSD
 // PPC64-FREEBSD: target datalayout = "E-m:e-i64:64-n32:64"
 
+// RUN: %clang_cc1 -triple powerpc64le-freebsd -o - -emit-llvm %s | \
+// RUN: FileCheck %s -check-prefix=PPC64LE-FREEBSD
+// PPC64LE-FREEBSD: target datalayout = "e-m:e-i64:64-n32:64"
+
 // RUN: %clang_cc1 -triple powerpc64-linux -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=PPC64-LINUX
 // PPC64-LINUX: target datalayout = "E-m:e-i64:64-n32:64"
Index: clang/lib/Basic/Targets.cpp

[PATCH] D85777: [OpenMP] Support std::complex math functions in target regions

[Jon Chesterfield via Phabricator via cfe-commits]

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

The duplication from libc++ doesn't feel good but I can see how it happened. 
Dependency breaking etc.




Comment at: clang/lib/Headers/openmp_wrappers/complex_cmath.h:58
+
+template  __DEVICE__ _Tp norm(const std::complex<_Tp> &__c) {
+  if (std::isinf(__c.real()))

Doesn't matter hugely given inlining, but I'm surprised to see std::complex 
taken by const & instead of by value.



Comment at: clang/test/Headers/Inputs/include/type_traits:3
+
+#pragma once
+

In order to run the tests without libc++/libstdc++ available?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85777

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

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

I think that the part of the description beginning "This can be used when..." 
is somewhat misleading since you could pretty much say the same thing about 
specifying `-fvisibility=hidden -fwhole-program-vtables` at compile time.

I think it would be better to focus on the specific capability that 
`--lto-visibility-hidden` gives you over just compiling with 
`-fvisibility=hidden`. Let's keep @evgeny777 's use case out of this because I 
still have concerns about it being dangerous. Can you simply say:

"This flag can be used to defer specifying whether classes have hidden LTO 
visibility until link time."

Then, in order to try to forbid inappropriate use of the symbols, you could add:

"Due to an implementation limitation, symbols associated with classes with 
hidden LTO visibility may still be exported from the binary when using this 
flag. It is unsafe to refer to these symbols, and their visibility may be 
relaxed to hidden in a future compiler release."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655

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

[PATCH] D85735: [OpenMP] Context selector extensions for template functions

[Jon Chesterfield via Phabricator via cfe-commits]

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

This looks good to me. I can't claim to have read the test case in detail. Some 
style suggestions inline around the vector, but they're not blocking.




Comment at: clang/include/clang/Sema/Sema.h:10042
   void ActOnFinishedFunctionDefinitionInOpenMPDeclareVariantScope(
-  FunctionDecl *FD, FunctionDecl *BaseFD);
+  Decl *D, SmallVectorImpl );
 

SmallVectorImpl => SmallVector?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5826
 
-FunctionDecl *
-Sema::ActOnStartOfFunctionDefinitionInOpenMPDeclareVariantScope(Scope *S,
-Declarator ) 
{
+void Sema::ActOnStartOfFunctionDefinitionInOpenMPDeclareVariantScope(
+Scope *S, Declarator , MultiTemplateParamsArg TemplateParamLists,

Return SmallVector by value instead of via the reference? Makes 
it clear that we aren't adding to an existing vector



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5875
+// TODO: Verify types for templates eventually.
+if (!UDeclTy->isDependentType()) {
+  QualType NewType = Context.mergeFunctionTypes(

JonChesterfield wrote:
> tabs!
This turned out to be how phabricator represents changes in indent. Unfortunate 
that the symbol is the one I usually see used for tabs.



Comment at: 
clang/test/AST/ast-dump-openmp-begin-declare-variant_template_2.cpp:57
+
+// CHECK:  |-FunctionTemplateDecl [[ADDR_0:0x[a-z0-9]*]] <{{.*}}, 
line:7:1> line:5:5 also_before
+// CHECK-NEXT: | |-TemplateTypeParmDecl [[ADDR_1:0x[a-z0-9]*]]  col:20 referenced typename depth 0 index 0 T

I'd like a different way for us to test this stuff. Realistically impossible to 
spot errors in 200 lines of tree. Don't have a suggestion at this time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85735

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

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added a comment.

`default(firstprivate)` was added in 
https://reviews.llvm.org/rG78443666bc18a6957d279a0f58319c8a3e57771a


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86562

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

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D86562#2237388 , @kkwli0 wrote:

> `present` map type and motion modifier
> `default(firstprivate)`

Present was added after 11.0 branching. Not aure about default(firstprivate)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86562

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

[clang] 9500a72 - Revert "[Coverage] Enable emitting gap area between macros"

[Zequan Wu via cfe-commits]

Author: Zequan Wu
Date: 2020-08-25T15:28:42-07:00
New Revision: 9500a7209163d6bfce7d554cb71a7c189ca5de58

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

LOG: Revert "[Coverage] Enable emitting gap area between macros"

This reverts commit a31c89c1b7a0a2fd3e2c0b8a587a60921abf4abd.

Added: 


Modified: 
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/test/CoverageMapping/if.cpp
clang/test/CoverageMapping/macro-expressions.cpp
clang/test/CoverageMapping/macroparams2.c
clang/test/CoverageMapping/macros.c
clang/test/CoverageMapping/macroscopes.cpp
clang/test/CoverageMapping/moremacros.c

Removed: 




diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 8277804d27c0..022713771342 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -868,13 +868,22 @@ struct CounterCoverageMappingBuilder
   /// Find a valid gap range between \p AfterLoc and \p BeforeLoc.
   Optional findGapAreaBetween(SourceLocation AfterLoc,
SourceLocation BeforeLoc) {
-AfterLoc = SM.getExpansionLoc(AfterLoc);
-BeforeLoc = SM.getExpansionLoc(BeforeLoc);
+// If the start and end locations of the gap are both within the same macro
+// file, the range may not be in source order.
+if (AfterLoc.isMacroID() || BeforeLoc.isMacroID())
+  return None;
 if (!SM.isWrittenInSameFile(AfterLoc, BeforeLoc))
   return None;
 return {{AfterLoc, BeforeLoc}};
   }
 
+  /// Find the source range after \p AfterStmt and before \p BeforeStmt.
+  Optional findGapAreaBetween(const Stmt *AfterStmt,
+   const Stmt *BeforeStmt) {
+return findGapAreaBetween(getPreciseTokenLocEnd(getEnd(AfterStmt)),
+  getStart(BeforeStmt));
+  }
+
   /// Emit a gap region between \p StartLoc and \p EndLoc with the given count.
   void fillGapAreaWithCount(SourceLocation StartLoc, SourceLocation EndLoc,
 Counter Count) {
@@ -1039,8 +1048,7 @@ struct CounterCoverageMappingBuilder
 adjustForOutOfOrderTraversal(getEnd(S));
 
 // The body count applies to the area immediately after the increment.
-auto Gap = findGapAreaBetween(getPreciseTokenLocEnd(S->getRParenLoc()),
-  getStart(S->getBody()));
+auto Gap = findGapAreaBetween(S->getCond(), S->getBody());
 if (Gap)
   fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), BodyCount);
 
@@ -1257,8 +1265,7 @@ struct CounterCoverageMappingBuilder
 propagateCounts(ParentCount, S->getCond());
 
 // The 'then' count applies to the area immediately after the condition.
-auto Gap = findGapAreaBetween(getPreciseTokenLocEnd(S->getRParenLoc()),
-  getStart(S->getThen()));
+auto Gap = findGapAreaBetween(S->getCond(), S->getThen());
 if (Gap)
   fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ThenCount);
 
@@ -1268,8 +1275,7 @@ struct CounterCoverageMappingBuilder
 Counter ElseCount = subtractCounters(ParentCount, ThenCount);
 if (const Stmt *Else = S->getElse()) {
   // The 'else' count applies to the area immediately after the 'then'.
-  Gap = findGapAreaBetween(getPreciseTokenLocEnd(getEnd(S->getThen())),
-   getStart(Else));
+  Gap = findGapAreaBetween(S->getThen(), Else);
   if (Gap)
 fillGapAreaWithCount(Gap->getBegin(), Gap->getEnd(), ElseCount);
   extendRegion(Else);

diff  --git a/clang/test/CoverageMapping/if.cpp 
b/clang/test/CoverageMapping/if.cpp
index 5cbc974aaf04..8ffc09d29a3c 100644
--- a/clang/test/CoverageMapping/if.cpp
+++ b/clang/test/CoverageMapping/if.cpp
@@ -44,3 +44,10 @@ int main() {// CHECK: File 0, 
[[@LINE]]:12 -> {{[0-9]+}}:2 =
 
   return 0;
 }
+
+#define FOO true
+
+// CHECK-LABEL: _Z7ternaryv:
+void ternary() {
+  true ? FOO : FOO; // CHECK-NOT: Gap,{{.*}}, [[@LINE]]:8 ->
+}

diff  --git a/clang/test/CoverageMapping/macro-expressions.cpp 
b/clang/test/CoverageMapping/macro-expressions.cpp
index 6ac82523fef8..60afc5238b9e 100644
--- a/clang/test/CoverageMapping/macro-expressions.cpp
+++ b/clang/test/CoverageMapping/macro-expressions.cpp
@@ -57,8 +57,7 @@ void foo(int i) {
   // CHECK: File 0, [[@LINE+1]]:10 -> [[@LINE+1]]:12 = #1
   if (0) {}
 
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:7 -> [[@LINE+3]]:11 = #0
-  // CHECK-NEXT: Gap,File 0, [[@LINE+2]]:15 -> [[@LINE+2]]:16 = #2
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:7 -> [[@LINE+2]]:11 = #0
   // CHECK-NEXT: File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:18 = #2
   if (EXPR(i)) {}
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:9 -> 

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added a comment.

`present` map type and motion modifier
`default(firstprivate)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86562

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

[PATCH] D86491: [DebugInfo] Move constructor homing case in shouldOmitDefinition.

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.

Looks good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86491

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

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/docs/LTOVisibility.rst:39
+During the LTO link, all classes with public LTO visibility will be refined
+to hidden LTO visibility when the ``-lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole-program-visibility`` for gold). This

This is now

``--lto-whole-program-visibility``

(I am currently preparing some lld/ELF release notes for 11.0.0...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655

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

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 28.
ABataev added a comment.

Misprint fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86562

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,7 +156,49 @@
 OpenMP Support in Clang
 ---
 
-- ...
+New features for OpenMP 5.0 were implemented.
+
+- OpenMP 5.0 is the default version supported by the compiler. User can switch
+  to OpenMP 4.5 using ``-fopenmp-version=45`` option.
+
+- Added support for declare variant directive.
+
+- Improved support of math functions and complex types for NVPTX target.
+
+- Added support for parallel execution of target regions for NVPTX target.
+
+- Added support for ``scan`` directives and ``inscan`` modifier in 
``reduction``
+  clauses.
+
+- Added support for ``iterator`` construct.
+
+- Added support for ``depobj`` construct.
+
+- Added support for ``detach`` clauses in task-based directives.
+
+- Added support for array shaping operations.
+
+- Added support for cancellation constructs in ``taskloop`` directives.
+
+- Nonmonotonic modifier is allowed with all schedule kinds.
+
+- Added support for ``task`` and ``default`` modifiers in ``reduction`` 
clauses.
+
+- Added support for strides in array sections.
+
+- Added support for ``use_device_addr`` clause.
+
+- Added support for ``uses_allocators`` clause.
+
+- Added support for ``defaultmap`` clause.
+
+- Added basic support for ``hint`` clause in ``atomic`` directives.
+
+- Added basic support for ``affinity`` clause.
+
+- Added basic support for ``ancestor`` modifier in ``device`` clause.
+
+- Bug fixes and optimizations.
 
 CUDA Support in Clang
 -


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,7 +156,49 @@
 OpenMP Support in Clang
 ---
 
-- ...
+New features for OpenMP 5.0 were implemented.
+
+- OpenMP 5.0 is the default version supported by the compiler. User can switch
+  to OpenMP 4.5 using ``-fopenmp-version=45`` option.
+
+- Added support for declare variant directive.
+
+- Improved support of math functions and complex types for NVPTX target.
+
+- Added support for parallel execution of target regions for NVPTX target.
+
+- Added support for ``scan`` directives and ``inscan`` modifier in ``reduction``
+  clauses.
+
+- Added support for ``iterator`` construct.
+
+- Added support for ``depobj`` construct.
+
+- Added support for ``detach`` clauses in task-based directives.
+
+- Added support for array shaping operations.
+
+- Added support for cancellation constructs in ``taskloop`` directives.
+
+- Nonmonotonic modifier is allowed with all schedule kinds.
+
+- Added support for ``task`` and ``default`` modifiers in ``reduction`` clauses.
+
+- Added support for strides in array sections.
+
+- Added support for ``use_device_addr`` clause.
+
+- Added support for ``uses_allocators`` clause.
+
+- Added support for ``defaultmap`` clause.
+
+- Added basic support for ``hint`` clause in ``atomic`` directives.
+
+- Added basic support for ``affinity`` clause.
+
+- Added basic support for ``ancestor`` modifier in ``device`` clause.
+
+- Bug fixes and optimizations.
 
 CUDA Support in Clang
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D86562#2237215 , @jdoerfert wrote:

> Nit: Maybe we should mention `complex` support on nvptx devices.

Ok, will add.

> The rest looks good to me. Others might want to add stuff

That was the intention.

> but we can merge this right away (IMHO)

It is up to Hans if he has some time. But I think I covered almost all new 
features (+ complex for NVPTX), so after the update it can be merged with 11.x 
release.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86562

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

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 287776.
ABataev added a comment.

+ complex types for NVPTX


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86562

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,7 +156,49 @@
 OpenMP Support in Clang
 ---
 
-- ...
+New features for OpenMP 5.0 were implemented.
+
+- OpenMP 5.0 is the default version supported by the compiler. User can switch
+  to OpenMP 4.5 using ``-fopenmp-version=45`` option.
+
+- Added support for declare variant directive.
+
+- Improved support of math functions and complex types for NVPTX target.
+
+- Added support for parallel execution of target regions for NVPTX target.
+
+- Addes support for ``scan`` directives and ``inscan`` modifier in 
``reduction``
+  clauses.
+
+- Added support for ``iterator`` construct.
+
+- Added support for ``depobj`` construct.
+
+- Added support for ``detach`` clauses in task-based directives.
+
+- Added support for array shaping operations.
+
+- Added support for cancellation constructs in ``taskloop`` directives.
+
+- Nonmonotonic modifier is allowed with all schedule kinds.
+
+- Added support for ``task`` and ``default`` modifiers in ``reduction`` 
clauses.
+
+- Added support for strides in array sections.
+
+- Added support for ``use_device_addr`` clause.
+
+- Added support for ``uses_allocators`` clause.
+
+- Added support for ``defaultmap`` clause.
+
+- Added basic support for ``hint`` clause in ``atomic`` directives.
+
+- Added basic support for ``affinity`` clause.
+
+- Added basic support for ``ancestor`` modifier in ``device`` clause.
+
+- Bug fixes and optimizations.
 
 CUDA Support in Clang
 -


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,7 +156,49 @@
 OpenMP Support in Clang
 ---
 
-- ...
+New features for OpenMP 5.0 were implemented.
+
+- OpenMP 5.0 is the default version supported by the compiler. User can switch
+  to OpenMP 4.5 using ``-fopenmp-version=45`` option.
+
+- Added support for declare variant directive.
+
+- Improved support of math functions and complex types for NVPTX target.
+
+- Added support for parallel execution of target regions for NVPTX target.
+
+- Addes support for ``scan`` directives and ``inscan`` modifier in ``reduction``
+  clauses.
+
+- Added support for ``iterator`` construct.
+
+- Added support for ``depobj`` construct.
+
+- Added support for ``detach`` clauses in task-based directives.
+
+- Added support for array shaping operations.
+
+- Added support for cancellation constructs in ``taskloop`` directives.
+
+- Nonmonotonic modifier is allowed with all schedule kinds.
+
+- Added support for ``task`` and ``default`` modifiers in ``reduction`` clauses.
+
+- Added support for strides in array sections.
+
+- Added support for ``use_device_addr`` clause.
+
+- Added support for ``uses_allocators`` clause.
+
+- Added support for ``defaultmap`` clause.
+
+- Added basic support for ``hint`` clause in ``atomic`` directives.
+
+- Added basic support for ``affinity`` clause.
+
+- Added basic support for ``ancestor`` modifier in ``device`` clause.
+
+- Bug fixes and optimizations.
 
 CUDA Support in Clang
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

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

rsmith added a comment.

Have you considered using the `ext_vector_type` attribute instead of 
`vector_size` for this? That would have a couple of advantages:

- It's not a GCC attribute, so there's no risk that GCC would give different 
meaning to the same syntax (such as using one byte per vector element, which 
would be the natural meaning for a vector of `bool` as an extension of existing 
functionality).
- `ext_vector_type` takes as an argument a number of elements, not a size in 
bytes, so you could support vectors of non-multiple-of-8 `bool`s.

However, using either an `ext_vector_type` of `bool` or a `vector_size` of 
`bool` would still seem problematic in a couple of other ways:

- Those types always produce a vector whose element type is the specified type; 
you can form an lvalue denoting the element, the element is always represented 
the same as the underlying type, and you can pun between a vector of N `T` and 
an array of N `T`. GCC even permits forming a pointer or reference to a vector 
element, and you should assume that Clang will eventually support that too. 
That would not be possible for `bool` if packed as a bit-vector.
- You can already perform logical operations between our existing vector types 
-- for example, comparing two vectors of 4 `int`s. If vectors of `bool` are 
valid, it would be logical and natural to expect a vector of 4 `bool`s to have 
some connection to the result of such a comparison, but it won't: the type of a 
comparison of two vectors of 4 `int`s is a vector of 4 `int`s each of which is 
either 0 or -1, and we can't convert that to a vector of 4 `bool`s represented 
as our existing vector types, because such conversions would be interpreted as 
bitcasts, and we'd reject because the bitwidth of the vectors is different.
- Vectors of integer types (such as `bool`) generally support arithmetic 
operators, which you presumably do not want to support here.
- Our existing vector types carry a lot of baggage (for example, lax vector 
conversions, and the egregious attribute-on-typedef-changes-the-type hack) that 
we would not want a new type to include.

So I think you should seriously consider using a new syntax to form a 
bit-vector type. It seems to me that an N-bit bit-vector is quite similar to an 
`_ExtInt(N)`, but without the arithmetic support and with a different expected 
IR type. Maybe following that syntactic path and adding `_BitVector(N)` syntax 
would work out better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

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

[PATCH] D86544: [SyntaxTree] Add support for call expressions

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:333
+///   call-arguments:
+/// delimited_list(expression, ',', )
+/// Note: This construct is a simplification of the grammar rule for





Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1070
+Builder.markChildToken(
+std::next(Builder.findToken(S->getCallee()->getEndLoc())),
+syntax::NodeRole::OpenParen);

Could you add an assertion that it is indeed an open paren?

Or, rather, due to the decltype test, it has to be a check, and a FIXME that 
says that the check should become an assertion.



Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:1159
 case syntax::NodeKind::UnknownExpression:
-  return RecursiveASTVisitor::WalkUpFromCXXOperatorCallExpr(S);
+  return WalkUpFromExpr(S);
 default:

I don't understand this change to the unknown case, could you explain?



Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2320
+
+TEST_P(SyntaxTreeTest, CallExpression_Callee_Operator) {
+  if (!GetParam().isCXX()) {





Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2343
+
+TEST_P(SyntaxTreeTest, CallExpression_Callee_OperatorChaining) {
+  if (!GetParam().isCXX()) {





Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2431-2436
+void f();
+void test() {
+  void (*pf)();
+  pf = 
+  [[(*pf)()]];
+}





Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2437
+}
+)cpp",
+  {R"txt(

Please also add a test for `pf()` -- that's allowed, but I believe the AST is 
slightly different from `f()`.



Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2734
+|   |   `-'args'
+|   `-'...'
+`-')' CloseParen

eduucaldas wrote:
> Here there is a divergence between the grammar and the ClangAST. 
> According to the [[ https://eel.is/c++draft/expr.post#nt:expression-list | 
> grammar ]]  `...` would an optional element of `CallArguments`, but here 
> `...` is part of an expression.
> 
> Perhaps I have used the wrong kind of `...`
> `...` would an optional element of CallArguments

I don't think so. Note that every element of initializer-list can have a `...` 
in it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86544

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

[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

No-no, recursive locks are much more complicated than that, please do them 
separately. You'll have to discriminate between a real double lock / double 
unlock bug and an actual valid use of the recursive mutex in order to emit any 
warnings at all. That's completely different checker logic.


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

https://reviews.llvm.org/D85984

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

[PATCH] D85485: Fix quiet mode in git-clang-format

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

I'll need a contact email to preserve authorship information, can you send me 
one at `sguel...@redhat.com`? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85485

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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/tools/driver/driver.cpp:537
+  // When running in integrated-cc1 mode, the CrashRecoveryContext returns
+  // the same code as if the program crashed. On Unix, that is codes >128.
+  IsCrash |= CommandRes > 128;

(From a non-native speaker)

"that is codes >128" Should the singular form be used?



Comment at: llvm/include/llvm/Support/Signals.h:119
+
+  void UnregisterHandlers();
 } // End sys namespace

`[readability-identifier-naming]`

Many very old function use UpperCase, but newer functions should use lowerCase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

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

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

[Johannes Doerfert via Phabricator via cfe-commits]

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

Nit: Maybe we should mention `complex` support on nvptx devices.

The rest looks good to me. Others might want to add stuff but we can merge this 
right away (IMHO)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86562

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

[PATCH] D85485: Fix quiet mode in git-clang-format

[serge via Phabricator via cfe-commits]

serge-sans-paille accepted this revision.
serge-sans-paille added a comment.

Sure, thanks for your patience!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85485

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

[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks amazing now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86293

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

[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:483
+llvm::raw_ostream ) {
+ BR.markInteresting(ThisRegion);
+ OS << "Smart pointer";

Wait, why are we marking the region interesting here? Not every null smart 
pointer is interesting, only the ones that are actually dereferenced.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:497-499
+ OS << "Smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
+ OS << " is non-null";

Also i'm starting to suspect that these notes aren't actually needed when 
there's no "Assuming..."; in particular, we don't emit such notes for raw 
pointers, so we probably shouldn't emit them for smart pointers either. There's 
anyway going to be a note about "Taking true branch..." or something like that, 
which is sufficient to understand what was the smart pointer.

I.e., this note is unnecessary:
```lang=c++
void foo() {
  std::unique_ptr P = nullptr;
  if (P) {// Smart pointer 'P' is null
  // Taking false branch
  }
}
```
The second note, "Taking false branch", is sufficient for explaining to the 
user that the smart pointer is known to be null. The user may ask why the smart 
pointer is null, but the note isn't explaining it. The way you marked the 
region as interesting (as i noticed in the above inline comment) would have 
indeed explained why it's null, but at this point we draw the line and believe 
that if the region isn't already interesting then the user most likely doesn't 
need to know why it's null (and if it's already interesting then there's no 
need to mark it again).

But this note is necessary:
```lang=c++
void foo(std::unique_ptr P) {
  if (P) {// Assuming smart pointer 'P' is null
  // Taking false branch
  }
}
```
This note conveys the extra information that we don't know for a fact that the 
smart pointer is null on the current path based on previous analysis, but 
instead the analyzer is *assuming* that it's possible that it's null due to the 
presence of the check in the code. That's a big deal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86027

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

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG97ccf93b3615: [SystemZ][z/OS] Add z/OS Target and define 
macros (authored by abhina.sreeskantharajan, committed by 
hubert.reinterpretcast).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/test/Preprocessor/init-zos.c


Index: clang/test/Preprocessor/init-zos.c
===
--- /dev/null
+++ clang/test/Preprocessor/init-zos.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
S390X-ZOS %s
+// RUN: %clang_cc1 -x c++ -std=gnu++14 -E -dM -ffreestanding 
-triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-GNUXX %s
+
+// S390X-ZOS-GNUXX:#define _EXT 1
+// S390X-ZOS:#define _LONG_LONG 1
+// S390X-ZOS-GNUXX:#define _MI_BUILTIN 1
+// S390X-ZOS:#define _OPEN_DEFAULT 1
+// S390X-ZOS:#define _UNIX03_WITHDRAWN 1
+// S390X-ZOS-GNUXX:#define _XOPEN_SOURCE 600
+// S390X-ZOS:#define __370__ 1
+// S390X-ZOS:#define __64BIT__ 1
+// S390X-ZOS:#define __BFP__ 1
+// S390X-ZOS:#define __BOOL__ 1
+// S390X-ZOS-GNUXX:#define __DLL__ 1
+// S390X-ZOS:#define __LONGNAME__ 1
+// S390X-ZOS:#define __MVS__ 1
+// S390X-ZOS:#define __THW_370__ 1
+// S390X-ZOS:#define __THW_BIG_ENDIAN__ 1
+// S390X-ZOS:#define __TOS_390__ 1
+// S390X-ZOS:#define __TOS_MVS__ 1
+// S390X-ZOS:#define __XPLINK__ 1
+// S390X-ZOS-GNUXX:#define __wchar_t 1
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -728,6 +728,55 @@
   bool defaultsToAIXPowerAlignment() const override { return true; }
 };
 
+// z/OS target
+template 
+class LLVM_LIBRARY_VISIBILITY ZOSTargetInfo : public OSTargetInfo {
+protected:
+  void getOSDefines(const LangOptions , const llvm::Triple ,
+MacroBuilder ) const override {
+// FIXME: _LONG_LONG should not be defined under -std=c89.
+Builder.defineMacro("_LONG_LONG");
+Builder.defineMacro("_OPEN_DEFAULT");
+// _UNIX03_WITHDRAWN is required to build libcxx.
+Builder.defineMacro("_UNIX03_WITHDRAWN");
+Builder.defineMacro("__370__");
+Builder.defineMacro("__BFP__");
+// FIXME: __BOOL__ should not be defined under -std=c89.
+Builder.defineMacro("__BOOL__");
+Builder.defineMacro("__LONGNAME__");
+Builder.defineMacro("__MVS__");
+Builder.defineMacro("__THW_370__");
+Builder.defineMacro("__THW_BIG_ENDIAN__");
+Builder.defineMacro("__TOS_390__");
+Builder.defineMacro("__TOS_MVS__");
+Builder.defineMacro("__XPLINK__");
+
+if (this->PointerWidth == 64)
+  Builder.defineMacro("__64BIT__");
+
+if (Opts.CPlusPlus) {
+  Builder.defineMacro("__DLL__");
+  // _XOPEN_SOURCE=600 is required to build libcxx.
+  Builder.defineMacro("_XOPEN_SOURCE", "600");
+}
+
+if (Opts.GNUMode) {
+  Builder.defineMacro("_MI_BUILTIN");
+  Builder.defineMacro("_EXT");
+}
+
+if (Opts.CPlusPlus && Opts.WChar) {
+  // Macro __wchar_t is defined so that the wchar_t data
+  // type is not declared as a typedef in system headers.
+  Builder.defineMacro("__wchar_t");
+}
+  }
+
+public:
+  ZOSTargetInfo(const llvm::Triple , const TargetOptions )
+  : OSTargetInfo(Triple, Opts) {}
+};
+
 void addWindowsDefines(const llvm::Triple , const LangOptions ,
MacroBuilder );
 
Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -452,6 +452,8 @@
 switch (os) {
 case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
+case llvm::Triple::ZOS:
+  return new ZOSTargetInfo(Triple, Opts);
 default:
   return new SystemZTargetInfo(Triple, Opts);
 }


Index: clang/test/Preprocessor/init-zos.c
===
--- /dev/null
+++ clang/test/Preprocessor/init-zos.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS %s
+// RUN: %clang_cc1 -x c++ -std=gnu++14 -E -dM -ffreestanding -triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-GNUXX %s
+
+// S390X-ZOS-GNUXX:#define _EXT 1
+// S390X-ZOS:#define _LONG_LONG 1
+// S390X-ZOS-GNUXX:#define _MI_BUILTIN 1
+// S390X-ZOS:#define _OPEN_DEFAULT 1
+// S390X-ZOS:#define _UNIX03_WITHDRAWN 1
+// S390X-ZOS-GNUXX:#define _XOPEN_SOURCE 600
+// 

[clang] 97ccf93 - [SystemZ][z/OS] Add z/OS Target and define macros

[Hubert Tong via cfe-commits]

Author: Abhina Sreeskantharajan
Date: 2020-08-25T15:51:59-04:00
New Revision: 97ccf93b3615ff4c0d5fe116e6a7c7b616d8ec0c

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

LOG: [SystemZ][z/OS] Add z/OS Target and define macros

This patch adds the z/OS target and defines macros as a stepping stone
towards enabling a native build on z/OS.

Reviewed By: hubert.reinterpretcast

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

Added: 
clang/test/Preprocessor/init-zos.c

Modified: 
clang/lib/Basic/Targets.cpp
clang/lib/Basic/Targets/OSTargets.h

Removed: 




diff  --git a/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index 38a388afa534..933b7b342891 100644
--- a/clang/lib/Basic/Targets.cpp
+++ b/clang/lib/Basic/Targets.cpp
@@ -452,6 +452,8 @@ TargetInfo *AllocateTarget(const llvm::Triple ,
 switch (os) {
 case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
+case llvm::Triple::ZOS:
+  return new ZOSTargetInfo(Triple, Opts);
 default:
   return new SystemZTargetInfo(Triple, Opts);
 }

diff  --git a/clang/lib/Basic/Targets/OSTargets.h 
b/clang/lib/Basic/Targets/OSTargets.h
index a2c0fd42f26d..9c206fc7e6a4 100644
--- a/clang/lib/Basic/Targets/OSTargets.h
+++ b/clang/lib/Basic/Targets/OSTargets.h
@@ -728,6 +728,55 @@ class AIXTargetInfo : public OSTargetInfo {
   bool defaultsToAIXPowerAlignment() const override { return true; }
 };
 
+// z/OS target
+template 
+class LLVM_LIBRARY_VISIBILITY ZOSTargetInfo : public OSTargetInfo {
+protected:
+  void getOSDefines(const LangOptions , const llvm::Triple ,
+MacroBuilder ) const override {
+// FIXME: _LONG_LONG should not be defined under -std=c89.
+Builder.defineMacro("_LONG_LONG");
+Builder.defineMacro("_OPEN_DEFAULT");
+// _UNIX03_WITHDRAWN is required to build libcxx.
+Builder.defineMacro("_UNIX03_WITHDRAWN");
+Builder.defineMacro("__370__");
+Builder.defineMacro("__BFP__");
+// FIXME: __BOOL__ should not be defined under -std=c89.
+Builder.defineMacro("__BOOL__");
+Builder.defineMacro("__LONGNAME__");
+Builder.defineMacro("__MVS__");
+Builder.defineMacro("__THW_370__");
+Builder.defineMacro("__THW_BIG_ENDIAN__");
+Builder.defineMacro("__TOS_390__");
+Builder.defineMacro("__TOS_MVS__");
+Builder.defineMacro("__XPLINK__");
+
+if (this->PointerWidth == 64)
+  Builder.defineMacro("__64BIT__");
+
+if (Opts.CPlusPlus) {
+  Builder.defineMacro("__DLL__");
+  // _XOPEN_SOURCE=600 is required to build libcxx.
+  Builder.defineMacro("_XOPEN_SOURCE", "600");
+}
+
+if (Opts.GNUMode) {
+  Builder.defineMacro("_MI_BUILTIN");
+  Builder.defineMacro("_EXT");
+}
+
+if (Opts.CPlusPlus && Opts.WChar) {
+  // Macro __wchar_t is defined so that the wchar_t data
+  // type is not declared as a typedef in system headers.
+  Builder.defineMacro("__wchar_t");
+}
+  }
+
+public:
+  ZOSTargetInfo(const llvm::Triple , const TargetOptions )
+  : OSTargetInfo(Triple, Opts) {}
+};
+
 void addWindowsDefines(const llvm::Triple , const LangOptions ,
MacroBuilder );
 

diff  --git a/clang/test/Preprocessor/init-zos.c 
b/clang/test/Preprocessor/init-zos.c
new file mode 100644
index ..50c4ed9e539e
--- /dev/null
+++ b/clang/test/Preprocessor/init-zos.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
S390X-ZOS %s
+// RUN: %clang_cc1 -x c++ -std=gnu++14 -E -dM -ffreestanding 
-triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-GNUXX %s
+
+// S390X-ZOS-GNUXX:#define _EXT 1
+// S390X-ZOS:#define _LONG_LONG 1
+// S390X-ZOS-GNUXX:#define _MI_BUILTIN 1
+// S390X-ZOS:#define _OPEN_DEFAULT 1
+// S390X-ZOS:#define _UNIX03_WITHDRAWN 1
+// S390X-ZOS-GNUXX:#define _XOPEN_SOURCE 600
+// S390X-ZOS:#define __370__ 1
+// S390X-ZOS:#define __64BIT__ 1
+// S390X-ZOS:#define __BFP__ 1
+// S390X-ZOS:#define __BOOL__ 1
+// S390X-ZOS-GNUXX:#define __DLL__ 1
+// S390X-ZOS:#define __LONGNAME__ 1
+// S390X-ZOS:#define __MVS__ 1
+// S390X-ZOS:#define __THW_370__ 1
+// S390X-ZOS:#define __THW_BIG_ENDIAN__ 1
+// S390X-ZOS:#define __TOS_390__ 1
+// S390X-ZOS:#define __TOS_MVS__ 1
+// S390X-ZOS:#define __XPLINK__ 1
+// S390X-ZOS-GNUXX:#define __wchar_t 1



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

[PATCH] D82582: [SVE] Remove calls to VectorType::getNumElements from clang

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau updated this revision to Diff 287755.
ctetreau marked 4 inline comments as done.
ctetreau added a comment.

address code review issues, rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82582

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp

Index: clang/lib/CodeGen/SwiftCallingConv.cpp
===
--- clang/lib/CodeGen/SwiftCallingConv.cpp
+++ clang/lib/CodeGen/SwiftCallingConv.cpp
@@ -320,9 +320,12 @@
   // If we have a vector type, split it.
   if (auto vecTy = dyn_cast_or_null(type)) {
 auto eltTy = vecTy->getElementType();
-CharUnits eltSize = (end - begin) / vecTy->getNumElements();
+CharUnits eltSize =
+(end - begin) / cast(vecTy)->getNumElements();
 assert(eltSize == getTypeStoreSize(CGM, eltTy));
-for (unsigned i = 0, e = vecTy->getNumElements(); i != e; ++i) {
+for (unsigned i = 0,
+  e = cast(vecTy)->getNumElements();
+ i != e; ++i) {
   addEntry(eltTy, begin, begin + eltSize);
   begin += eltSize;
 }
@@ -674,8 +677,9 @@
 
 bool swiftcall::isLegalVectorType(CodeGenModule , CharUnits vectorSize,
   llvm::VectorType *vectorTy) {
-  return isLegalVectorType(CGM, vectorSize, vectorTy->getElementType(),
-   vectorTy->getNumElements());
+  return isLegalVectorType(
+  CGM, vectorSize, vectorTy->getElementType(),
+  cast(vectorTy)->getNumElements());
 }
 
 bool swiftcall::isLegalVectorType(CodeGenModule , CharUnits vectorSize,
@@ -688,7 +692,7 @@
 std::pair
 swiftcall::splitLegalVectorType(CodeGenModule , CharUnits vectorSize,
 llvm::VectorType *vectorTy) {
-  auto numElts = vectorTy->getNumElements();
+  auto numElts = cast(vectorTy)->getNumElements();
   auto eltTy = vectorTy->getElementType();
 
   // Try to split the vector type in half.
@@ -710,7 +714,7 @@
   }
 
   // Try to split the vector into legal subvectors.
-  auto numElts = origVectorTy->getNumElements();
+  auto numElts = cast(origVectorTy)->getNumElements();
   auto eltTy = origVectorTy->getElementType();
   assert(numElts != 1);
 
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1320,7 +1320,7 @@
"Splatted expr doesn't match with vector element type?");
 
 // Splat the element across to all elements
-unsigned NumElements = cast(DstTy)->getNumElements();
+unsigned NumElements = cast(DstTy)->getNumElements();
 return Builder.CreateVectorSplat(NumElements, Src, "splat");
   }
 
@@ -1553,12 +1553,12 @@
 Value *RHS = CGF.EmitScalarExpr(E->getExpr(1));
 Value *Mask;
 
-llvm::VectorType *LTy = cast(LHS->getType());
+auto *LTy = cast(LHS->getType());
 unsigned LHSElts = LTy->getNumElements();
 
 Mask = RHS;
 
-llvm::VectorType *MTy = cast(Mask->getType());
+auto *MTy = cast(Mask->getType());
 
 // Mask off the high bits of each shuffle index.
 Value *MaskBits =
@@ -1763,7 +1763,7 @@
 return Visit(E->getInit(0));
   }
 
-  unsigned ResElts = VType->getNumElements();
+  unsigned ResElts = cast(VType)->getNumElements();
 
   // Loop over initializers collecting the Value for each, and remembering
   // whether the source was swizzle (ExtVectorElementExpr).  This will allow
@@ -1787,7 +1787,8 @@
   if (isa(IE)) {
 llvm::ExtractElementInst *EI = cast(Init);
 
-if (EI->getVectorOperandType()->getNumElements() == ResElts) {
+if (cast(EI->getVectorOperandType())
+->getNumElements() == ResElts) {
   llvm::ConstantInt *C = cast(EI->getIndexOperand());
   Value *LHS = nullptr, *RHS = nullptr;
   if (CurIdx == 0) {
@@ -1825,7 +1826,7 @@
   continue;
 }
 
-unsigned InitElts = VVT->getNumElements();
+unsigned InitElts = cast(VVT)->getNumElements();
 
 // If the initializer is an ExtVecEltExpr (a swizzle), and the swizzle's
 // input is the same width as the vector being constructed, generate an
@@ -1834,7 +1835,7 @@
 if (isa(IE)) {
   llvm::ShuffleVectorInst *SVI = cast(Init);
   Value *SVOp = SVI->getOperand(0);
-  llvm::VectorType *OpTy = cast(SVOp->getType());
+  auto *OpTy = cast(SVOp->getType());
 
   if (OpTy->getNumElements() == ResElts) {
 for (unsigned j = 0; j != CurIdx; ++j) {
@@ -2170,7 +2171,7 @@
 llvm::Type *DstTy = ConvertType(DestTy);
 Value *Elt = Visit(const_cast(E));
 // Splat the element across to all elements
-unsigned NumElements = cast(DstTy)->getNumElements();
+unsigned NumElements = cast(DstTy)->getNumElements();
 return 

[clang] b1009ee - Reland "[DebugInfo] Move constructor homing case in shouldOmitDefinition."

[Amy Huang via cfe-commits]

Author: Amy Huang
Date: 2020-08-25T12:36:11-07:00
New Revision: b1009ee84fc0242bcebd07889306bf39d9b7170f

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

LOG: Reland "[DebugInfo] Move constructor homing case in shouldOmitDefinition."

For some reason the ctor homing case was before the template
specialization case, and could have returned false too early.
I moved the code out into a separate function to avoid this.

This reverts commit 05777ab941063192b9ccb1775358a83a2700ccc1.

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index e3442ecd4bd5..36bab9c22d43 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2260,6 +2260,25 @@ static bool 
hasExplicitMemberDefinition(CXXRecordDecl::method_iterator I,
   return false;
 }
 
+static bool canUseCtorHoming(const CXXRecordDecl *RD) {
+  // Constructor homing can be used for classes that have at least one
+  // constructor and have no trivial or constexpr constructors.
+  // Skip this optimization if the class or any of its methods are marked
+  // dllimport.
+  if (RD->isLambda() || RD->hasConstexprNonCopyMoveConstructor() ||
+  isClassOrMethodDLLImport(RD))
+return false;
+
+  if (RD->ctors().empty())
+return false;
+
+  for (const auto *Ctor : RD->ctors())
+if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
+  return false;
+
+  return true;
+}
+
 static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
  bool DebugTypeExtRefs, const RecordDecl *RD,
  const LangOptions ) {
@@ -2294,23 +2313,6 @@ static bool 
shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
   !isClassOrMethodDLLImport(CXXDecl))
 return true;
 
-  // In constructor debug mode, only emit debug info for a class when its
-  // constructor is emitted. Skip this optimization if the class or any of
-  // its methods are marked dllimport.
-  //
-  // This applies to classes that don't have any trivial constructors and have
-  // at least one constructor.
-  if (DebugKind == codegenoptions::DebugInfoConstructor &&
-  !CXXDecl->isLambda() && !CXXDecl->hasConstexprNonCopyMoveConstructor() &&
-  !isClassOrMethodDLLImport(CXXDecl)) {
-if (CXXDecl->ctors().empty())
-  return false;
-for (const auto *Ctor : CXXDecl->ctors())
-  if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
-return false;
-return true;
-  }
-
   TemplateSpecializationKind Spec = TSK_Undeclared;
   if (const auto *SD = dyn_cast(RD))
 Spec = SD->getSpecializationKind();
@@ -2320,6 +2322,12 @@ static bool 
shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
   CXXDecl->method_end()))
 return true;
 
+  // In constructor homing mode, only emit complete debug info for a class
+  // when its constructor is emitted.
+  if ((DebugKind == codegenoptions::DebugInfoConstructor) &&
+  canUseCtorHoming(CXXDecl))
+return true;
+
   return false;
 }
 

diff  --git 
a/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp 
b/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
index 4e41c4092bf4..b756674f54c4 100644
--- a/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
+++ b/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=limited %s -o - | FileCheck %s
 
+// Make sure this still works with constructor homing.
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=constructor %s -o - | FileCheck %s
+
 // Run again with -gline-tables-only or -gline-directives-only and verify we 
don't crash.  We won't output
 // type info at all.
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=line-tables-only %s -o - | FileCheck %s -check-prefix 
LINES-ONLY



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

[PATCH] D86491: [DebugInfo] Move constructor homing case in shouldOmitDefinition.

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 287748.
akhuang added a comment.

Fix errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86491

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp


Index: clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
+++ clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=limited %s -o - | FileCheck %s
 
+// Make sure this still works with constructor homing.
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=constructor %s -o - | FileCheck %s
+
 // Run again with -gline-tables-only or -gline-directives-only and verify we 
don't crash.  We won't output
 // type info at all.
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=line-tables-only %s -o - | FileCheck %s -check-prefix 
LINES-ONLY
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2260,6 +2260,25 @@
   return false;
 }
 
+static bool canUseCtorHoming(const CXXRecordDecl *RD) {
+  // Constructor homing can be used for classes that have at least one
+  // constructor and have no trivial or constexpr constructors.
+  // Skip this optimization if the class or any of its methods are marked
+  // dllimport.
+  if (RD->isLambda() || RD->hasConstexprNonCopyMoveConstructor() ||
+  isClassOrMethodDLLImport(RD))
+return false;
+
+  if (RD->ctors().empty())
+return false;
+
+  for (const auto *Ctor : RD->ctors())
+if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
+  return false;
+
+  return true;
+}
+
 static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
  bool DebugTypeExtRefs, const RecordDecl *RD,
  const LangOptions ) {
@@ -2294,23 +2313,6 @@
   !isClassOrMethodDLLImport(CXXDecl))
 return true;
 
-  // In constructor debug mode, only emit debug info for a class when its
-  // constructor is emitted. Skip this optimization if the class or any of
-  // its methods are marked dllimport.
-  //
-  // This applies to classes that don't have any trivial constructors and have
-  // at least one constructor.
-  if (DebugKind == codegenoptions::DebugInfoConstructor &&
-  !CXXDecl->isLambda() && !CXXDecl->hasConstexprNonCopyMoveConstructor() &&
-  !isClassOrMethodDLLImport(CXXDecl)) {
-if (CXXDecl->ctors().empty())
-  return false;
-for (const auto *Ctor : CXXDecl->ctors())
-  if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
-return false;
-return true;
-  }
-
   TemplateSpecializationKind Spec = TSK_Undeclared;
   if (const auto *SD = dyn_cast(RD))
 Spec = SD->getSpecializationKind();
@@ -2320,6 +2322,12 @@
   CXXDecl->method_end()))
 return true;
 
+  // In constructor homing mode, only emit complete debug info for a class
+  // when its constructor is emitted.
+  if ((DebugKind == codegenoptions::DebugInfoConstructor) &&
+  canUseCtorHoming(CXXDecl))
+return true;
+
   return false;
 }
 


Index: clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
+++ clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-kind=limited %s -o - | FileCheck %s
 
+// Make sure this still works with constructor homing.
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-kind=constructor %s -o - | FileCheck %s
+
 // Run again with -gline-tables-only or -gline-directives-only and verify we don't crash.  We won't output
 // type info at all.
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-kind=line-tables-only %s -o - | FileCheck %s -check-prefix LINES-ONLY
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2260,6 +2260,25 @@
   return false;
 }
 
+static bool canUseCtorHoming(const CXXRecordDecl *RD) {
+  // Constructor homing can be used for classes that have at least one
+  // constructor and have no trivial or constexpr constructors.
+  // Skip this optimization if the class or any of its methods are marked
+  // dllimport.
+  if (RD->isLambda() || RD->hasConstexprNonCopyMoveConstructor() ||
+  

[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 287749.
vsapsai added a comment.

Tweak the error text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84458

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Modules/diagnostics.modulemap


Index: clang/test/Modules/diagnostics.modulemap
===
--- clang/test/Modules/diagnostics.modulemap
+++ clang/test/Modules/diagnostics.modulemap
@@ -28,3 +28,9 @@
   header "quux.h" { size 1 mtime 2 }
   header "no_attrs.h" {}
 }
+
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:8: error: no module named 
'unknown' found, parent module must be defined before the submodule
+module unknown.submodule {}
+module known_top_level {}
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:24: error: no module named 
'unknown' in 'known_top_level', parent module must be defined before the 
submodule
+module known_top_level.unknown.submodule {}
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1903,18 +1903,16 @@
 continue;
   }
 
-  if (ActiveModule) {
-Diags.Report(Id[I].second, diag::err_mmap_missing_module_qualified)
-  << Id[I].first
-  << ActiveModule->getTopLevelModule()->getFullModuleName();
-  } else {
-Diags.Report(Id[I].second, diag::err_mmap_expected_module_name);
-  }
+  Diags.Report(Id[I].second, diag::err_mmap_missing_parent_module)
+  << Id[I].first << (ActiveModule != nullptr)
+  << (ActiveModule
+  ? ActiveModule->getTopLevelModule()->getFullModuleName()
+  : "");
   HadError = true;
-  return;
 }
 
-if (ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
+if (TopLevelModule &&
+ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
   assert(ModuleMapFile != Map.getModuleMapFileForUniquing(TopLevelModule) 
&&
  "submodule defined in same file as 'module *' that allowed its "
  "top-level module");
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -695,6 +695,9 @@
   "no module named '%0' visible from '%1'">;
 def err_mmap_missing_module_qualified : Error<
   "no module named '%0' in '%1'">;
+def err_mmap_missing_parent_module: Error<
+  "no module named '%0' %select{found|in '%2'}1, "
+  "parent module must be defined before the submodule">;
 def err_mmap_top_level_inferred_submodule : Error<
   "only submodules and framework modules may be inferred with wildcard 
syntax">;
 def err_mmap_inferred_no_umbrella : Error<


Index: clang/test/Modules/diagnostics.modulemap
===
--- clang/test/Modules/diagnostics.modulemap
+++ clang/test/Modules/diagnostics.modulemap
@@ -28,3 +28,9 @@
   header "quux.h" { size 1 mtime 2 }
   header "no_attrs.h" {}
 }
+
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:8: error: no module named 'unknown' found, parent module must be defined before the submodule
+module unknown.submodule {}
+module known_top_level {}
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:24: error: no module named 'unknown' in 'known_top_level', parent module must be defined before the submodule
+module known_top_level.unknown.submodule {}
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1903,18 +1903,16 @@
 continue;
   }
 
-  if (ActiveModule) {
-Diags.Report(Id[I].second, diag::err_mmap_missing_module_qualified)
-  << Id[I].first
-  << ActiveModule->getTopLevelModule()->getFullModuleName();
-  } else {
-Diags.Report(Id[I].second, diag::err_mmap_expected_module_name);
-  }
+  Diags.Report(Id[I].second, diag::err_mmap_missing_parent_module)
+  << Id[I].first << (ActiveModule != nullptr)
+  << (ActiveModule
+  ? ActiveModule->getTopLevelModule()->getFullModuleName()
+  : "");
   HadError = true;
-  return;
 }
 
-if (ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
+if (TopLevelModule &&
+ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
   assert(ModuleMapFile != Map.getModuleMapFileForUniquing(TopLevelModule) &&
  "submodule defined in same file as 'module *' that allowed its "
  "top-level module");
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- 

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

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 287747.
aganea marked an inline comment as done.
aganea added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Rebase.
Address @MaskRay's suggestions.
Made `safeLldLink` a public API and made it possible to provide user-defined 
stdout/stderr streams.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

Files:
  clang/tools/driver/driver.cpp
  lld/COFF/Driver.cpp
  lld/COFF/Writer.cpp
  lld/COFF/Writer.h
  lld/Common/ErrorHandler.cpp
  lld/ELF/Driver.cpp
  lld/MachO/Driver.cpp
  lld/include/lld/Common/Driver.h
  lld/include/lld/Common/ErrorHandler.h
  lld/lib/Driver/DarwinLdDriver.cpp
  lld/test/COFF/dll.test
  lld/test/COFF/guardcf-lto.ll
  lld/test/COFF/lit.local.cfg
  lld/tools/lld/lld.cpp
  lld/wasm/Driver.cpp
  llvm/include/llvm/Support/CrashRecoveryContext.h
  llvm/include/llvm/Support/Signals.h
  llvm/lib/Support/CrashRecoveryContext.cpp
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/Signals.inc

Index: llvm/lib/Support/Windows/Signals.inc
===
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -868,3 +868,5 @@
  #pragma GCC diagnostic warning "-Wformat"
  #pragma GCC diagnostic warning "-Wformat-extra-args"
 #endif
+
+void sys::UnregisterHandlers() {}
Index: llvm/lib/Support/Unix/Signals.inc
===
--- llvm/lib/Support/Unix/Signals.inc
+++ llvm/lib/Support/Unix/Signals.inc
@@ -331,7 +331,7 @@
 registerHandler(S, SignalKind::IsInfo);
 }
 
-static void UnregisterHandlers() {
+void sys::UnregisterHandlers() {
   // Restore all of the signal handlers to how they were before we showed up.
   for (unsigned i = 0, e = NumRegisteredSignals.load(); i != e; ++i) {
 sigaction(RegisteredSignalInfo[i].SigNo,
@@ -367,7 +367,7 @@
   // crashes when we return and the signal reissues.  This also ensures that if
   // we crash in our signal handler that the program will terminate immediately
   // instead of recursing in the signal handler.
-  UnregisterHandlers();
+  sys::UnregisterHandlers();
 
   // Unmask all potentially blocked kill signals.
   sigset_t SigMask;
Index: llvm/lib/Support/CrashRecoveryContext.cpp
===
--- llvm/lib/Support/CrashRecoveryContext.cpp
+++ llvm/lib/Support/CrashRecoveryContext.cpp
@@ -97,6 +97,13 @@
 
 CrashRecoveryContextCleanup::~CrashRecoveryContextCleanup() {}
 
+CrashRecoveryContext::CrashRecoveryContext() {
+  // On Windows, if abort() was previously triggered (and caught by a previous
+  // CrashRecoveryContext) the Windows CRT removes our installed signal handler,
+  // so we need to install it again.
+  sys::DisableSystemDialogsOnCrash();
+}
+
 CrashRecoveryContext::~CrashRecoveryContext() {
   // Reclaim registered resources.
   CrashRecoveryContextCleanup *i = head;
@@ -370,9 +377,10 @@
   sigaddset(, Signal);
   sigprocmask(SIG_UNBLOCK, , nullptr);
 
-  // As per convention, -2 indicates a crash or timeout as opposed to failure to
-  // execute (see llvm/include/llvm/Support/Program.h)
-  int RetCode = -2;
+  // Return the same error code as if the program crashed, as mentioned in the
+  // section "Exit Status for Commands":
+  // https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
+  int RetCode = 128 + Signal;
 
   // Don't consider a broken pipe as a crash (see clang/lib/Driver/Driver.cpp)
   if (Signal == SIGPIPE)
Index: llvm/include/llvm/Support/Signals.h
===
--- llvm/include/llvm/Support/Signals.h
+++ llvm/include/llvm/Support/Signals.h
@@ -115,6 +115,8 @@
   /// Context is a system-specific failure context: it is the signal type on
   /// Unix; the ExceptionContext on Windows.
   void CleanupOnSignal(uintptr_t Context);
+
+  void UnregisterHandlers();
 } // End sys namespace
 } // End llvm namespace
 
Index: llvm/include/llvm/Support/CrashRecoveryContext.h
===
--- llvm/include/llvm/Support/CrashRecoveryContext.h
+++ llvm/include/llvm/Support/CrashRecoveryContext.h
@@ -44,11 +44,11 @@
 /// executed in any case, whether crash occurs or not. These actions may be used
 /// to reclaim resources in the case of crash.
 class CrashRecoveryContext {
-  void *Impl;
-  CrashRecoveryContextCleanup *head;
+  void *Impl = nullptr;
+  CrashRecoveryContextCleanup *head = nullptr;
 
 public:
-  CrashRecoveryContext() : Impl(nullptr), head(nullptr) {}
+  CrashRecoveryContext();
   ~CrashRecoveryContext();
 
   /// Register cleanup handler, which is used when the recovery context is
Index: lld/wasm/Driver.cpp
===
--- lld/wasm/Driver.cpp
+++ lld/wasm/Driver.cpp
@@ -85,6 +85,8 @@
   

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D86559#2236931 , @Quuxplusone wrote:

> It seems like this patch would basically "copy" the `[[likely]]` attribute 
> from line C up to lines A and B, where it would reinforce the likelihood of 
> path A and (maybe?) "cancel out" the unlikelihood of path B, without actually 
> saying anything specifically about the likelihood of label C (which is surely 
> what the programmer intended by applying the attribute, right?).

Yes A is double `[[likely]]` and B is both `[[likely]]` and `[[unlikely]]`.  
Based on http://eel.is/c++draft/dcl.attr.likelihood#:attribute,likely
"The use of the likely attribute is intended to allow implementations to 
optimize for the case where paths of execution including it are arbitrarily 
more likely than any alternative path of execution that does not include such 
an attribute on a statement or label." 
So it seem it's intended to see a goto as a part of a path of execution, since 
it's an unconditional jump.

But I think the standard should be improved:

  if(foo) {
[[likely]][[unlikely]] bar(1); // error: The attribute-token likely shall 
not appear in an
bar(2);  //  attribute-specifier-seq 
that contains the attribute-token unlikely.
  }
  if(foo) {
[[likely]] bar(1); 
[[unlikely]] bar(2);// This contradiction in the `ThenStmt` 
is allowed
  }
  if(foo) {
[[likely]] bar(1);
  } else {
[[likely]] bar(2);   // This contradiction between the 
`ThenStmt` and `ElseStmt` is allowed
  }

So I'll work on a paper to discuss these issues. I already have some notes, but 
I would prefer to get more implementation experience before starting to write.
IMO we should ignore conflicting likelihoods and issue a diagnostic. (For a 
switch multiple `[[likely]]` cases would be allowed, but still no mixing.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559

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

[PATCH] D86065: [SVE] Make ElementCount members private

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau added inline comments.



Comment at: llvm/include/llvm/Support/TypeSize.h:56
 
+  friend bool operator>(const ElementCount , const ElementCount ) {
+assert(LHS.Scalable == RHS.Scalable &&

fpetrogalli wrote:
> paulwalker-arm wrote:
> > david-arm wrote:
> > > ctetreau wrote:
> > > > fpetrogalli wrote:
> > > > > I think that @ctetreau is right on 
> > > > > https://reviews.llvm.org/D85794#inline-793909. We should not overload 
> > > > > a comparison operator on this class because the set it represent it 
> > > > > cannot be ordered.
> > > > > 
> > > > > Chris suggests an approach of writing a static function that can be 
> > > > > used as a comparison operator,  so that we can make it explicit of 
> > > > > what kind of comparison we  are doing. 
> > > > In C++, it's common to overload the comparison operators for the 
> > > > purposes of being able to std::sort and use ordered sets. Normally, I 
> > > > would be OK with such usages. However, since `ElementCount` is 
> > > > basically a numeric type, and they only have a partial ordering, I 
> > > > think this is dangerous. I'm concerned that this will result in more 
> > > > bugs whereby somebody didn't remember that vectors can be scalable.
> > > > 
> > > > I don't have a strong opinion what the comparator function should be 
> > > > called, but I strongly prefer that it not be a comparison operator.
> > > Hi @ctetreau, yeah I understand. The reason I chose to use operators was 
> > > simply to be consistent with what we have already in TypeSize. Also, we 
> > > have existing "==" and "!=" operators in ElementCount too, although these 
> > > are essentially testing that two ElementCounts are identically the same 
> > > or not, i.e. for 2 given polynomials (a + bx) and (c + dx) we're 
> > > essentially asking if both a==c and b==d.
> > > 
> > > If I introduce a new comparison function, I'll probably keep the asserts 
> > > in for now, but in general we can do better than simply asserting if 
> > > something is scalable or not. For example, we know that (vscale * 4) is 
> > > definitely >= 4 because vscale is at least 1. I'm just not sure if we 
> > > have that need yet.
> > I think we should treat the non-equality comparison functions more like 
> > floating point.  What we don't want is somebody writing !GreaterThan when 
> > they actually mean LessThan.
> > 
> > Perhaps we should name the functions accordingly (i.e. ogt for 
> > OrderedAndGreaterThan).  We will also need matching less than functions 
> > since I can see those being useful when analysing constant insert/extract 
> > element indices which stand a good chance to be a known comparison (with 0 
> > being the most common index).
> > 
> May I suggest the following name scheme? (my 2 c, will not hold the patch for 
> not addressing this comment)
> 
> ```
> static bool [Non]Total(...)
> ```
> 
> with `` being 
> 
> * `LT` -> less than, aka `<`
> * `LToE` -> less than or equal, aka "<="
> * `GT` -> greater than, aka ">"
> * `GToE` -> greater than or equal, aka ">="
> 
> and `Total` , `NonTotal` being the prefix that gives information about the 
> behavior on the value of scalable:
> 
> `Total` -> for example, all scalable ECs are bigger than fixed ECs.
> `NonTotal` -> asserting on `(LHS.Scalable == RHS.Scalable)` before returning 
> `LHS.Min  RHS.Min`.
> 
> Taking it further: it could also be a template on an enumeration that list 
> the type of comparisons?
> 
> ```
> enum CMPType {
>   TotalGT,
>   NonTotalLT,
>   fancy_one
> };
> 
> ...
> 
> template 
> static bool cmp(ElementCount , ElementCount  );
> 
> ...
> 
> static bool ElementCount::cmp(ElementCount 
> , ElementCount  ) {
>  /// implementation
> }
> static bool ElementCount::cmp(ElementCount 
> , ElementCount  ) {
>  /// implementation
> }
> ```
> 
> 
Honestly, I think this is actually worse. My issue is the fact that, from a 
mathematical perspective, `vscale_1 * min_1 < vscale_2 * min_2` is a function 
of `vscale_1` and `vscale_2`. In principle, we can know some ordering 
relationships between certain element counts (such as `vscale * min_1 >= min_2 
= true`), but in general, this function does not make sense. However, an 
`operator<` is useful because it allows you to put an `ElementCount` into an 
ordered set, and it will just work. Renaming the function to `olt` just makes 
it so that you can't put `ElementCount`s into an ordered set, but still implies 
that `ElementCount`s are comparable in general. This will also blow up if you 
actually try to mix fixed width and scalable `ElementCount`s in an ordered set, 
which should work IMO.

Here is what I propose:

1) Add a predicate for establishing an arbitrary ordering. The predicate would 
be completely arbitrary, because it's only useful for establishing an ordering 
for an ordered set or in a sorting algorithm. It could look something like this:

```
static bool orderedBefore(const ElementCount , const ElementCount ) {
  auto l = 

[PATCH] D86491: [DebugInfo] Move constructor homing case in shouldOmitDefinition.

[Amy Huang via Phabricator via cfe-commits]

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

just reopening to update the diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86491

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

[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

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

steakhal added a comment.

In D85728#2237006 , @NoQ wrote:

> Hans is pinging us on Bugzilla because this patch is marked as a release 
> blocker (and i believe that it indeed is). I think we should land these 
> patches.

I believe @baloghadamsoftware is on vacation.
I'm sure he won't be mad if you commit on his behalf.


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

https://reviews.llvm.org/D85728

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

[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hans is pinging us on Bugzilla because this patch is marked as a release 
blocker (and i believe that it indeed is). I think we should land these patches.


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

https://reviews.llvm.org/D85728

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

[clang] 01eb123 - [X86] Mention -march=sapphirerapids in the release notes.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2020-08-25T11:57:34-07:00
New Revision: 01eb1233db54454b146cb1e70d6f810ffbc354e5

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

LOG: [X86] Mention -march=sapphirerapids in the release notes.

This was just added in e02d081f2b60b61eb60ef6a49b1a9f907e432d4c.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
llvm/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c036f66d60bf7..e810b92c49271 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -188,6 +188,8 @@ X86 Support in Clang
 - The x86 intrinsics ``__rorb``, ``__rorw``, ``__rord``, ``__rorq`, ``_rotr``,
   ``_rotwr`` and ``_lrotr`` may now be used within constant expressions.
 
+- Support for -march=sapphirerapids was added.
+
 Internal API Changes
 
 

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 2a2775f13dd1f..c6b2a3ac9bb57 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -97,6 +97,7 @@ During this release ...
 * The 'mpx' feature was removed from the backend. It had been removed from 
clang
   frontend in 10.0. Mention of the 'mpx' feature in an IR file will print a
   message to stderr, but IR should still compile.
+* Support for -march=sapphirerapids was added.
 
 Changes to the AMDGPU Target
 -



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

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

This feels like the wrong approach to me... but I admit that I don't know what 
the "right" approach might be. (I doubt any right approach exists.)

  if (ch == ' ') [[likely]] {
  goto whitespace;  // A
  } else if (ch == '\n' || ch == '\t') [[unlikely]] {
  goto whitespace;  // B
  } else {
  foo();
  }
  [[likely]] whitespace: bar();  // C

It seems like this patch would basically "copy" the `[[likely]]` attribute from 
line C up to lines A and B, where it would reinforce the likelihood of path A 
and (maybe?) "cancel out" the unlikelihood of path B, without actually saying 
anything specifically about the likelihood of label C (which is surely what the 
programmer intended by applying the attribute, right?). OTOH, I can't think of 
any particular optimization that would care about the likelihood of label C. I 
could imagine trying to align label C to a 4-byte boundary or something, but 
that wouldn't be an //optimization// on any platform as far as I know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559

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

[PATCH] D83845: [LTO/WPD] Remove special type test handling for -flto-visibility-public-std

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83845

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

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

@pcc ping. I'd like to get something in for LLVM 11 if at all possible. How 
about for now I make the last sentence something like "This is useful in 
situations where it is not safe to specify``-fvisibility=hidden`` at compile 
time, for example when bitcode objects will be consumed by different LTO links 
that don't all have whole program visibility." ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655

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

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added reviewers: jdoerfert, kkwli0, RaviNarayanaswamy, hfinkel.
Herald added subscribers: jfb, guansong, yaxunl.
Herald added a project: clang.
ABataev requested review of this revision.
Herald added a subscriber: sstefan1.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86562

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,7 +156,49 @@
 OpenMP Support in Clang
 ---
 
-- ...
+New features for OpenMP 5.0 were implemented.
+
+- OpenMP 5.0 is the default version supported by the compiler. User can switch
+  to OpenMP 4.5 using ``-fopenmp-version=45`` option.
+
+- Added support for declare variant directive.
+
+- Improved support of math functions for NVPTX target.
+
+- Added support for parallel execution of target regions for NVPTX target.
+
+- Addes support for ``scan`` directives and ``inscan`` modifier in 
``reduction``
+  clauses.
+
+- Added support for ``iterator`` construct.
+
+- Added support for ``depobj`` construct.
+
+- Added support for ``detach`` clauses in task-based directives.
+
+- Added support for array shaping operations.
+
+- Added support for cancellation constructs in ``taskloop`` directives.
+
+- Nonmonotonic modifier is allowed with all schedule kinds.
+
+- Added support for ``task`` and ``default`` modifiers in ``reduction`` 
clauses.
+
+- Added support for strides in array sections.
+
+- Added support for ``use_device_addr`` clause.
+
+- Added support for ``uses_allocators`` clause.
+
+- Added support for ``defaultmap`` clause.
+
+- Added basic support for ``hint`` clause in ``atomic`` directives.
+
+- Added basic support for ``affinity`` clause.
+
+- Added basic support for ``ancestor`` modifier in ``device`` clause.
+
+- Bug fixes and optimizations.
 
 CUDA Support in Clang
 -


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,7 +156,49 @@
 OpenMP Support in Clang
 ---
 
-- ...
+New features for OpenMP 5.0 were implemented.
+
+- OpenMP 5.0 is the default version supported by the compiler. User can switch
+  to OpenMP 4.5 using ``-fopenmp-version=45`` option.
+
+- Added support for declare variant directive.
+
+- Improved support of math functions for NVPTX target.
+
+- Added support for parallel execution of target regions for NVPTX target.
+
+- Addes support for ``scan`` directives and ``inscan`` modifier in ``reduction``
+  clauses.
+
+- Added support for ``iterator`` construct.
+
+- Added support for ``depobj`` construct.
+
+- Added support for ``detach`` clauses in task-based directives.
+
+- Added support for array shaping operations.
+
+- Added support for cancellation constructs in ``taskloop`` directives.
+
+- Nonmonotonic modifier is allowed with all schedule kinds.
+
+- Added support for ``task`` and ``default`` modifiers in ``reduction`` clauses.
+
+- Added support for strides in array sections.
+
+- Added support for ``use_device_addr`` clause.
+
+- Added support for ``uses_allocators`` clause.
+
+- Added support for ``defaultmap`` clause.
+
+- Added basic support for ``hint`` clause in ``atomic`` directives.
+
+- Added basic support for ``affinity`` clause.
+
+- Added basic support for ``ancestor`` modifier in ``device`` clause.
+
+- Bug fixes and optimizations.
 
 CUDA Support in Clang
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82582: [SVE] Remove calls to VectorType::getNumElements from clang

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau marked 8 inline comments as done.
ctetreau added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:1765
   }
   auto *VectorTy = dyn_cast(
   cast(Addr.getPointer()->getType())->getElementType());

c-rhodes wrote:
> cast here
that would be a behavior change because of the dyn_cast.



Comment at: clang/lib/CodeGen/CGExpr.cpp:1799
   llvm::Type *SrcTy = Value->getType();
   auto *VecTy = dyn_cast(SrcTy);
   // Handle vec3 special.

c-rhodes wrote:
> cast here
that would be a behavior change because of the dyn_cast



Comment at: clang/lib/CodeGen/CGExpr.cpp:2214
   if (const VectorType *VTy = Dst.getType()->getAs()) {
 unsigned NumSrcElts = VTy->getNumElements();
 unsigned NumDstElts =

c-rhodes wrote:
> missed one here
this VTy is a clang::VectorType



Comment at: clang/lib/CodeGen/SwiftCallingConv.cpp:321
   // If we have a vector type, split it.
   if (auto vecTy = dyn_cast_or_null(type)) {
 auto eltTy = vecTy->getElementType();

c-rhodes wrote:
> cast here
this would be a behavior change due to dyn_cast


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82582

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

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: llvm/lib/Analysis/OptimizationRemarkEmitter.cpp:103
 BFI = ().getBFI();
-  else
+// Get hotness threshold from PSI. This should only happen once.
+if (Context.isDiagnosticsHotnessThresholdSetFromPSI()) {

Can you clarify what you mean by this only happening once?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rsmith, rjmccall, aaron.ballman.
Mordante added a project: clang.
Herald added a subscriber: cfe-commits.
Mordante requested review of this revision.

Allows the likelihood attributes on label, which are not part of a case 
statement. When a `goto` is used it accepts the attribute placed on the target 
label.

Depends on D85091 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86559

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/Sema/attr-likelihood.c
  clang/test/SemaCXX/attr-likelihood.cpp

Index: clang/test/SemaCXX/attr-likelihood.cpp
===
--- clang/test/SemaCXX/attr-likelihood.cpp
+++ clang/test/SemaCXX/attr-likelihood.cpp
@@ -115,8 +115,7 @@
   if (x)
 goto lbl;
 
-  // FIXME: allow the attribute on the label
-  [[unlikely]] lbl : // expected-error {{'unlikely' attribute cannot be applied to a declaration}}
+  [[likely]] lbl :
  [[likely]] x = x + 1;
 
   [[likely]]++ x;
Index: clang/test/Sema/attr-likelihood.c
===
--- clang/test/Sema/attr-likelihood.c
+++ clang/test/Sema/attr-likelihood.c
@@ -43,8 +43,7 @@
   if (x)
 goto lbl;
 
-  // FIXME: allow the attribute on the label
-  [[clang::unlikely]] lbl : // expected-error {{'unlikely' attribute cannot be applied to a declaration}}
+  [[clang::unlikely]] lbl :
   [[clang::likely]] x = x + 1;
 
   [[clang::likely]]++ x;
Index: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
===
--- clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
+++ clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
@@ -125,7 +125,45 @@
 // Make sure the branches aren't optimized away.
 b = true;
   }
+
+  // CHECK: br {{.*}} !prof !8
+  if (b) {
+ goto uend;
+  } else {
+// Make sure the branches aren't optimized away.
+b = true;
+  }
+
+  // CHECK: br {{.*}} !prof !7
+  if (b) {
+ goto lend;
+  } else {
+// Make sure the branches aren't optimized away.
+b = true;
+  }
+
+  // CHECK: br {{.*}} !prof !8
+  if (b) {
+ goto ugcc;
+  } else {
+// Make sure the branches aren't optimized away.
+b = true;
+  }
+
+  // CHECK: br {{.*}} !prof !7
+  if (b) {
+ goto lgcc;
+  } else {
+// Make sure the branches aren't optimized away.
+b = true;
+  }
 end:;
+[[unlikely]] uend:;
+[[likely]] lend:;
+// GCC attributes are handled in a special way, make sure the likelihood
+// attributes aren't ignored.
+[[unlikely]] ugcc: __attribute__((unused));;
+[[likely]] lgcc: __attribute__((unused));;
 }
 
 void ReturnStmt() {
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6410,6 +6410,29 @@
   D->addAttr(::new (S.Context) DeprecatedAttr(S.Context, AL, Str, Replacement));
 }
 
+static bool validateLikelihoodAttr(Sema , Decl *D, const ParsedAttr ) {
+  if (!isa(D)) {
+S.Diag(A.getRange().getBegin(), diag::err_stmt_attribute_invalid_on_decl)
+<< A << D->getBeginLoc();
+return false;
+  }
+
+  if (!S.getLangOpts().CPlusPlus20 && A.isCXX11Attribute() && !A.getScopeName())
+S.Diag(A.getLoc(), diag::ext_cxx20_attr) << A << A.getRange();
+
+  return true;
+}
+
+static void handleLikelyAttr(Sema , Decl *D, const ParsedAttr ) {
+  if (validateLikelihoodAttr(S, D, A))
+D->addAttr(::new (S.Context) LikelyAttr(S.Context, A));
+}
+
+static void handleUnlikelyAttr(Sema , Decl *D, const ParsedAttr ) {
+  if (validateLikelihoodAttr(S, D, A))
+D->addAttr(::new (S.Context) UnlikelyAttr(S.Context, A));
+}
+
 static bool isGlobalVar(const Decl *D) {
   if (const auto *S = dyn_cast(D))
 return S->hasGlobalStorage();
@@ -6943,6 +6966,12 @@
   case ParsedAttr::AT_Deprecated:
 handleDeprecatedAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_Likely:
+handleLikelyAttr(S, D, AL);
+break;
+  case ParsedAttr::AT_Unlikely:
+handleUnlikelyAttr(S, D, AL);
+break;
   case ParsedAttr::AT_Destructor:
 if (S.Context.getTargetInfo().getTriple().isOSAIX())
   llvm::report_fatal_error("'destructor' attribute is not yet supported on AIX");
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -672,17 +672,8 @@
   LikelihoodAttributeFinder(const Stmt *S) { Visit(S); }
 
   void VisitAttributedStmt(const AttributedStmt *S) {
-for (const auto *A : S->getAttrs()) {
-  if (isa(A)) {
-Result = Likely;
-return;
-  }
-  if (isa(A)) {
-Result = Unlikely;
-return;
-  }
-}
-

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I'll land tomorrow unless Alexandre beats me to it.


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

https://reviews.llvm.org/D85998

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

[PATCH] D86491: [DebugInfo] Move constructor homing case in shouldOmitDefinition.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

In D86491#2235206 , @MaskRay wrote:

> Note that Harbormaster actually reported the check-clang-codegencxx issues 
> B69369 ..

Definitely my bad; I thought I had run check-clang locally before committing, 
but maybe .. I didn't?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86491

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

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

Thanks @hans!  I'll need you (or someone else with permission) to land the 
change on my behalf.


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

https://reviews.llvm.org/D85998

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

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Z. Zheng via Phabricator via cfe-commits]

zzheng added a comment.

In D84414#2234267 , @lenary wrote:

> Ok, so any compilation units without `-fsanitize=shadow-call-stack` should be 
> compiled with `-ffixed-x18` if you want to link those together. That is 
> reasonable. My question was whether we can ensure that 
> `-fsanitize=shadow-call-stack` can imply `-ffixed-x18` rather than having to 
> pass both options.
>
> It is my understanding that all functions in a CU with 
> `-fsanitize=shadow-call-stack` will get the SCS function attribute, so why 
> can't we use that attribute to work out whether x18 should be reserved, 
> rather than using `-ffixed-x18`? You'll see 
> `RISCVRegisterInfo::getReservedRegs` reserves `fp` and `bp` only if they're 
> needed by the function (which can be based on attributes or other info), 
> rather than using `isRegisterReservedByUser` - which seems cleaner to me.



In D84414#2234327 , @pcc wrote:

> FWIW, on aarch64 we decided to make `-fsanitize=shadow-call-stack` require 
> the x18 reservation (instead of implying it) to try to avoid ABI mismatch 
> problems. That is, it should be safe to mix and match code compiled with and 
> without `-fsanitize=shadow-call-stack`. If we make 
> `-fsanitize=shadow-call-stack` imply the x18 reservation, it makes it more 
> likely that someone will accidentally build and link in incompatible code 
> that does not reserve x18.

Yes, binding '-fsanitize=shadow-call-stack' with '-ffixed-x18' is to ensure 
user don't accidentally link SCS-enabled code with non-SCS code that 
uses/overwrites x18. We can always infer that x18 is reserved when SCS in on.

There's no need to check `RISCVRegisterInfo::getReservedRegs` for x18 by 
looking into function attr. It forms a circular chain: is x18 reserved for SCS? 
<--> SCS is enabled, so x18 should be reserved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414

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

[PATCH] D86558: [OPENMP]Add support for allocate vars in untied tasks.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added a reviewer: jdoerfert.
Herald added subscribers: guansong, yaxunl.
Herald added a project: clang.
ABataev requested review of this revision.
Herald added a subscriber: sstefan1.

Local vars, marked with pragma allocate, mustbe allocate by the call of
the runtime function and cannot be allocated as other local variables.
Instead, we allocate a space for the pointer in private record and store
the address, returned by kmpc_alloc call in this pointer.
So, for untied tasks

  #pragma omp task untied
  {
S s;
 #pragma omp allocate(s) allocator(allocator)
s = x;
  }

compiler generates something like this:

  struct task_with_privates {
S *ptr;
  };
  
  void entry(task_with_privates *p) {
S *s = p->s;
switch(partid) {
case 1:
  p->s = (S*)kmpc_alloc();
  kmpc_omp_task();
  br exit;
case 2:
  *s = x;
  kmpc_omp_task();
  br exit;
case 2:
  ~S(s);
  kmpc_free((void*)s);
  br exit;
}
  exit:
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86558

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/allocate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/parallel_firstprivate_codegen.cpp
  clang/test/OpenMP/parallel_private_codegen.cpp
  clang/test/OpenMP/task_codegen.cpp

Index: clang/test/OpenMP/task_codegen.cpp
===
--- clang/test/OpenMP/task_codegen.cpp
+++ clang/test/OpenMP/task_codegen.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix UNTIEDRT
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - -DUNTIEDRT | FileCheck %s --check-prefix CHECK --check-prefix UNTIEDRT
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s -DUNTIEDRT
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix UNTIEDRT
 //
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fopenmp-enable-irbuilder -x c++ -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
@@ -14,6 +14,19 @@
 #ifndef HEADER
 #define HEADER
 
+enum omp_allocator_handle_t {
+  omp_null_allocator = 0,
+  omp_default_mem_alloc = 1,
+  omp_large_cap_mem_alloc = 2,
+  omp_const_mem_alloc = 3,
+  omp_high_bw_mem_alloc = 4,
+  omp_low_lat_mem_alloc = 5,
+  omp_cgroup_mem_alloc = 6,
+  omp_pteam_mem_alloc = 7,
+  omp_thread_mem_alloc = 8,
+  KMP_ALLOCATOR_MAX_HANDLE = __UINTPTR_MAX__
+};
+
 // CHECK-DAG: [[IDENT_T:%.+]] = type { i32, i32, i32, i32, i8* }
 // CHECK-DAG: [[STRUCT_SHAREDS:%.+]] = type { i8*, [2 x [[STRUCT_S:%.+]]]* }
 // CHECK-DAG: [[STRUCT_SHAREDS1:%.+]] = type { [2 x [[STRUCT_S:%.+]]]* }
@@ -258,21 +271,26 @@
 a = 4;
 c = 5;
   }
-// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i32 0, i64 48, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
+// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i32 0, i64 256, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
 // CHECK: call i32 @__kmpc_omp_task([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i8* [[ORIG_TASK_PTR]])
-#pragma omp task untied
+#pragma omp task untied firstprivate(c) allocate(omp_pteam_mem_alloc:c)
   {
-S s1;
+S s1, s2;
+#ifdef UNTIEDRT
+#pragma omp allocate(s2) allocator(omp_pteam_mem_alloc)
+#endif
+s2.a = 0;
 #pragma omp task
-a = 4;
+a = c = 4;
 #pragma omp taskyield
 s1 = S();
+s2.a = 10;
 #pragma omp taskwait
   }
   return a;
 }
 // CHECK: define internal i32 [[TASK_ENTRY1]](i32 %0, [[KMP_TASK_T]]{{.*}}* noalias %1)
-// CHECK: store i32 15, i32* [[A_PTR:@.+]]
+// CHECK: store i32 15, i32* [[A_PTR:@.+]],
 // CHECK: [[A_VAL:%.+]] = load i32, i32* [[A_PTR]]
 // CHECK: [[A_VAL_I8:%.+]] = trunc i32 [[A_VAL]] to i8
 // CHECK: store i8 [[A_VAL_I8]], i8* %{{.+}}
@@ -294,10 +312,13 @@
 // CHECK: define internal i32
 // CHECK: store i32 4, i32* [[A_PTR]]
 
-// CHECK: define internal i32 [[TASK_ENTRY6]](i32 %0, [[KMP_TASK_T]]{{.*}}* noalias %1)
+// CHECK: define internal i32 [[TASK_ENTRY6]](i32 %0, [[KMP_TASK_T]]{{.*}}* noalias %{{.+}})
 // UNTIEDRT: 

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

Looks pretty good but I think it can be simplified in one place as I note below.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:596
+  // Handle remark diagnostics on screen options: '-Rpass-*'.
+  if (usesRpassOptions(Args))
+renderRpassOptions(Args, CmdArgs);

Is this guard needed? It looks like renderRpassOptions will do the right thing 
if none are specified (won't add anything). Right now you end up checking each 
one a second time if any are enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

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

[PATCH] D86376: [HIP] Improve kernel launching latency

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D86376#2236501 , @yaxunl wrote:

> My previous measurements did not warming up, which caused some one time 
> overhead due to device initialization and loading of device binary. With warm 
> up, the call of `__hipPushCallConfigure/__hipPopCallConfigure` takes about 19 
> us. Based on the trace from rocprofile, the time spent inside these functions 
> can be ignored. Most of the time is spent making the calls. These functions 
> stay in a shared library, which may be the reason why they take such long 
> time. Making them always_inline may get rid of the overhead, however, that 
> would require exposing internal data structures.

It's still suspiciously high. AFAICT, config/push/pull is just an std::vector 
push/pop. It should not take *that* long.  Few function calls should not lead 
to microseconds of overhead, once linker has resolved the symbol, if they come 
from a shared library.
https://github.com/ROCm-Developer-Tools/HIP/blob/master/vdi/hip_platform.cpp#L590

I wonder if it's the logging facilities that add all this overhead.

> The kernel launching latency are measured by a simple loop in which a simple 
> kernel is launched then hipStreamSynchronize is called. trace is collected by 
> rocprofiler and the latency is measured from the end of hipStreamSynchronize 
> to the real start of kernel execution. Without this patch, the latency is 
> about 77 us. With this patch, the latency is about 46 us. The improvement is 
> about 40%. The decrement of 31 us is more than 19 us since it also eliminates 
> the overhead of kernel stub.

This is rather surprising. A function call by itself does *not* have such high 
overhead. There must be something else. I strongly suspect logging. If you 
remove logging statements from push/pop without changing anything else, how 
does that affect performance?

>>> I would like to say the motivation of this change is two folds: 1. improve 
>>> latency 2. interoperability with C++ programs.
>>
>> Could you elaborate on the "interoperability with C++ programs"? I don't 
>> think I see how this patch helps with that. Or what exactly is the issue 
>> with C++ interoperability we have now?
>
> In HIP program, a global symbol is generated in host binary to identify each 
> kernel. This symbol is associated with the device kernel by a call of 
> hipRegisterFunction in init functions. Each time the kernel needs to be 
> called, the associated symbol is passed to hipLaunchKernel. In host code, 
> this symbol represents the kernel. Let's call it the kernel symbol. Currently 
> it is the kernel stub function, however, it could be any global symbol, as 
> long as it is registered with hipRegisterFunction, then hipLaunchKernel can 
> use it to find the right kernel and launch it.

So far so good, it matches the way CUDA does that.

> In a C/C++ program, a kernel is launched by call of hipLaunchKernel with the 
> kernel symbol.

Do you mean the host-side symbol, registered with the runtime that you've 
described above? Or do you mean that the device-side symbol is somehow visible 
from the host side. I think that's where HIP is different from CUDA.

> Since the kernel symbol is defined in object files generated from HIP. 
> For C/C++ program, as long as it declares the kernel symbol as an external 
> function or variable which matches the name of the original symbol, the 
> linker will resolve to the correct kernel symbol, then the correct kernel can 
> be launched.

The first sentence looks incomplete. It seems to imply that hipLaunchKernel 
uses the device-side kernel symbol and it's the linker which ties host-side 
reference with device-side symbol. If that's the case, then I don't understand 
what purpose is served by hipRegisterFunction. AFAICT, it's not used in this 
scenario at all.

My mental model of kernel launch mechanics looks like this:

- For a kernel foo, there is a host-side symbol (it's the stub for CUDA) with 
the name 'foo' and device-side real kernel 'foo'.
- host side linker has no access to device-side symbols, but we do need to 
associate host and device side 'foo' instances.
- address of host-side foo is registered with runtime to map it to device 
symbol with the name 'foo'
- when a kernel is launched, call site sets up launch config and calls the 
stub, passing it the kernel arguments.
- the stub calls the kernel launch function, and passes host-side foo address 
to the kernel launch function
- launch function finds device-side symbol name via the registration info and 
does device-side address lookup to obtain it's device address
- run device-side function.

In this scenario, the host-side stub for foo is a regular function, which gdb 
can stop on and examine kernel arguments.

How is the process different for HIP? I know that we've changed the stub name 
to avoid debugger confusion about which if the entities corresponds to 'foo'.

> Here comes the nuance with kernel stub function as the kernel 

[PATCH] D86544: [SyntaxTree] Add support for call expressions

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added a reviewer: gribozavr2.
eduucaldas added inline comments.



Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:506
 | `-'~'
-|-'decltype'
+|-'decltype' OpenParen
 |-'('

This is wrong but it's just because decltype is all wrong



Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2734
+|   |   `-'args'
+|   `-'...'
+`-')' CloseParen

Here there is a divergence between the grammar and the ClangAST. 
According to the [[ https://eel.is/c++draft/expr.post#nt:expression-list | 
grammar ]]  `...` would an optional element of `CallArguments`, but here `...` 
is part of an expression.

Perhaps I have used the wrong kind of `...`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86544

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

[PATCH] D86544: [SyntaxTree] Add support for call expressions

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 287694.
eduucaldas added a comment.

- Nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86544

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -316,12 +316,12 @@
   `-CompoundStatement
 |-'{' OpenParen
 |-ExpressionStatement CompoundStatement_statement
-| |-UnknownExpression ExpressionStatement_expression
-| | |-IdExpression
+| |-CallExpression ExpressionStatement_expression
+| | |-IdExpression CallExpression_callee
 | | | `-UnqualifiedId IdExpression_id
 | | |   `-'test'
-| | |-'('
-| | `-')'
+| | |-'(' OpenParen
+| | `-')' CloseParen
 | `-';'
 |-IfStatement CompoundStatement_statement
 | |-'if' IntroducerKeyword
@@ -330,21 +330,21 @@
 | | `-'1' LiteralToken
 | |-')'
 | |-ExpressionStatement IfStatement_thenStatement
-| | |-UnknownExpression ExpressionStatement_expression
-| | | |-IdExpression
+| | |-CallExpression ExpressionStatement_expression
+| | | |-IdExpression CallExpression_callee
 | | | | `-UnqualifiedId IdExpression_id
 | | | |   `-'test'
-| | | |-'('
-| | | `-')'
+| | | |-'(' OpenParen
+| | | `-')' CloseParen
 | | `-';'
 | |-'else' IfStatement_elseKeyword
 | `-ExpressionStatement IfStatement_elseStatement
-|   |-UnknownExpression ExpressionStatement_expression
-|   | |-IdExpression
+|   |-CallExpression ExpressionStatement_expression
+|   | |-IdExpression CallExpression_callee
 |   | | `-UnqualifiedId IdExpression_id
 |   | |   `-'test'
-|   | |-'('
-|   | `-')'
+|   | |-'(' OpenParen
+|   | `-')' CloseParen
 |   `-';'
 `-'}' CloseParen
 )txt"));
@@ -378,20 +378,21 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-IdExpression
+CallExpression ExpressionStatement_expression
+|-IdExpression CallExpression_callee
 | `-UnqualifiedId IdExpression_id
 |   |-'operator'
 |   `-'+'
-|-'('
-|-IdExpression
-| `-UnqualifiedId IdExpression_id
-|   `-'x'
-|-','
-|-IdExpression
-| `-UnqualifiedId IdExpression_id
-|   `-'x'
-`-')'
+|-'(' OpenParen
+|-CallArguments CallExpression_arguments
+| |-IdExpression List_element
+| | `-UnqualifiedId IdExpression_id
+| |   `-'x'
+| |-',' List_delimiter
+| `-IdExpression List_element
+|   `-UnqualifiedId IdExpression_id
+| `-'x'
+`-')' CloseParen
 )txt"}));
 }
 
@@ -409,8 +410,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -419,8 +420,8 @@
 |   `-UnqualifiedId IdExpression_id
 | |-'operator'
 | `-'int'
-|-'('
-`-')'
+|-'(' OpenParen
+`-')' CloseParen
 )txt"}));
 }
 
@@ -436,16 +437,17 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-IdExpression
+CallExpression ExpressionStatement_expression
+|-IdExpression CallExpression_callee
 | `-UnqualifiedId IdExpression_id
 |   |-'operator'
 |   |-'""'
 |   `-'_w'
-|-'('
-|-CharacterLiteralExpression
-| `-''1'' LiteralToken
-`-')'
+|-'(' OpenParen
+|-CallArguments CallExpression_arguments
+| `-CharacterLiteralExpression List_element
+|   `-''1'' LiteralToken
+`-')' CloseParen
 )txt"}));
 }
 
@@ -461,8 +463,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -471,8 +473,8 @@
 |   `-UnqualifiedId IdExpression_id
 | |-'~'
 | `-'X'
-|-'('
-`-')'
+|-'(' OpenParen
+`-')' CloseParen
 )txt"}));
 }
 
@@ -492,8 +494,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -501,12 +503,12 @@
 | `-IdExpression MemberExpression_member
 |   `-UnqualifiedId IdExpression_id
 | `-'~'
-|-'decltype'
+|-'decltype' OpenParen
 |-'('
 |-'x'
 |-')'
 |-'('
-`-')'
+`-')' CloseParen
 )txt"}));
 }
 
@@ -523,15 +525,15 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-IdExpression
+CallExpression ExpressionStatement_expression
+|-IdExpression CallExpression_callee
 | `-UnqualifiedId IdExpression_id
 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 287692.
simoll added a comment.

NFC. Make clang-tidy happy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool-hvx64.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-conditional.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- clang/test/SemaCXX/vector-conditional.cpp
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((__vector_size__(2)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 

[PATCH] D77925: Revert "[TLI] Per-function fveclib for math library used for vectorization"

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D77925#2229484 , @mehdi_amini wrote:

> Overall that would likely work for XLA. Something I'd like to mention though 
> in response to:
>
>> The veclib type is also tied to the accepted values for -fveclib, which is a 
>> list of supported lib,
>
> `-fveclib` is a Clang thing, it shouldn't limit what LLVM does. Of course 
> LLVM needs to support Clang, but does not have to inherit the limitation of 
> map 1:1 to Clang UI.
> In particular as a library, it isn't clear why we would make the choice to 
> write LLVM VecLib support this way.

Is there any benefit to keeping a closed list like this in LLVM? If not (and 
presumably clang is checking for valid values of -fveclib), then I think I 
agree with @mehdi_amini. Unless there is an efficiency reason for doing it via 
an enum. It's been awhile since I looked through this code in detail...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77925

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

[PATCH] D86376: [HIP] Improve kernel launching latency

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D86376#2234824 , @tra wrote:

> In D86376#2234719 , @yaxunl wrote:
>
>>> This patch appears to be somewhere in the gray area to me. My prior 
>>> experience with CUDA suggests that it will make little to no difference. On 
>>> the other hand, AMD GPUs may be different enough to prove me wrong. Without 
>>> specific evidence, I still can't tell what's the case here.
>>
>> Sorry, the overhead due to `__hipPushConfigure/__hipPopConfigure` is about 
>> 60 us. The typical kernel launching latency is about 500us, therefore the 
>> improvement is around 10%.
>
> 60 *micro seconds* to store/load something from memory? It does not sound 
> right. 0.5 millisecond per kernel launch is also suspiciously high. 
> For CUDA it's ~5us 
> (https://www.hpcs.cs.tsukuba.ac.jp/icpp2019/data/posters/Poster17-abst.pdf). 
> If it does indeed take 60 microseconds to push/pop a O(cacheline) worth of 
> launch config data, the implementation may be doing something wrong. We're 
> talking about O(100) syscalls and that's way too much work for something that 
> simple. What do those calls do?
>
> Can you confirm that the units are indeed microseconds and not nanoseconds?

My previous measurements did not warming up, which caused some one time 
overhead due to device initialization and loading of device binary. With warm 
up, the call of `__hipPushCallConfigure/__hipPopCallConfigure` takes about 19 
us. Based on the trace from rocprofile, the time spent inside these functions 
can be ignored. Most of the time is spent making the calls. These functions 
stay in a shared library, which may be the reason why they take such long time. 
Making them always_inline may get rid of the overhead, however, that would 
require exposing internal data structures.

The kernel launching latency are measured by a simple loop in which a simple 
kernel is launched then hipStreamSynchronize is called. trace is collected by 
rocprofiler and the latency is measured from the end of hipStreamSynchronize to 
the real start of kernel execution. Without this patch, the latency is about 77 
us. With this patch, the latency is about 46 us. The improvement is about 40%. 
The decrement of 31 us is more than 19 us since it also eliminates the overhead 
of kernel stub.

>> I would like to say the motivation of this change is two folds: 1. improve 
>> latency 2. interoperability with C++ programs.
>
> Could you elaborate on the "interoperability with C++ programs"? I don't 
> think I see how this patch helps with that. Or what exactly is the issue with 
> C++ interoperability we have now?

In HIP program, a global symbol is generated in host binary to identify each 
kernel. This symbol is associated with the device kernel by a call of 
hipRegisterFunction in init functions. Each time the kernel needs to be called, 
the associated symbol is passed to hipLaunchKernel. In host code, this symbol 
represents the kernel. Let's call it the kernel symbol. Currently it is the 
kernel stub function, however, it could be any global symbol, as long as it is 
registered with hipRegisterFunction, then hipLaunchKernel can use it to find 
the right kernel and launch it.

In a C/C++ program, a kernel is launched by call of hipLaunchKernel with the 
kernel symbol. Since the kernel symbol is defined in object files generated 
from HIP. For C/C++ program, as long as it declares the kernel symbol as an 
external function or variable which matches the name of the original symbol, 
the linker will resolve to the correct kernel symbol, then the correct kernel 
can be launched.

Here comes the nuance with kernel stub function as the kernel symbol. If you 
still remember, there was a previous patch for HIP to change the kernel stub 
name. rocgdb requires the device stub to have a different name than the real 
kernel, since otherwise it will not be able to break on the real kernel only. 
As a result, the kernel stub now has a prefix `__device_stub_` before mangling.

For example, a kernel `foo` will have a kernel stub with name 
`__device_stub_foo`.

For a C/C++ program to call kernel `foo`, it needs to declare an external 
symbol `__device_stub_foo` then launch it. Of course this is an annoyance for 
C/C++ users, especially this involves mangled names.

However, we cannot change the name of the kernel stub to be the same as the 
kernel, since that will break rocgdb.

Now the solution is to get rid of the kernel stub function. Instead of use 
kernel stub function as kernel symbol, we will emit a global variable as kernel 
symbol. This global variable can have the same name as the kernel, since rocgdb 
will not break on it.


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

https://reviews.llvm.org/D86376

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

[PATCH] D86221: [compiler-rt][builtins] Do not assume int to be at least 32 bit wide

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko abandoned this revision.
atrosinenko added a comment.

Replaced this by D86546: [compiler-rt][builtins] Use explicitly-sized integer 
types for LibCalls  and D86547: 
[compiler-rt][builtins] Use c[tl]zsi macro instead of __builtin_c[tl]z 
. These two patches include everything except 
for factoring out `src_rep_t_clz()` and `rep_clz()` to `clzdi()` for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86221

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

[PATCH] D86547: [compiler-rt][builtins] Use c[tl]zsi macro instead of __builtin_c[tl]z

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko created this revision.
atrosinenko added reviewers: efriedma, MaskRay, aykevl, uabelho.
Herald added subscribers: Sanitizers, dberris.
Herald added a project: Sanitizers.
atrosinenko requested review of this revision.

`__builtin_c[tl]z` accepts `unsigned int` argument that is not always the
same as uint32_t. For example, `unsigned int` is uint16_t on MSP430.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86547

Files:
  compiler-rt/lib/builtins/floatsisf.c
  compiler-rt/lib/builtins/floatsitf.c
  compiler-rt/lib/builtins/floatunsisf.c
  compiler-rt/lib/builtins/floatunsitf.c
  compiler-rt/lib/builtins/fp_extend.h
  compiler-rt/lib/builtins/udivmoddi4.c


Index: compiler-rt/lib/builtins/udivmoddi4.c
===
--- compiler-rt/lib/builtins/udivmoddi4.c
+++ compiler-rt/lib/builtins/udivmoddi4.c
@@ -82,7 +82,7 @@
 r.s.high = n.s.high & (d.s.high - 1);
 *rem = r.all;
   }
-  return n.s.high >> __builtin_ctz(d.s.high);
+  return n.s.high >> ctzsi(d.s.high);
 }
 // K K
 // ---
@@ -112,7 +112,7 @@
   *rem = n.s.low & (d.s.low - 1);
 if (d.s.low == 1)
   return n.all;
-sr = __builtin_ctz(d.s.low);
+sr = ctzsi(d.s.low);
 q.s.high = n.s.high >> sr;
 q.s.low = (n.s.high << (n_uword_bits - sr)) | (n.s.low >> sr);
 return q.all;
Index: compiler-rt/lib/builtins/fp_extend.h
===
--- compiler-rt/lib/builtins/fp_extend.h
+++ compiler-rt/lib/builtins/fp_extend.h
@@ -33,9 +33,9 @@
   return __builtin_clzl(a);
 #else
   if (a & REP_C(0x))
-return __builtin_clz(a >> 32);
+return clzsi(a >> 32);
   else
-return 32 + __builtin_clz(a & REP_C(0x));
+return 32 + clzsi(a & REP_C(0x));
 #endif
 }
 
Index: compiler-rt/lib/builtins/floatunsitf.c
===
--- compiler-rt/lib/builtins/floatunsitf.c
+++ compiler-rt/lib/builtins/floatunsitf.c
@@ -25,7 +25,7 @@
 return fromRep(0);
 
   // Exponent of (fp_t)a is the width of abs(a).
-  const int exponent = (aWidth - 1) - __builtin_clz(a);
+  const int exponent = (aWidth - 1) - clzsi(a);
   rep_t result;
 
   // Shift a into the significand field and clear the implicit bit.
Index: compiler-rt/lib/builtins/floatunsisf.c
===
--- compiler-rt/lib/builtins/floatunsisf.c
+++ compiler-rt/lib/builtins/floatunsisf.c
@@ -26,7 +26,7 @@
 return fromRep(0);
 
   // Exponent of (fp_t)a is the width of abs(a).
-  const int exponent = (aWidth - 1) - __builtin_clz(a);
+  const int exponent = (aWidth - 1) - clzsi(a);
   rep_t result;
 
   // Shift a into the significand field, rounding if it is a right-shift
Index: compiler-rt/lib/builtins/floatsitf.c
===
--- compiler-rt/lib/builtins/floatsitf.c
+++ compiler-rt/lib/builtins/floatsitf.c
@@ -33,7 +33,7 @@
   }
 
   // Exponent of (fp_t)a is the width of abs(a).
-  const int exponent = (aWidth - 1) - __builtin_clz(aAbs);
+  const int exponent = (aWidth - 1) - clzsi(aAbs);
   rep_t result;
 
   // Shift a into the significand field and clear the implicit bit.
Index: compiler-rt/lib/builtins/floatsisf.c
===
--- compiler-rt/lib/builtins/floatsisf.c
+++ compiler-rt/lib/builtins/floatsisf.c
@@ -33,7 +33,7 @@
   }
 
   // Exponent of (fp_t)a is the width of abs(a).
-  const int exponent = (aWidth - 1) - __builtin_clz(a);
+  const int exponent = (aWidth - 1) - clzsi(a);
   rep_t result;
 
   // Shift a into the significand field, rounding if it is a right-shift


Index: compiler-rt/lib/builtins/udivmoddi4.c
===
--- compiler-rt/lib/builtins/udivmoddi4.c
+++ compiler-rt/lib/builtins/udivmoddi4.c
@@ -82,7 +82,7 @@
 r.s.high = n.s.high & (d.s.high - 1);
 *rem = r.all;
   }
-  return n.s.high >> __builtin_ctz(d.s.high);
+  return n.s.high >> ctzsi(d.s.high);
 }
 // K K
 // ---
@@ -112,7 +112,7 @@
   *rem = n.s.low & (d.s.low - 1);
 if (d.s.low == 1)
   return n.all;
-sr = __builtin_ctz(d.s.low);
+sr = ctzsi(d.s.low);
 q.s.high = n.s.high >> sr;
 q.s.low = (n.s.high << (n_uword_bits - sr)) | (n.s.low >> sr);
 return q.all;
Index: compiler-rt/lib/builtins/fp_extend.h
===
--- compiler-rt/lib/builtins/fp_extend.h
+++ compiler-rt/lib/builtins/fp_extend.h
@@ -33,9 +33,9 @@
   return __builtin_clzl(a);
 #else
   if (a & REP_C(0x))
-return __builtin_clz(a >> 32);
+return clzsi(a >> 32);
   else
-return 32 + __builtin_clz(a & REP_C(0x));
+return 32 + clzsi(a & 

[PATCH] D86546: [compiler-rt][builtins] Use explicitly-sized integer types for LibCalls

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko created this revision.
atrosinenko added reviewers: efriedma, MaskRay, aykevl, uabelho.
Herald added subscribers: Sanitizers, dberris.
Herald added a project: Sanitizers.
atrosinenko requested review of this revision.

Use s[iu]_int instead of `(unsigned) int` and d[ui]_int instead of
`(unsigned) long long` for LibCall arguments.

Note: the `*vfp` LibCall versions were NOT touched.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86546

Files:
  compiler-rt/lib/builtins/floatsisf.c
  compiler-rt/lib/builtins/floatsitf.c
  compiler-rt/lib/builtins/floatunsisf.c
  compiler-rt/lib/builtins/floatunsitf.c
  compiler-rt/test/builtins/Unit/floatditf_test.c
  compiler-rt/test/builtins/Unit/floatsitf_test.c
  compiler-rt/test/builtins/Unit/floatunditf_test.c
  compiler-rt/test/builtins/Unit/floatunsitf_test.c

Index: compiler-rt/test/builtins/Unit/floatunsitf_test.c
===
--- compiler-rt/test/builtins/Unit/floatunsitf_test.c
+++ compiler-rt/test/builtins/Unit/floatunsitf_test.c
@@ -8,9 +8,9 @@
 
 #include "fp_test.h"
 
-COMPILER_RT_ABI long double __floatunsitf(unsigned int a);
+COMPILER_RT_ABI long double __floatunsitf(su_int a);
 
-int test__floatunsitf(unsigned int a, uint64_t expectedHi, uint64_t expectedLo)
+int test__floatunsitf(su_int a, uint64_t expectedHi, uint64_t expectedLo)
 {
 long double x = __floatunsitf(a);
 int ret = compareResultLD(x, expectedHi, expectedLo);
Index: compiler-rt/test/builtins/Unit/floatunditf_test.c
===
--- compiler-rt/test/builtins/Unit/floatunditf_test.c
+++ compiler-rt/test/builtins/Unit/floatunditf_test.c
@@ -12,9 +12,9 @@
 
 // Returns: long integer converted to long double
 
-COMPILER_RT_ABI long double __floatunditf(unsigned long long a);
+COMPILER_RT_ABI long double __floatunditf(du_int a);
 
-int test__floatunditf(unsigned long long a, uint64_t expectedHi, uint64_t expectedLo)
+int test__floatunditf(du_int a, uint64_t expectedHi, uint64_t expectedLo)
 {
 long double x = __floatunditf(a);
 int ret = compareResultLD(x, expectedHi, expectedLo);
Index: compiler-rt/test/builtins/Unit/floatsitf_test.c
===
--- compiler-rt/test/builtins/Unit/floatsitf_test.c
+++ compiler-rt/test/builtins/Unit/floatsitf_test.c
@@ -8,9 +8,9 @@
 
 #include "fp_test.h"
 
-long COMPILER_RT_ABI double __floatsitf(int a);
+COMPILER_RT_ABI long double __floatsitf(si_int a);
 
-int test__floatsitf(int a, uint64_t expectedHi, uint64_t expectedLo)
+int test__floatsitf(si_int a, uint64_t expectedHi, uint64_t expectedLo)
 {
 long double x = __floatsitf(a);
 int ret = compareResultLD(x, expectedHi, expectedLo);
Index: compiler-rt/test/builtins/Unit/floatditf_test.c
===
--- compiler-rt/test/builtins/Unit/floatditf_test.c
+++ compiler-rt/test/builtins/Unit/floatditf_test.c
@@ -12,9 +12,9 @@
 
 // Returns: long integer converted to long double
 
-COMPILER_RT_ABI long double __floatditf(long long a);
+COMPILER_RT_ABI long double __floatditf(di_int a);
 
-int test__floatditf(long long a, uint64_t expectedHi, uint64_t expectedLo)
+int test__floatditf(di_int a, uint64_t expectedHi, uint64_t expectedLo)
 {
 long double x = __floatditf(a);
 int ret = compareResultLD(x, expectedHi, expectedLo);
Index: compiler-rt/lib/builtins/floatunsitf.c
===
--- compiler-rt/lib/builtins/floatunsitf.c
+++ compiler-rt/lib/builtins/floatunsitf.c
@@ -16,7 +16,7 @@
 #include "fp_lib.h"
 
 #if defined(CRT_HAS_128BIT) && defined(CRT_LDBL_128BIT)
-COMPILER_RT_ABI fp_t __floatunsitf(unsigned int a) {
+COMPILER_RT_ABI fp_t __floatunsitf(su_int a) {
 
   const int aWidth = sizeof a * CHAR_BIT;
 
Index: compiler-rt/lib/builtins/floatunsisf.c
===
--- compiler-rt/lib/builtins/floatunsisf.c
+++ compiler-rt/lib/builtins/floatunsisf.c
@@ -17,7 +17,7 @@
 
 #include "int_lib.h"
 
-COMPILER_RT_ABI fp_t __floatunsisf(unsigned int a) {
+COMPILER_RT_ABI fp_t __floatunsisf(su_int a) {
 
   const int aWidth = sizeof a * CHAR_BIT;
 
Index: compiler-rt/lib/builtins/floatsitf.c
===
--- compiler-rt/lib/builtins/floatsitf.c
+++ compiler-rt/lib/builtins/floatsitf.c
@@ -16,7 +16,7 @@
 #include "fp_lib.h"
 
 #if defined(CRT_HAS_128BIT) && defined(CRT_LDBL_128BIT)
-COMPILER_RT_ABI fp_t __floatsitf(int a) {
+COMPILER_RT_ABI fp_t __floatsitf(si_int a) {
 
   const int aWidth = sizeof a * CHAR_BIT;
 
@@ -26,10 +26,10 @@
 
   // All other cases begin by extracting the sign and absolute value of a
   rep_t sign = 0;
-  unsigned aAbs = (unsigned)a;
+  su_int aAbs = (su_int)a;
   if (a < 0) {
 sign = signBit;
-aAbs = ~(unsigned)a + 1U;
+aAbs = ~(su_int)a + 

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

The real reason we don't see it internally is because we use -c for all 
compilation.


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

https://reviews.llvm.org/D85998

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

[PATCH] D86169: Initial support for letting plugins perform custom parsing of attribute arguments.

[Jonathan Protzenko via Phabricator via cfe-commits]

jonathan.protzenko added a comment.

I also seem to have broken a bunch of tests, although I'm not sure why. If you 
see an obvious cause, I'm happy to get some pointers, otherwise, I'll just dig 
in an debug as I extend the test suite once the patch stabilizes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86169

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

[PATCH] D86169: Initial support for letting plugins perform custom parsing of attribute arguments.

[Jonathan Protzenko via Phabricator via cfe-commits]

jonathan.protzenko added a comment.

Thanks for the review!

Regarding Declarator: I briefly mentioned something related in the details 
section... actually maybe you can tell me what's the best approach here. I was 
thinking of just passing in a void* and expecting plugins to do

  if (const Declarator *D = isa_and_nonnull(Context))

This would allow a future-proof API where we can, in the future, pass in more 
classes for the "Context" argument (not just a Declarator), relying on clients 
doing dynamic casts. If I understood 
https://llvm.org/docs/ProgrammersManual.html#isa this `void*` cast would work?

My only hesitation is I'm not sure about the right technical device to use for 
this polymorphism pattern, as I've seen a couple related things in the clang 
code base:

- I've seen a pattern where there's a wrapper class that has a Kind() method 
and then allows downcasting based on the results of the Kind (seems overkill 
here)
- I've also seen references to `opaque_ptr` somewhere with a `get` method... 
(maybe these are helpers to do what I'm doing better?)

If you can confirm the void* is the right way to go (or point me to a suitable 
pattern I can take inspiration from), I'll submit a revised patch, along with 
the extra spelling argument (good point, thanks!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86169

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

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Zachary Henkel via Phabricator via cfe-commits]

zahen marked an inline comment as done.
zahen added a comment.

In D85998#2236401 , @hans wrote:

> I'm still curious why it seems it's not looking for link.exe in the /fake dir 
> though.

clang\lib\Driver\ToolChains\MSVC.cpp:311

FindVisualStudioExecutable 
FilePath = /fake\bin\Hostx64\x64\link.exe 
The test llvm::sys::fs::can_execute(FilePath) fails and the function returns 
"link.exe".  This isn't seen in our environment because //absent path// probes 
are always allowed, it's only //actual file reads// that need to be tracked.


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

https://reviews.llvm.org/D85998

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

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

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

Szelethus marked an inline comment as done and an inline comment as not done.
Szelethus added a comment.

I'll get the nits I didn't object to fixed, thats the status you're looking for 
:)




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1338-1339
 void TokenPrinter::printToken(const Token ) {
+  // TODO: Handle the case where hash and hashhash occurs right before
+  // __VA_ARGS__.
+

steakhal wrote:
> Szelethus wrote:
> > steakhal wrote:
> > > What does //hashhash// mean? I might lack some context though :D
> > `#` and `##` respectively. The test cases you pointed out as flawed refer 
> > to this FIXME, though a FIXME in the tests themselves wouldn't hurt.
> Maybe `HashtagHashtag`?
> Or an example would be even better like: `##__VA_ARGS__`
If you look a few lines down, you can see that its not up to us to choose this 
one :^) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86135

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

[PATCH] D86544: [SyntaxTree] Add support for call expressions

[Eduardo Caldas via Phabricator via cfe-commits]

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

Add support for all C++ call expressions, including those coming from 
`operator()`.
A call expression's arguments are inside their own node - `CallArguments` - 
based on the `List` base API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86544

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -316,12 +316,12 @@
   `-CompoundStatement
 |-'{' OpenParen
 |-ExpressionStatement CompoundStatement_statement
-| |-UnknownExpression ExpressionStatement_expression
-| | |-IdExpression
+| |-CallExpression ExpressionStatement_expression
+| | |-IdExpression CallExpression_callee
 | | | `-UnqualifiedId IdExpression_id
 | | |   `-'test'
-| | |-'('
-| | `-')'
+| | |-'(' OpenParen
+| | `-')' CloseParen
 | `-';'
 |-IfStatement CompoundStatement_statement
 | |-'if' IntroducerKeyword
@@ -330,21 +330,21 @@
 | | `-'1' LiteralToken
 | |-')'
 | |-ExpressionStatement IfStatement_thenStatement
-| | |-UnknownExpression ExpressionStatement_expression
-| | | |-IdExpression
+| | |-CallExpression ExpressionStatement_expression
+| | | |-IdExpression CallExpression_callee
 | | | | `-UnqualifiedId IdExpression_id
 | | | |   `-'test'
-| | | |-'('
-| | | `-')'
+| | | |-'(' OpenParen
+| | | `-')' CloseParen
 | | `-';'
 | |-'else' IfStatement_elseKeyword
 | `-ExpressionStatement IfStatement_elseStatement
-|   |-UnknownExpression ExpressionStatement_expression
-|   | |-IdExpression
+|   |-CallExpression ExpressionStatement_expression
+|   | |-IdExpression CallExpression_callee
 |   | | `-UnqualifiedId IdExpression_id
 |   | |   `-'test'
-|   | |-'('
-|   | `-')'
+|   | |-'(' OpenParen
+|   | `-')' CloseParen
 |   `-';'
 `-'}' CloseParen
 )txt"));
@@ -378,20 +378,21 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-IdExpression
+CallExpression ExpressionStatement_expression
+|-IdExpression CallExpression_callee
 | `-UnqualifiedId IdExpression_id
 |   |-'operator'
 |   `-'+'
-|-'('
-|-IdExpression
-| `-UnqualifiedId IdExpression_id
-|   `-'x'
-|-','
-|-IdExpression
-| `-UnqualifiedId IdExpression_id
-|   `-'x'
-`-')'
+|-'(' OpenParen
+|-CallArguments CallExpression_arguments
+| |-IdExpression List_element
+| | `-UnqualifiedId IdExpression_id
+| |   `-'x'
+| |-',' List_delimiter
+| `-IdExpression List_element
+|   `-UnqualifiedId IdExpression_id
+| `-'x'
+`-')' CloseParen
 )txt"}));
 }
 
@@ -409,8 +410,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -419,8 +420,8 @@
 |   `-UnqualifiedId IdExpression_id
 | |-'operator'
 | `-'int'
-|-'('
-`-')'
+|-'(' OpenParen
+`-')' CloseParen
 )txt"}));
 }
 
@@ -436,16 +437,17 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-IdExpression
+CallExpression ExpressionStatement_expression
+|-IdExpression CallExpression_callee
 | `-UnqualifiedId IdExpression_id
 |   |-'operator'
 |   |-'""'
 |   `-'_w'
-|-'('
-|-CharacterLiteralExpression
-| `-''1'' LiteralToken
-`-')'
+|-'(' OpenParen
+|-CallArguments CallExpression_arguments
+| `-CharacterLiteralExpression List_element
+|   `-''1'' LiteralToken
+`-')' CloseParen
 )txt"}));
 }
 
@@ -461,8 +463,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -471,8 +473,8 @@
 |   `-UnqualifiedId IdExpression_id
 | |-'~'
 | `-'X'
-|-'('
-`-')'
+|-'(' OpenParen
+`-')' CloseParen
 )txt"}));
 }
 
@@ -492,8 +494,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -501,12 +503,12 @@
 | `-IdExpression MemberExpression_member
 |   `-UnqualifiedId IdExpression_id
 | `-'~'
-|-'decltype'
+|-'decltype' OpenParen
 |-'('
 |-'x'
 |-')'
 |-'('
-`-')'
+`-')' CloseParen
 )txt"}));
 }
 
@@ -523,15 +525,15 @@
 }
 )cpp",
   

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 287680.
martong added a comment.

- Use FileCheck in the lit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86531

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX-lookup.c

Index: clang/test/Analysis/std-c-library-functions-POSIX-lookup.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-POSIX-lookup.c
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s --allow-empty
+
+// We test here the implementation of our summary API with Optional types. In
+// this TU we do not provide declaration for any of the functions that have
+// summaries. The implementation should be able to handle the nonexistent
+// declarations in a way that the summary is not added to the map. We expect no
+// crashes (i.e. no optionals should be 'dereferenced') and no output.
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
+
+// CHECK-NOT: Loaded summary for:
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -314,7 +314,8 @@
   /// The complete list of constraints that defines a single branch.
   typedef std::vector ConstraintSet;
 
-  using ArgTypes = std::vector;
+  using ArgTypes = std::vector>;
+  using RetType = Optional;
 
   // A placeholder type, we use it whenever we do not care about the concrete
   // type in a Signature.
@@ -324,17 +325,37 @@
   // The signature of a function we want to describe with a summary. This is a
   // concessive signature, meaning there may be irrelevant types in the
   // signature which we do not check against a function with concrete types.
-  struct Signature {
-ArgTypes ArgTys;
+  // All types in the spec need to be canonical.
+  class Signature {
+using ArgQualTypes = std::vector;
+ArgQualTypes ArgTys;
 QualType RetTy;
-Signature(ArgTypes ArgTys, QualType RetTy) : ArgTys(ArgTys), RetTy(RetTy) {
-  assertRetTypeSuitableForSignature(RetTy);
-  for (size_t I = 0, E = ArgTys.size(); I != E; ++I) {
-QualType ArgTy = ArgTys[I];
-assertArgTypeSuitableForSignature(ArgTy);
+// True if any component type is not found by lookup.
+bool Invalid = false;
+
+  public:
+// Construct a signature from optional types. If any of the optional types
+// are not set then the signature will be invalid.
+Signature(ArgTypes ArgTys, RetType RetTy) {
+  for (Optional Arg : ArgTys) {
+if (!Arg) {
+  Invalid = true;
+  return;
+} else {
+  assertArgTypeSuitableForSignature(*Arg);
+  this->ArgTys.push_back(*Arg);
+}
+  }
+  if (!RetTy) {
+Invalid = true;
+return;
+  } else {
+assertRetTypeSuitableForSignature(*RetTy);
+this->RetTy = *RetTy;
   }
 }
 
+bool isInvalid() const { return Invalid; }
 bool matches(const FunctionDecl *FD) const;
 
   private:
@@ -388,6 +409,9 @@
   ///   rules for the given parameter's type, those rules are checked once the
   ///   signature is matched.
   class Summary {
+// FIXME Probably the Signature should not be part of the Summary,
+// We can remove once all overload of addToFunctionSummaryMap requires the
+// Signature explicitly given.
 Optional Sign;
 const InvalidationKind InvalidationKd;
 Cases CaseConstraints;
@@ -398,11 +422,13 @@
 const FunctionDecl *FD = nullptr;
 
   public:
-Summary(ArgTypes ArgTys, QualType RetTy, InvalidationKind InvalidationKd)
+Summary(ArgTypes ArgTys, RetType RetTy, InvalidationKind InvalidationKd)
 : Sign(Signature(ArgTys, RetTy)), InvalidationKd(InvalidationKd) {}
 
 Summary(InvalidationKind InvalidationKd) : InvalidationKd(InvalidationKd) {}
 
+// FIXME Remove, once all overload of addToFunctionSummaryMap requires the
+// Signature explicitly given.
 Summary (const Signature ) {
   Sign = S;
   return *this;
@@ -438,6 +464,13 @@
   return Result;
 }
 
+// FIXME Remove, once all overload of addToFunctionSummaryMap requires the
+// Signature explicitly given.
+bool hasInvalidSignature() {
+  

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

[Lucas Prates via Phabricator via cfe-commits]

pratlucas added a comment.

Hi @hans , I'll have a look at it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75169

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

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Seems reasonable to me.

I'm still curious why it seems it's not looking for link.exe in the /fake dir 
though.


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

https://reviews.llvm.org/D85998

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

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/test/SemaCXX/ms_dynamic_cast.cpp:16
+B *b = new D1();
+auto d = dynamic_cast(b); // expected-warning{{should not use 
dynamic_cast with /GR-}}
+}

zequanwu wrote:
> lebedev.ri wrote:
> > I'm not sure it makes sense to talk about MSVC/clang-cl flags when normal 
> > clang is used.
> > Either the diag should only be used in MSVC compat mode,
> > or it should talk about `-fno-rtti` in non-MSVC compat mode.
> I am not sure either... But `-fno-rtti-data` is only used when clang-cl 
> driver translate `/GR-` to `-fno-rtti-data`.
Instead of "should not use" maybe the warning could say something which 
suggests it won't work, like "dynamic_cast will not work since RTTI data is 
disabled" or something like that.

I'm not sure what to do about the name of the option. -fno-rtti-data was added 
to support clang-cl's /GR- option, but it could also be used with regular 
Clang. Maybe the warning doesn't need to mention the name of the flag? Or it 
could mention both? Or maybe it could figure out if it's in clang-cl or regular 
clang mode?

Also, should it also warn about typeid()?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

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

[PATCH] D86065: [SVE] Make ElementCount members private

[David Sherwood via Phabricator via cfe-commits]

david-arm updated this revision to Diff 287672.
david-arm added a comment.
Herald added a subscriber: rogfer01.

- Changed comparison function from gt to ogt and added a olt (less than) 
comparison function too.
- Instead of adding the ">>=" operator I've added "/=" instead as I think this 
is more common. In places where ">>= 1" was used we now do "/= 2".
- After rebasing it was necessary to add a "*=" operator too for the Loop 
Vectorizer.


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

https://reviews.llvm.org/D86065

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/Analysis/VectorUtils.h
  llvm/include/llvm/CodeGen/ValueTypes.h
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/DerivedTypes.h
  llvm/include/llvm/IR/Instructions.h
  llvm/include/llvm/Support/MachineValueType.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/lib/Analysis/VFABIDemangling.cpp
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/TargetLoweringBase.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/ConstantFold.cpp
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/lib/IR/Type.cpp
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/lib/Transforms/Vectorize/VPlan.cpp
  llvm/lib/Transforms/Vectorize/VPlan.h
  llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
  llvm/unittests/IR/VectorTypesTest.cpp

Index: llvm/unittests/IR/VectorTypesTest.cpp
===
--- llvm/unittests/IR/VectorTypesTest.cpp
+++ llvm/unittests/IR/VectorTypesTest.cpp
@@ -119,8 +119,8 @@
   EXPECT_EQ(ConvTy->getElementType()->getScalarSizeInBits(), 64U);
 
   EltCnt = V8Int64Ty->getElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_FALSE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_FALSE(EltCnt.isScalable());
 }
 
 TEST(VectorTypesTest, Scalable) {
@@ -215,8 +215,8 @@
   EXPECT_EQ(ConvTy->getElementType()->getScalarSizeInBits(), 64U);
 
   EltCnt = ScV8Int64Ty->getElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_TRUE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_TRUE(EltCnt.isScalable());
 }
 
 TEST(VectorTypesTest, BaseVectorType) {
@@ -250,7 +250,7 @@
 // test I == J
 VectorType *VI = VTys[I];
 ElementCount ECI = VI->getElementCount();
-EXPECT_EQ(isa(VI), ECI.Scalable);
+EXPECT_EQ(isa(VI), ECI.isScalable());
 
 for (size_t J = I + 1, JEnd = VTys.size(); J < JEnd; ++J) {
   // test I < J
Index: llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
===
--- llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
+++ llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
@@ -71,8 +71,8 @@
 
   // Check fields inside llvm::ElementCount
   EltCnt = Vnx4i32.getVectorElementCount();
-  EXPECT_EQ(EltCnt.Min, 4U);
-  ASSERT_TRUE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 4U);
+  ASSERT_TRUE(EltCnt.isScalable());
 
   // Check that fixed-length vector types aren't scalable.
   EVT V8i32 = EVT::getVectorVT(Ctx, MVT::i32, 8);
@@ -82,8 +82,8 @@
 
   // Check that llvm::ElementCount works for fixed-length types.
   EltCnt = V8i32.getVectorElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_FALSE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_FALSE(EltCnt.isScalable());
 }
 
 TEST(ScalableVectorMVTsTest, IRToVTTranslation) {
Index: llvm/lib/Transforms/Vectorize/VPlan.h
===
--- llvm/lib/Transforms/Vectorize/VPlan.h
+++ llvm/lib/Transforms/Vectorize/VPlan.h
@@ -151,14 +151,15 @@
   /// \return True if the map has a scalar entry for \p Key and \p Instance.
   bool hasScalarValue(Value *Key, const VPIteration ) const {
 assert(Instance.Part < UF && "Queried Scalar Part is too large.");
-assert(Instance.Lane < VF.Min && "Queried Scalar Lane is too large.");
-assert(!VF.Scalable && "VF is assumed to be non scalable.");
+assert(Instance.Lane < VF.getKnownMinValue() &&
+   "Queried Scalar Lane is too large.");
+assert(!VF.isScalable() && "VF is assumed to be non scalable.");
 
 if (!hasAnyScalarValue(Key))
   return false;
 const ScalarParts  = ScalarMapStorage.find(Key)->second;
 assert(Entry.size() == UF && 

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Zachary Henkel via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/cl-options.c:686
+// vctoolsdir is handled by the driver; just check that we don't error. Pass 
-c because fakedir isn't a real toolchain path
+// RUN: %clang_cl -c -vctoolsdir fakedir -- %s 2>&1
+

hans wrote:
> zahen wrote:
> > zahen wrote:
> > > hans wrote:
> > > > zahen wrote:
> > > > > hans wrote:
> > > > > > aganea wrote:
> > > > > > > Check that we're not detecting a local installation, and that we 
> > > > > > > fallback to the default triple 19.11, ie.
> > > > > > > ```
> > > > > > > // RUN: %clang_cl ... -vctoolsdir "" -### | FileCheck %s 
> > > > > > > --check-prefix VCTOOLSDIR
> > > > > > > // VCTOOLSDIR: "-cc1" "-triple" "x86_64-pc-windows-msvc19.11.0"
> > > > > > > ```
> > > > > > Maybe add a comment explaining the purpose of the test.
> > > > > > 
> > > > > > And would it be possible to have a test where -vctoolsdir is set to 
> > > > > > something, to see that it's picked up correctly?
> > > > > What's the expectation on the test boxes?  I can author such a test 
> > > > > but it would be very brittle unless we have a spec for how VS should 
> > > > > be installed.
> > > > I'd suggest passing -vctoolsdir "/foo" and check that some /foo dir is 
> > > > getting passed as a system include dir to cc1, and that the link.exe 
> > > > invocation is under /foo.
> > > > 
> > > > I don't think it would be brittle.
> > > Clever idea! I'll do it that way and key off the "ignored directory" 
> > > warning that's emitted.
> > Did further digging and the path test would depend on other environment 
> > variables.  If the %INCLUDE% variable is set, as it is by vcvars.bat, those 
> > paths are adopted and VCToolChainPath is not used.  I'm hesitant to change 
> > that logic without stronger motivation.
> > 
> > Other ideas for a test that would work run both from a vc command prompt 
> > and a vanilla command prompt?  I don't see anything else in the full 
> > verbose invocation to key off of:
> > 
> > 
> > ```
> > D:\repos\llvm\llvm-project\build\Debug\bin>clang-cl -vctoolsdir "/fake" -v 
> > main.cpp
> > clang version 12.0.0 (https://github.com/llvm/llvm-project 
> > 537f5483fe4e09c39ffd7ac837c00b50dd13d676)
> > Target: x86_64-pc-windows-msvc
> > Thread model: posix
> > InstalledDir: D:\repos\llvm\llvm-project\build\Debug\bin
> >  "D:\\repos\\llvm\\llvm-project\\build\\Debug\\bin\\clang-cl.exe" -cc1 
> > -triple x86_64-pc-windows-msvc19.11.0 -emit-obj -mrelax-all 
> > -mincremental-linker-compatible --mrelax-relocations -disable-free 
> > -main-file-name main.cpp -mrelocation-model pic -pic-level 2 
> > -mframe-pointer=none -relaxed-aliasing -fmath-errno -fno-rounding-math 
> > -mconstructor-aliases -munwind-tables -target-cpu x86-64 -mllvm 
> > -x86-asm-syntax=intel -D_MT -flto-visibility-public-std 
> > --dependent-lib=libcmt --dependent-lib=oldnames -stack-protector 2 
> > -fms-volatile -fdiagnostics-format msvc -v -resource-dir 
> > "D:\\repos\\llvm\\llvm-project\\build\\Debug\\lib\\clang\\12.0.0" 
> > -internal-isystem 
> > "D:\\repos\\llvm\\llvm-project\\build\\Debug\\lib\\clang\\12.0.0\\include" 
> > -internal-isystem "C:\\Program Files (x86)\\Microsoft Visual 
> > Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.27.29110\\ATLMFC\\include" 
> > -internal-isystem "C:\\Program Files (x86)\\Microsoft Visual 
> > Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.27.29110\\include" 
> > -internal-isystem "C:\\Program Files (x86)\\Windows 
> > Kits\\NETFXSDK\\4.8\\include\\um" -internal-isystem "C:\\Program Files 
> > (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\ucrt" -internal-isystem 
> > "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\shared" 
> > -internal-isystem "C:\\Program Files (x86)\\Windows 
> > Kits\\10\\include\\10.0.18362.0\\um" -internal-isystem "C:\\Program Files 
> > (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\winrt" -internal-isystem 
> > "C:\\Program Files (x86)\\Windows 
> > Kits\\10\\include\\10.0.18362.0\\cppwinrt" -fdeprecated-macro 
> > -fdebug-compilation-dir "D:\\repos\\llvm\\llvm-project\\build\\Debug\\bin" 
> > -ferror-limit 19 -fmessage-length=121 -fno-use-cxa-atexit -fms-extensions 
> > -fms-compatibility -fms-compatibility-version=19.11 -std=c++14 
> > -fdelayed-template-parsing -fcolor-diagnostics -faddrsig -o 
> > "C:\\Users\\zahen\\AppData\\Local\\Temp\\main-8f329a.obj" -x c++ main.cpp
> > clang -cc1 version 12.0.0 based upon LLVM 12.0.0git default target 
> > x86_64-pc-windows-msvc
> > #include "..." search starts here:
> > #include <...> search starts here:
> >  D:\repos\llvm\llvm-project\build\Debug\lib\clang\12.0.0\include
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\ATLMFC\include
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\include
> >  C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um
> >  

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Zachary Henkel via Phabricator via cfe-commits]

zahen updated this revision to Diff 287671.
zahen added a comment.
Herald added a subscriber: ormris.

It felt too invasive to swap the precedence of using vctoolsdir vs %INCLUDE% 
for all clang-cl users so I compromised by skipping the %INCLUDE% check when 
vctoolsdir was passed on the command line.  This way only the users of the new 
flag will see different behavior.

With that change the suggested test for -internal-isystem passes in both 
vanilla and vc command prompts.


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

https://reviews.llvm.org/D85998

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/cl-options.c

Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -682,4 +682,13 @@
 // CLANG-NOT: "--dependent-lib=libcmt"
 // CLANG-NOT: "-vectorize-slp"
 
+// Validate that the default triple is used when run an empty tools dir is specified
+// RUN: %clang_cl -vctoolsdir "" -### -- %s 2>&1 | FileCheck %s --check-prefix VCTOOLSDIR
+// VCTOOLSDIR: "-cc1" "-triple" "x86_64-pc-windows-msvc19.11.0"
+
+// Validate that built-in include paths are based on the supplied path
+// RUN: %clang_cl -vctoolsdir "/fake" -### -- %s 2>&1 | FileCheck %s --check-prefix FAKEDIR
+// FAKEDIR: "-internal-isystem" "/fake\\include"
+// FAKEDIR: "-internal-isystem" "/fake\\atlmfc\\include"
+
 void f() { }
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -66,6 +66,20 @@
 static bool getSystemRegistryString(const char *keyPath, const char *valueName,
 std::string , std::string *phValue);
 
+// Check command line arguments to try and find a toolchain.
+static bool
+findVCToolChainViaCommandLine(const ArgList , std::string ,
+  MSVCToolChain::ToolsetLayout ) {
+  // Don't validate the input; trust the value supplied by the user.
+  // The primary motivation is to prevent unnecessary file and registry access.
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_vctoolsdir)) {
+Path = A->getValue();
+VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
+return true;
+  }
+  return false;
+}
+
 // Check various environment variables to try and find a toolchain.
 static bool findVCToolChainViaEnvironment(std::string ,
   MSVCToolChain::ToolsetLayout ) {
@@ -747,11 +761,12 @@
   if (getDriver().getInstalledDir() != getDriver().Dir)
 getProgramPaths().push_back(getDriver().Dir);
 
-  // Check the environment first, since that's probably the user telling us
-  // what they want to use.
-  // Failing that, just try to find the newest Visual Studio version we can
-  // and use its default VC toolchain.
-  findVCToolChainViaEnvironment(VCToolChainPath, VSLayout) ||
+  // Check the command line first, that's the user explicitly telling us what to
+  // use. Check the environment next, in case we're being invoked from a VS
+  // command prompt. Failing that, just try to find the newest Visual Studio
+  // version we can and use its default VC toolchain.
+  findVCToolChainViaCommandLine(Args, VCToolChainPath, VSLayout) ||
+  findVCToolChainViaEnvironment(VCToolChainPath, VSLayout) ||
   findVCToolChainViaSetupConfig(VCToolChainPath, VSLayout) ||
   findVCToolChainViaRegistry(VCToolChainPath, VSLayout);
 }
@@ -1263,15 +1278,18 @@
 return;
 
   // Honor %INCLUDE%. It should know essential search paths with vcvarsall.bat.
-  if (llvm::Optional cl_include_dir =
-  llvm::sys::Process::GetEnv("INCLUDE")) {
-SmallVector Dirs;
-StringRef(*cl_include_dir)
-.split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
-for (StringRef Dir : Dirs)
-  addSystemInclude(DriverArgs, CC1Args, Dir);
-if (!Dirs.empty())
-  return;
+  // Skip if the user expressly set a vctoolsdir
+  if (!DriverArgs.getLastArg(options::OPT__SLASH_vctoolsdir)) {
+if (llvm::Optional cl_include_dir =
+llvm::sys::Process::GetEnv("INCLUDE")) {
+  SmallVector Dirs;
+  StringRef(*cl_include_dir)
+  .split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+  for (StringRef Dir : Dirs)
+addSystemInclude(DriverArgs, CC1Args, Dir);
+  if (!Dirs.empty())
+return;
+}
   }
 
   // When built with access to the proper Windows APIs, try to actually find
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4737,6 +4737,8 @@
 def _SLASH_Tp : CLCompileJoinedOrSeparate<"Tp">,
   HelpText<"Treat  as C++ source file">, MetaVarName<"">;
 def _SLASH_TP : CLCompileFlag<"TP">, 

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan added a comment.

In D85324#2234712 , 
@hubert.reinterpretcast wrote:

> In D85324#2233290 , 
> @abhina.sreeskantharajan wrote:
>
>> Thanks Hubert, I fixed the comment.
>
> Got it; I'll look into committing this.

Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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

[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 287669.
ASDenysPetrov added a comment.

Added recursive_mutex support.


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

https://reviews.llvm.org/D85984

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp

Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1,207 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+struct mutex {
+  void lock();
+  bool try_lock();
+  void unlock();
+};
+
+struct recursive_mutex {
+  void lock();
+  bool try_lock();
+  void unlock();
+};
+
+template 
+struct guard_lock {
+  T 
+  guard_lock(T ) : t(m) {
+t.lock();
+  }
+  ~guard_lock() {
+t.unlock();
+  }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+
+// mutex ok
+
+void m_ok1() {
+  m1.lock(); // no-warning
+}
+
+void m_ok2() {
+  m1.unlock(); // no-warning
+}
+
+void m_ok3() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+}
+
+void m_ok4() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+}
+
+void m_ok5() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m2.lock();   // no-warning
+  m2.unlock(); // no-warning
+}
+
+void m_ok6(void) {
+  m1.lock();   // no-warning
+  m2.lock();   // no-warning
+  m2.unlock(); // no-warning
+  m1.unlock(); // no-warning
+}
+
+void m_ok7(void) {
+  if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok8(void) {
+  m1.unlock();   // no-warning
+  if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok9(void) {
+  if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+  m1.unlock();// no-warning
+}
+
+void m_ok10() {
+  std::guard_lock gl1(m1); // no-warning
+}
+
+// mutex bad
+
+void m_bad1() {
+  m1.lock(); // no-warning
+  m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad2() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad3() {
+  m1.lock();   // no-warning
+  m2.lock();   // no-warning
+  m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+  m2.unlock(); // no-warning
+}
+
+void m_bad5() {
+  while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad6() {
+  while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+std::recursive_mutex rm1;
+std::recursive_mutex rm2;
+
+// recursive_mutex ok
+
+void rm_ok1() {
+  rm1.lock(); // no-warning
+}
+
+void rm_ok2() {
+  rm1.unlock(); // no-warning
+}
+
+void rm_ok3() {
+  rm1.lock();   // no-warning
+  rm1.unlock(); // no-warning
+}
+
+void rm_ok4() {
+  rm1.lock();   // no-warning
+  rm1.unlock(); // no-warning
+  rm1.lock();   // no-warning
+  rm1.unlock(); // no-warning
+}
+
+void rm_ok5() {
+  rm1.lock();   // no-warning
+  rm1.unlock(); // no-warning
+  rm2.lock();   // no-warning
+  rm2.unlock(); // no-warning
+}
+
+void rm_ok6(void) {
+  rm1.lock();   // no-warning
+  rm2.lock();   // no-warning
+  rm2.unlock(); // no-warning
+  rm1.unlock(); // no-warning
+}
+
+void rm_ok7(void) {
+  if (rm1.try_lock()) // no-warning
+rm1.unlock(); // no-warning
+}
+
+void rm_ok8(void) {
+  rm1.unlock();   // no-warning
+  if (rm1.try_lock()) // no-warning
+rm1.unlock(); // no-warning
+}
+
+void rm_ok9(void) {
+  if (!rm1.try_lock()) // no-warning
+rm1.lock();// no-warning
+  rm1.unlock();// no-warning
+}
+
+void rm_ok10() {
+  std::guard_lock gl2(rm1); // no-warning
+}
+
+void rm_ok11() {
+  rm1.lock(); // no-warning
+  rm1.lock(); // no-warning
+}
+
+void rm_ok12() {
+  while (true)
+rm1.lock(); // no-warning
+}
+
+// recursive_mutex bad
+
+void rm_bad1() {
+  rm1.lock();   // no-warning
+  rm1.unlock(); // no-warning
+  rm1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void rm_bad2() {
+  rm1.lock();   // no-warning
+  rm2.lock();   // no-warning
+  rm1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+  rm2.unlock(); // no-warning
+}
+
+void rm_bad3() {
+  while (true)
+rm1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -70,11 +70,17 

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

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

steakhal added a comment.

I'm not sure about the //status// of this patch.
If you say that further improvements will be done later and this functionality 
is enough, I'm fine with that.




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:901
+
+  void next(Token ) {
+if (CurrTokenIt == TokenRange.end()) {

Szelethus wrote:
> steakhal wrote:
> > I don't like output parameters.
> > If we really need them, we should at least have a suspicious name.
> > Unfortunately, I can not come up with anything useful :|
> This is intentional, it meant to replicate the `Lexer`'s interface. I would 
> prefer to keep this as-is.
Sure, be it.



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1338-1339
 void TokenPrinter::printToken(const Token ) {
+  // TODO: Handle the case where hash and hashhash occurs right before
+  // __VA_ARGS__.
+

Szelethus wrote:
> steakhal wrote:
> > What does //hashhash// mean? I might lack some context though :D
> `#` and `##` respectively. The test cases you pointed out as flawed refer to 
> this FIXME, though a FIXME in the tests themselves wouldn't hurt.
Maybe `HashtagHashtag`?
Or an example would be even better like: `##__VA_ARGS__`



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:481
+  i = 0;
+#define DISPATCH(...) PARAMS_RESOLVE_TO_VA_ARGS(__VA_ARGS__);
+

Szelethus wrote:
> steakhal wrote:
> > You don't need an ending semicolon here. It will be already there at the 
> > expansion location.
> > This way you introduce an empty expression after the macro expansion.
> > The same happens in all the other cases as well.
> You are correct, though the point of macro expansion testing is to see 
> whether we nailed what the preprocessor is supposed to do -- not whether the 
> code it creates makes such sense. In fact, I would argue that most GNU 
> extensions to the preprocessor shouldn't be a thing, but we still need to 
> support it.
Oh, now I get it.
I didn't know that this was ann extension lol.



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:484-486
+  int x = 1;
+  DISPATCH(x, "LF1M healer");
+  (void)(10 / x); // expected-warning{{Division by zero}}

Szelethus wrote:
> steakhal wrote:
> > Should we really abuse the division by zero checker here?
> > Can't we just use an ExprInspection call here?
> > Maybe it requires a specific BugPath visitor, and that is why we do it this 
> > way?
> We could totally use `ExprInspection` -- but I'd argue that using something 
> else isn't an abuse of the specific checker :) Since the entire file is 
> already written this way, and would demand changes in the large plist file, 
> I'd prefer to keep it this way.
Perfectly fine. I agree with you knowing this. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86135

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

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Lucas, this seems to have casued https://bugs.llvm.org/show_bug.cgi?id=47001. 
Can you take a look? (I would cc you on the bug, but I couldn't find your email 
in bugzilla.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75169

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

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

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

steakhal added a comment.

This is a huge change, I've got pretty tired at the end of the review.
I haven't checked the test-cases too deeply.

I've found only minor typos, clarity enhancement opportunities etc. nothing 
serious.
It was awesome to see how this code can be improved, even becoming 
comprehensible.

Sorry for my nit spam if it needs further discussion - but it looks good to me.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:35-36
 
-  Range(const llvm::APSInt )
-  : std::pair(, ) 
{}
+  Range(const llvm::APSInt )
+  : std::pair(, ) 
{}
 

I don't think we should inherit from standard type. It should be a private 
member instead.
https://quuxplusone.github.io/blog/2018/12/11/dont-inherit-from-std-types/#never-ever-inherit-from-any-std



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:51
   }
-};
+  void print(raw_ostream ) const;
 

We have `dump` methods all over the place. Except this.
I propose to rename this to `dump`.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:53-55
+  // In order to keep non-overlapping ranges sorted, we can compare only From
+  // points.
+  inline bool operator<(const Range ) const { return From() < RHS.From(); }

It's a good practice to define comparison operators as //friend// functions 
inline.
Even if we don't rely on implicit conversions.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:110
+  iterator end() const { return Impl->end(); }
+  size_t size() const { return Impl->size(); }
+

I think it should be `std::size_t`.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:118-123
+/// Create a new set with all ranges from both LHS and RHS.
+/// Possible intersections are not checked here.
+///
+/// Complexity: O(N + M)
+/// where N = size(LHS), M = size(RHS)
+RangeSet add(RangeSet LHS, RangeSet RHS);

What about `merge` or `combine`? I know that `union` is a keyword thus we can 
not use.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:135
+/// @{
+RangeSet getSet(Range Origin);
+RangeSet getSet(const llvm::APSInt , const llvm::APSInt ) {

NoQ wrote:
> "But what about `setGet()`???" - some user, probably :)
Why don't we call this `createSetOf`?
And `createEmptySet`.
I know that we don't create the empty set, but then what does a //factory// if 
not create stuff?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:149-160
+/// Intersect the given set with the closed range [Lower, Upper].
+///
+/// Unlike the Range type, this range uses modular arithmetic, 
corresponding
+/// to the common treatment of C integer overflow. Thus, if the Lower bound
+/// is greater than the Upper bound, the range is taken to wrap around. 
This
+/// is equivalent to taking the intersection with the two ranges [Min,
+/// Upper] and [Lower, Max], or, alternatively, /removing/ all integers

I think an example would be awesome here.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:170-174
+/// Delete the given point from the range set.
+///
+/// Complexity: O(N)
+/// where N = size(From)
+RangeSet deletePoint(const llvm::APSInt , RangeSet From);

Why does this operation take `O(N)` time? Shouldn't be `O(logN)` instead?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:190-193
+/// And it makes us to separate the range
+/// like [MIN, N] to [MIN, MIN] U [-N, MAX].
+/// For instance, whole range is {-128..127} and subrange is [-128,-126],
+/// thus [-128,-127,-126,...] negates to [-128,...,126,127].





Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:195-196
+///
+/// Negate restores disrupted ranges on bounds,
+/// e.g. [MIN, B] => [MIN, MIN] U [-B, MAX] => [MIN, B].
+///





Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:203
+  private:
+RangeSet makePersistent(ContainerType &);
+ContainerType *construct(ContainerType &);

As commented later, this function worth to be documented.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:207-208
+
+RangeSet intersect(const RangeSet::ContainerType ,
+   

[PATCH] D86065: [SVE] Make ElementCount members private

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D86065#2235434 , @david-arm wrote:

> Hi @fpetrogalli, if you don't mind I think I'll stick with Paul's idea for 
> ogt because this matches the IR neatly, i.e. "fcmp ogt". Also, for me 
> personally it's much simpler and more intuitive.

I don't mind at all, please follow Paul's suggestion, mine was just an 
alternative. Thanks!


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

https://reviews.llvm.org/D86065

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

[PATCH] D85934: Enable OpenMP offloading to VE and enable tests for offloading to VE

[Manoel Roemmer via Phabricator via cfe-commits]

manorom updated this revision to Diff 287660.
manorom edited the summary of this revision.
manorom added a comment.

- Move VE specific vars into test run line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85934

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  openmp/libomptarget/CMakeLists.txt
  openmp/libomptarget/test/api/omp_get_num_devices.c
  openmp/libomptarget/test/api/omp_get_num_devices_with_empty_target.c
  openmp/libomptarget/test/env/omp_target_debug.c
  openmp/libomptarget/test/lit.cfg
  openmp/libomptarget/test/mapping/alloc_fail.c
  openmp/libomptarget/test/mapping/target_implicit_partial_map.c
  openmp/libomptarget/test/offloading/offloading_success.c

Index: openmp/libomptarget/test/offloading/offloading_success.c
===
--- openmp/libomptarget/test/offloading/offloading_success.c
+++ openmp/libomptarget/test/offloading/offloading_success.c
@@ -3,6 +3,7 @@
 // RUN: %libomptarget-compile-run-and-check-powerpc64le-ibm-linux-gnu
 // RUN: %libomptarget-compile-run-and-check-x86_64-pc-linux-gnu
 // RUN: %libomptarget-compile-run-and-check-nvptx64-nvidia-cuda
+// RUN: %libomptarget-compile-ve-unknown-linux-unknown | env VE_LD_LIBRARY_PATH=%host-rtl-dir %libomptarget-run-ve-unknown-linux-unknown | %fcheck-ve-unknown-linux-unknown
 
 #include 
 #include 
Index: openmp/libomptarget/test/mapping/target_implicit_partial_map.c
===
--- openmp/libomptarget/test/mapping/target_implicit_partial_map.c
+++ openmp/libomptarget/test/mapping/target_implicit_partial_map.c
@@ -13,6 +13,10 @@
 // RUN: %libomptarget-compile-x86_64-pc-linux-gnu
 // RUN: %libomptarget-run-x86_64-pc-linux-gnu 2>&1 \
 // RUN: | %fcheck-x86_64-pc-linux-gnu
+
+// RUN: %libomptarget-compile-ve-unknown-linux-unknown
+// RUN: env VE_LD_LIBRARY_PATH=%host-rtl-dir %libomptarget-run-ve-unknown-linux-unknown 2>&1 \
+// RUN: | %fcheck-ve-unknown-linux-unknown
 //
 // END.
 
Index: openmp/libomptarget/test/mapping/alloc_fail.c
===
--- openmp/libomptarget/test/mapping/alloc_fail.c
+++ openmp/libomptarget/test/mapping/alloc_fail.c
@@ -18,6 +18,10 @@
 // RUN: %libomptarget-run-fail-nvptx64-nvidia-cuda 2>&1 \
 // RUN: | %fcheck-nvptx64-nvidia-cuda
 
+// RUN: %libomptarget-compile-ve-unknown-linux-unknown
+// RUN: env VE_LD_LIBRARY_PATH=%host-rtl-dir %libomptarget-run-fail-ve-unknown-linux-unknown 2>&1 \
+// RUN: | %fcheck-ve-unknown-linux-unknown
+
 // CHECK: Libomptarget fatal error 1: failure of target construct while offloading is mandatory
 
 int main() {
Index: openmp/libomptarget/test/lit.cfg
===
--- openmp/libomptarget/test/lit.cfg
+++ openmp/libomptarget/test/lit.cfg
@@ -75,6 +75,11 @@
 append_dynamic_library_path('LIBRARY_PATH', \
 config.omp_host_rtl_directory, ":")
 
+# Target specific flags
+target_flags = {}
+target_flags['ve-unknown-linux-unknown'] = "-Xopenmp-target \"-L " + \
+config.omp_host_rtl_directory + "\""
+
 # substitutions
 # - for targets that exist in the system create the actual command.
 # - for valid targets that do not exist in the system, return false, so that the
@@ -116,9 +121,13 @@
 libomptarget_target, \
 "%not --crash %t-" + libomptarget_target))
 config.substitutions.append(("%clangxx-" + libomptarget_target, \
-"%clangxx %openmp_flags %flags -fopenmp-targets=" + libomptarget_target))
+"%clangxx %openmp_flags %flags-" + libomptarget_target + \
+" -fopenmp-targets=" + libomptarget_target))
 config.substitutions.append(("%clang-" + libomptarget_target, \
-"%clang %openmp_flags %flags -fopenmp-targets=" + libomptarget_target))
+"%clang %openmp_flags %flags-" + libomptarget_target + \
+" -fopenmp-targets=" + libomptarget_target))
+config.substitutions.append(("%flags-" + libomptarget_target, \
+"%flags " + target_flags.get(libomptarget_target, "")))
 config.substitutions.append(("%fcheck-" + libomptarget_target, \
 config.libomptarget_filecheck + " %s"))
 else:
@@ -157,4 +166,5 @@
 config.substitutions.append(("%clang", config.test_c_compiler))
 config.substitutions.append(("%openmp_flags", config.test_openmp_flags))
 config.substitutions.append(("%flags", config.test_flags))
+config.substitutions.append(("%host-rtl-dir", config.omp_host_rtl_directory))
 config.substitutions.append(("%not", config.libomptarget_not))
Index: openmp/libomptarget/test/env/omp_target_debug.c
===
--- openmp/libomptarget/test/env/omp_target_debug.c
+++ openmp/libomptarget/test/env/omp_target_debug.c
@@ -8,6 +8,8 @@
 // RUN: %libomptarget-compile-x86_64-pc-linux-gnu && 

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 287658.
simoll added a comment.

- made bool-vector layout target dependent (one byte per bool element on 
Hexagon, one bit everywhere else).
- Added hvx-specific debug info test.
- Updated docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool-hvx64.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-conditional.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int _to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- clang/test/SemaCXX/vector-conditional.cpp
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((__vector_size__(2)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' 

[PATCH] D85485: Fix quiet mode in git-clang-format

[Gvald Ike via Phabricator via cfe-commits]

Gvald added a comment.

@serge-sans-paille - the diff is PEP8 compliant now.
Care to commit, as I don't have the right permissions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85485

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