[clang] bb1d8bf - [X86] Add CLWB to Tremont CPU. Remove CLDEMOTE, MOVDIRI, MOVDIR64B, and WAITPKG to match gcc.

2020-06-02 Thread Craig Topper via cfe-commits

Author: Craig Topper
Date: 2020-06-02T22:38:51-07:00
New Revision: bb1d8bf2707bdca89c1f5e719057f1000232ccc3

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

LOG: [X86] Add CLWB to Tremont CPU. Remove CLDEMOTE, MOVDIRI, MOVDIR64B, and 
WAITPKG to match gcc.

Added: 


Modified: 
clang/lib/Basic/Targets/X86.cpp
clang/test/Preprocessor/predefined-arch-macros.c
llvm/lib/Target/X86/X86.td

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index f4fd5c5835d9..e47cb178792b 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -257,11 +257,8 @@ bool X86TargetInfo::initFeatureMap(
 break;
 
   case CK_Tremont:
-setFeatureEnabledImpl(Features, "cldemote", true);
-setFeatureEnabledImpl(Features, "movdiri", true);
-setFeatureEnabledImpl(Features, "movdir64b", true);
+setFeatureEnabledImpl(Features, "clwb", true);
 setFeatureEnabledImpl(Features, "gfni", true);
-setFeatureEnabledImpl(Features, "waitpkg", true);
 LLVM_FALLTHROUGH;
   case CK_GoldmontPlus:
 setFeatureEnabledImpl(Features, "ptwrite", true);

diff  --git a/clang/test/Preprocessor/predefined-arch-macros.c 
b/clang/test/Preprocessor/predefined-arch-macros.c
index ab36e98beb71..f7de81028327 100644
--- a/clang/test/Preprocessor/predefined-arch-macros.c
+++ b/clang/test/Preprocessor/predefined-arch-macros.c
@@ -1802,15 +1802,16 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck %s -check-prefix=CHECK_TRM_M32
 // CHECK_TRM_M32: #define __AES__ 1
-// CHECK_TRM_M32: #define __CLDEMOTE__ 1
+// CHECK_TRM_M32-NOT: #define __CLDEMOTE__ 1
 // CHECK_TRM_M32: #define __CLFLUSHOPT__ 1
+// CHECK_TRM_M32: #define __CLWB__ 1
 // CHECK_TRM_M32: #define __FSGSBASE__ 1
 // CHECK_TRM_M32: #define __FXSR__ 1
 // CHECK_TRM_M32: #define __GFNI__ 1
 // CHECK_TRM_M32: #define __MMX__ 1
 // CHECK_TRM_M32: #define __MOVBE__ 1
-// CHECK_TRM_M32: #define __MOVDIR64B__ 1
-// CHECK_TRM_M32: #define __MOVDIRI__ 1
+// CHECK_TRM_M32-NOT: #define __MOVDIR64B__ 1
+// CHECK_TRM_M32-NOT: #define __MOVDIRI__ 1
 // CHECK_TRM_M32: #define __PCLMUL__ 1
 // CHECK_TRM_M32: #define __POPCNT__ 1
 // CHECK_TRM_M32: #define __PRFCHW__ 1
@@ -1827,7 +1828,7 @@
 // CHECK_TRM_M32: #define __SSE_MATH__ 1
 // CHECK_TRM_M32: #define __SSE__ 1
 // CHECK_TRM_M32: #define __SSSE3__ 1
-// CHECK_TRM_M32: #define __WAITPKG__ 1
+// CHECK_TRM_M32-NOT: #define __WAITPKG__ 1
 // CHECK_TRM_M32: #define __XSAVEC__ 1
 // CHECK_TRM_M32: #define __XSAVEOPT__ 1
 // CHECK_TRM_M32: #define __XSAVES__ 1
@@ -1843,15 +1844,16 @@
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck %s -check-prefix=CHECK_TRM_M64
 // CHECK_TRM_M64: #define __AES__ 1
-// CHECK_TRM_M64: #define __CLDEMOTE__ 1
+// CHECK_TRM_M64-NOT: #define __CLDEMOTE__ 1
 // CHECK_TRM_M64: #define __CLFLUSHOPT__ 1
+// CHECK_TRM_M64: #define __CLWB__ 1
 // CHECK_TRM_M64: #define __FSGSBASE__ 1
 // CHECK_TRM_M64: #define __FXSR__ 1
 // CHECK_TRM_M64: #define __GFNI__ 1
 // CHECK_TRM_M64: #define __MMX__ 1
 // CHECK_TRM_M64: #define __MOVBE__ 1
-// CHECK_TRM_M64: #define __MOVDIR64B__ 1
-// CHECK_TRM_M64: #define __MOVDIRI__ 1
+// CHECK_TRM_M64-NOT: #define __MOVDIR64B__ 1
+// CHECK_TRM_M64-NOT: #define __MOVDIRI__ 1
 // CHECK_TRM_M64: #define __PCLMUL__ 1
 // CHECK_TRM_M64: #define __POPCNT__ 1
 // CHECK_TRM_M64: #define __PRFCHW__ 1
@@ -1867,7 +1869,7 @@
 // CHECK_TRM_M64: #define __SSE4_2__ 1
 // CHECK_TRM_M64: #define __SSE__ 1
 // CHECK_TRM_M64: #define __SSSE3__ 1
-// CHECK_TRM_M64: #define __WAITPKG__ 1
+// CHECK_TRM_M64-NOT: #define __WAITPKG__ 1
 // CHECK_TRM_M64: #define __XSAVEC__ 1
 // CHECK_TRM_M64: #define __XSAVEOPT__ 1
 // CHECK_TRM_M64: #define __XSAVES__ 1

diff  --git a/llvm/lib/Target/X86/X86.td b/llvm/lib/Target/X86/X86.td
index e897d65174eb..bcbc1ae492b2 100644
--- a/llvm/lib/Target/X86/X86.td
+++ b/llvm/lib/Target/X86/X86.td
@@ -789,15 +789,13 @@ def ProcessorFeatures {
 !listconcat(GLPInheritableFeatures, GLPSpecificFeatures);
 
   // Tremont
-  list TRMAdditionalFeatures = [FeatureCLDEMOTE,
-  FeatureGFNI,
-  FeatureMOVDIRI,
-  FeatureMOVDIR64B,
-  FeatureWAITPKG];
+  list TRMAdditionalFeatures = [FeatureCLWB,
+  FeatureGFNI];
   list TRMSpecificFeatures = [FeatureUseGLMDivSqrtCosts];
+  list TRMInheritableFeatures =
+!listconcat(GLPInheritableFeatures, TRMAdditionalFeatures);
   list TRMFeatures =
-!listconcat(GLPInheritableFeatures, TRMAdditionalFeatures,
-TRMSpecificFeatures);
+

[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

2020-06-02 Thread Daniel via Phabricator via cfe-commits
Daniel599 updated this revision to Diff 268055.
Daniel599 added a comment.

Added `StringMap::operator!=` and also unit tests


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

https://reviews.llvm.org/D80753

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
  llvm/include/llvm/ADT/StringMap.h
  llvm/unittests/ADT/StringMapTest.cpp

Index: llvm/unittests/ADT/StringMapTest.cpp
===
--- llvm/unittests/ADT/StringMapTest.cpp
+++ llvm/unittests/ADT/StringMapTest.cpp
@@ -387,6 +387,70 @@
   ASSERT_EQ(B.count("x"), 0u);
 }
 
+TEST_F(StringMapTest, EqualEmpty) {
+  StringMap A;
+  StringMap B;
+  ASSERT_TRUE(A == B);
+  ASSERT_FALSE(A != B);
+  ASSERT_TRUE(A == A); // self check
+}
+
+TEST_F(StringMapTest, EqualWithValues) {
+  StringMap A;
+  A["A"] = 1;
+  A["B"] = 2;
+  A["C"] = 3;
+  A["D"] = 3;
+
+  StringMap B;
+  B["A"] = 1;
+  B["B"] = 2;
+  B["C"] = 3;
+  B["D"] = 3;
+
+  ASSERT_TRUE(A == B);
+  ASSERT_TRUE(B == A);
+  ASSERT_FALSE(A != B);
+  ASSERT_FALSE(B != A);
+  ASSERT_TRUE(A == A); // self check
+}
+
+TEST_F(StringMapTest, NotEqualMissingKeys) {
+  StringMap A;
+  A["A"] = 1;
+  A["B"] = 2;
+
+  StringMap B;
+  B["A"] = 1;
+  B["B"] = 2;
+  B["C"] = 3;
+  B["D"] = 3;
+
+  ASSERT_FALSE(A == B);
+  ASSERT_FALSE(B == A);
+  ASSERT_TRUE(A != B);
+  ASSERT_TRUE(B != A);
+}
+
+TEST_F(StringMapTest, NotEqualWithDifferentValues) {
+  StringMap A;
+  A["A"] = 1;
+  A["B"] = 2;
+  A["C"] = 100;
+  A["D"] = 3;
+
+  StringMap B;
+  B["A"] = 1;
+  B["B"] = 2;
+  B["C"] = 3;
+  B["D"] = 3;
+
+  ASSERT_FALSE(A == B);
+  ASSERT_FALSE(B == A);
+  ASSERT_TRUE(A != B);
+  ASSERT_TRUE(B != A);
+}
+
 struct Countable {
   int 
   int Number;
Index: llvm/include/llvm/ADT/StringMap.h
===
--- llvm/include/llvm/ADT/StringMap.h
+++ llvm/include/llvm/ADT/StringMap.h
@@ -248,6 +248,26 @@
 return count(MapEntry.getKey());
   }
 
+  /// equal - check whether both of the containers are equal
+  bool operator==(const StringMap ) const {
+if (size() != RHS.size())
+  return false;
+
+for (const auto  : *this) {
+  auto FindInRHS = RHS.find(KeyValue.getKey());
+
+  if (FindInRHS == RHS.end())
+return false;
+
+  if (!(KeyValue.getValue() == FindInRHS->getValue()))
+return false;
+}
+
+return true;
+  }
+
+  bool operator!=(const StringMap ) const { return !(*this == RHS); }
+
   /// insert - Insert the specified key/value pair into the map.  If the key
   /// already exists in the map, return false and ignore the request, otherwise
   /// insert it and return true.
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
@@ -2,8 +2,7 @@
 
 void alwaysThrows() {
   int ex = 42;
-  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err09-cpp]
-  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err61-cpp]
+  // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err09-cpp,cert-err61-cpp]
   throw ex;
 }
 
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init,hicpp-member-init,modernize-use-emplace,hicpp-use-emplace %t
+
+namespace std {
+
+template 
+class vector {
+public:
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template 
+  void emplace_back(Args &&... args){};
+};
+} // namespace std
+
+class Foo {
+public:
+  Foo() : _num1(0)
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [cppcoreguidelines-pro-type-member-init,hicpp-member-init]
+  {
+_num1 = 10;
+  }
+
+  int use_the_members() const {
+return _num1 + _num2;
+  }
+
+private:
+  int _num1;
+  int _num2;
+  // CHECK-FIXES: _num2{};
+};
+
+int should_use_emplace(std::vector ) {
+  v.push_back(Foo());
+  // CHECK-FIXES: v.emplace_back();
+  // CHECK-MESSAGES: warning: use emplace_back instead of push_back [hicpp-use-emplace,modernize-use-emplace]
+}
+
Index: 

[PATCH] D80835: [AIX] Change the default target CPU to power4 for AIX on Power

2020-06-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM; thanks.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:301
+TargetCPUName = "pwr4";
+  else if (!T.isOSDarwin()) {
+if (T.getArch() == llvm::Triple::ppc64)

A separate patch may be in order for changing this to take the non-Darwin path 
without checking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80835



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


[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

2020-06-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast marked an inline comment as done.
hubert.reinterpretcast added a comment.

I believe I have responded to @joerg's comments with rationale to support the 
current direction. I intend to commit this patch within a week or two.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80300



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


[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

2020-06-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: llvm/docs/CodingStandards.rst:1578
+unnecessary and otherwise meaningless code. Braces should be used however
+in cases where it significantly improves readability, such as when the single
+statement is accompanied by a comment that loses its meaning if hoisted above 
the `if`

erichkeane wrote:
> hubert.reinterpretcast wrote:
> > I think "navigability" is also negatively affected by omission of braces. 
> > This seems to be an aspect of readability this is not always considered. It 
> > tends to be easier to consume code in an editor when placing a cursor on a 
> > brace highlights the matching brace. If a reviewer in a web interface 
> > needed to scroll or "draw a line" to where a loop or if/else chain starts 
> > when reaching the end of a block, then the lack of braces is harmful. This 
> > would especially be the case if the code was such that having comments 
> > after the brace would be helpful.
> > 
> > The use of braces to proactively avoid running into the dangling-else 
> > problem should also be permitted or even encouraged.
> > 
> > Replacing the list of cases where braces help readability with a list of 
> > cases where omitting braces are harmful may help. We can then enforce 
> > braces for some classes of harmful brace-omission and permit braces for 
> > other classes.
> > 
> > Examples of "mild" harmful cases can then include mixing of braced and 
> > non-braced blocks in an if/else chain.
> Can you suggest an alternate wording here?  I'm not sure how to capture what 
> you're saying.
Suggested wording:
```
When writing the body of an ``if``, ``else``, or loop statement, omit the 
braces to avoid unnecessary line noise. However, braces should be used in cases 
where the omission of braces harm the readability and maintainability of the 
code.

Readability is harmed when a single statement is accompanied by a comment that 
loses its meaning if hoisted above the ``if`` or loop statement. Similarly, 
braces should be used when single-statement body is complex enough that it 
becomes difficult to see where the block containing the following statement 
began. An ``if``/``else`` chain or a loop is considered a single statement for 
this rule, and this rule applies recursively. This list is not exhaustive, for 
example, readability is also harmed if an ``if``/``else`` chain starts using 
braced bodies partway through and does not continue on with braced bodies.

Maintainability is harmed if the body of an ``if`` ends with a (directly or 
indirectly) nested ``if`` statement with no ``else``. Braces on the outer 
``if`` would help to avoid running into a "dangling else" situation.
```



Comment at: llvm/docs/CodingStandards.rst:1599
+// surprisingly long comment, so it would be unclear without the braces 
whether
+// the following statement is in the scope of the else.
+handleOtherDecl(D);

I believe my comment was misunderstood. I meant that this part was fine (and 
there should be something to distinguish between the keywords and the English 
words). What I meant was that the change implied by another comment of mine 
(https://reviews.llvm.org/D80947?id=267696#inline-744103) did not apply here.


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

https://reviews.llvm.org/D80947



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


[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-06-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D77168#2070049 , @jcai19 wrote:

> In D77168#2069783 , @MaskRay wrote:
>
> > Do we have a mechanism bisecting pragmas? If yes, we can let that tool add 
> > `#pragma clang attribute push([[clang::uninitialized]], apply_to = 
> > variable)`
>
>
> Not that I am aware of unfortunately. The whole point of having this patch is 
> to add the ability to bisect programmatically, along the approach of using 
> pragma as introduced in https://reviews.llvm.org/D78693.


If we have a mechanism bisecting pragmas, this option will not be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77168



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


[PATCH] D80954: [NFC,MTE] Drop unneeded attribute from test

2020-06-02 Thread Vitaly Buka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG85fdec988fc5: [NFC,MTE] Drop unneeded attribute from test 
(authored by vitalybuka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80954

Files:
  clang/test/Driver/memtag_lto.c


Index: clang/test/Driver/memtag_lto.c
===
--- clang/test/Driver/memtag_lto.c
+++ clang/test/Driver/memtag_lto.c
@@ -121,7 +121,7 @@
 
 // SSI-LABEL: @fn
 // SSI-LABEL: allocas uses:
-__attribute__((visibility("default"))) int fn() {
+int fn() {
   // XUNSAFE-DAG: [4]: full-set
   // XSAFE-DAG: [4]: [0,4)
   int x;


Index: clang/test/Driver/memtag_lto.c
===
--- clang/test/Driver/memtag_lto.c
+++ clang/test/Driver/memtag_lto.c
@@ -121,7 +121,7 @@
 
 // SSI-LABEL: @fn
 // SSI-LABEL: allocas uses:
-__attribute__((visibility("default"))) int fn() {
+int fn() {
   // XUNSAFE-DAG: [4]: full-set
   // XSAFE-DAG: [4]: [0,4)
   int x;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-06-02 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment.

In D77168#2069783 , @MaskRay wrote:

> Do we have a mechanism bisecting pragmas? If yes, we can let that tool add 
> `#pragma clang attribute push([[clang::uninitialized]], apply_to = variable)`


Not that I am aware of unfortunately. The whole point of having this patch is 
to add the ability to bisect programmatically, along the approach of using 
pragma as introduced in https://reviews.llvm.org/D78693.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77168



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


[PATCH] D80758: [PowerPC] Add -m[no-]power10-vector clang and llvm option

2020-06-02 Thread Ahsan Saghir via Phabricator via cfe-commits
saghir updated this revision to Diff 268031.
saghir marked an inline comment as done.
saghir removed a reviewer: power-llvm-team.
saghir added a comment.
Herald added a subscriber: kbarton.

Addressed nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80758

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/test/Driver/ppc-dependent-options.cpp
  clang/test/Driver/ppc-features.cpp
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h

Index: llvm/lib/Target/PowerPC/PPCSubtarget.h
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -105,6 +105,7 @@
   bool HasP8Crypto;
   bool HasP9Vector;
   bool HasP9Altivec;
+  bool HasP10Vector;
   bool HasPrefixInstrs;
   bool HasPCRelativeMemops;
   bool HasFCPSGN;
@@ -262,6 +263,7 @@
   bool hasP8Crypto() const { return HasP8Crypto; }
   bool hasP9Vector() const { return HasP9Vector; }
   bool hasP9Altivec() const { return HasP9Altivec; }
+  bool hasP10Vector() const { return HasP10Vector; }
   bool hasPrefixInstrs() const { return HasPrefixInstrs; }
   bool hasPCRelativeMemops() const { return HasPCRelativeMemops; }
   bool hasMFOCRF() const { return HasMFOCRF; }
Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -78,6 +78,7 @@
   HasP8Crypto = false;
   HasP9Vector = false;
   HasP9Altivec = false;
+  HasP10Vector = false;
   HasPrefixInstrs = false;
   HasPCRelativeMemops = false;
   HasFCPSGN = false;
Index: llvm/lib/Target/PowerPC/PPC.td
===
--- llvm/lib/Target/PowerPC/PPC.td
+++ llvm/lib/Target/PowerPC/PPC.td
@@ -216,6 +216,10 @@
 "Enable POWER9 vector instructions",
 [FeatureISA3_0, FeatureP8Vector,
  FeatureP9Altivec]>;
+def FeatureP10Vector  : SubtargetFeature<"power10-vector", "HasP10Vector",
+ "true",
+ "Enable POWER10 vector instructions",
+ [FeatureISA3_1, FeatureP9Vector]>;
 // A separate feature for this even though it is equivalent to P9Vector
 // because this is a feature of the implementation rather than the architecture
 // and may go away with future CPU's.
@@ -337,7 +341,7 @@
   // still exist with the exception of those we know are Power9 specific.
   list P10AdditionalFeatures =
 [DirectivePwr10, FeatureISA3_1, FeaturePrefixInstrs,
- FeaturePCRelativeMemops];
+ FeaturePCRelativeMemops, FeatureP10Vector];
   list P10SpecificFeatures = [];
   list P10InheritableFeatures =
 !listconcat(P9InheritableFeatures, P10AdditionalFeatures);
Index: clang/test/Driver/ppc-features.cpp
===
--- clang/test/Driver/ppc-features.cpp
+++ clang/test/Driver/ppc-features.cpp
@@ -150,6 +150,12 @@
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-power8-vector -mpower8-vector -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-P8VECTOR %s
 // CHECK-P8VECTOR: "-target-feature" "+power8-vector"
 
+// RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-power10-vector -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOP10VECTOR %s
+// CHECK-NOP10VECTOR: "-target-feature" "-power10-vector"
+
+// RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-power10-vector -mpower10-vector -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-P10VECTOR %s
+// CHECK-P10VECTOR: "-target-feature" "+power10-vector"
+
 // RUN: %clang -target powerpc64-unknown-linux-gnu %s -mno-crbits -### -o %t.o 2>&1 | FileCheck -check-prefix=CHECK-NOCRBITS %s
 // CHECK-NOCRBITS: "-target-feature" "-crbits"
 
Index: clang/test/Driver/ppc-dependent-options.cpp
===
--- clang/test/Driver/ppc-dependent-options.cpp
+++ clang/test/Driver/ppc-dependent-options.cpp
@@ -58,6 +58,14 @@
 // RUN: -mcpu=power9 -std=c++11 -mno-vsx -mfloat128 -mpower9-vector %s 2>&1 | \
 // RUN: FileCheck %s -check-prefix=CHECK-NVSX-MULTI
 
+// RUN: not %clang -target powerpc64le-unknown-unknown -fsyntax-only \
+// RUN: -mcpu=power10 -std=c++11 %s 2>&1 | FileCheck %s \
+// RUN: -check-prefix=CHECK-DEFAULT-P10
+
+// RUN: not %clang -target powerpc64le-unknown-unknown -fsyntax-only \
+// RUN: -mcpu=power10 -std=c++11 -mno-vsx -mpower10-vector %s 2>&1 | \
+// RUN: FileCheck %s -check-prefix=CHECK-NVSX-P10V
+
 #ifdef __VSX__
 static_assert(false, "VSX enabled");
 #endif
@@ -70,6 +78,10 @@
 static_assert(false, "P9V 

[clang] 85fdec9 - [NFC,MTE] Drop unneeded attribute from test

2020-06-02 Thread Vitaly Buka via cfe-commits

Author: Vitaly Buka
Date: 2020-06-02T18:28:45-07:00
New Revision: 85fdec988fc55d56988d57fa88e2b870f6e0e8e9

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

LOG: [NFC,MTE] Drop unneeded attribute from test

Summary: Depends on D80847.

Reviewers: eugenis

Reviewed By: eugenis

Subscribers: hiraditya, steven_wu, dexonsmith, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/test/Driver/memtag_lto.c

Removed: 




diff  --git a/clang/test/Driver/memtag_lto.c b/clang/test/Driver/memtag_lto.c
index ab9bbd2f76d2..d9cf327c1bca 100644
--- a/clang/test/Driver/memtag_lto.c
+++ b/clang/test/Driver/memtag_lto.c
@@ -121,7 +121,7 @@ __attribute__((noinline)) void use_local(char *p) { *p = w; 
}
 
 // SSI-LABEL: @fn
 // SSI-LABEL: allocas uses:
-__attribute__((visibility("default"))) int fn() {
+int fn() {
   // XUNSAFE-DAG: [4]: full-set
   // XSAFE-DAG: [4]: [0,4)
   int x;



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


[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

2020-06-02 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2031-2033
+  "%select{|member |static member }0%2"
+  "%select{| of %3| of struct %4| of union %4| of class %4"
+  "| of structured binding %3}1">, InGroup;

I wonder if we could just print out a simple access path here (eg `x.a[3]`). 
`APValue` has support for representing and pretty-printing such things already, 
and you could try passing arbitrary lvalue expressions to the constant 
evaluator to try to form them rather than special-casing a few things in 
`getMemoryLocationImpl`.



Comment at: clang/lib/Sema/SemaChecking.cpp:12782-12783
+  //
+  // FIXME: Is this duplicating some code which already exists somewhere else ?
+  // If not, should this be moved somewhere where it can be reused ?
+  static MemoryLocation

Per the above, I think this can be replaced by calling `EvaluateAsLValue` on 
the expression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57660



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


[PATCH] D79959: [SampleFDO] Add use-sample-profile function attribute

2020-06-02 Thread Wei Mi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7a6c89427c9b: [SampleFDO] Add use-sample-profile function 
attribute. (authored by wmi).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D79959?vs=267957=268030#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79959

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/use-sample-profile-attr.c
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/test/LTO/Resolution/X86/load-sample-prof-icp.ll
  llvm/test/LTO/Resolution/X86/load-sample-prof-lto.ll
  llvm/test/LTO/Resolution/X86/load-sample-prof.ll
  llvm/test/Transforms/Inline/inline-incompat-attrs.ll
  llvm/test/Transforms/Inline/partial-inline-incompat-attrs.ll
  llvm/test/Transforms/SampleProfile/Inputs/profile-symbol-list.ll
  llvm/test/Transforms/SampleProfile/Inputs/use-sample-profile-attr.prof
  llvm/test/Transforms/SampleProfile/branch.ll
  llvm/test/Transforms/SampleProfile/calls.ll
  llvm/test/Transforms/SampleProfile/cold-indirect-call.ll
  llvm/test/Transforms/SampleProfile/cov-zero-samples.ll
  llvm/test/Transforms/SampleProfile/coverage-warning.ll
  llvm/test/Transforms/SampleProfile/discriminator.ll
  llvm/test/Transforms/SampleProfile/early-inline.ll
  llvm/test/Transforms/SampleProfile/entry_counts.ll
  llvm/test/Transforms/SampleProfile/entry_counts_cold.ll
  llvm/test/Transforms/SampleProfile/entry_counts_missing_dbginfo.ll
  llvm/test/Transforms/SampleProfile/fnptr.ll
  llvm/test/Transforms/SampleProfile/function_metadata.ll
  llvm/test/Transforms/SampleProfile/gcc-simple.ll
  llvm/test/Transforms/SampleProfile/indirect-call-gcc.ll
  llvm/test/Transforms/SampleProfile/indirect-call.ll
  llvm/test/Transforms/SampleProfile/inline-callee-update.ll
  llvm/test/Transforms/SampleProfile/inline-cold-callsite-samplepgo.ll
  llvm/test/Transforms/SampleProfile/inline-cold.ll
  llvm/test/Transforms/SampleProfile/inline-combine.ll
  llvm/test/Transforms/SampleProfile/inline-coverage.ll
  llvm/test/Transforms/SampleProfile/inline-mergeprof.ll
  llvm/test/Transforms/SampleProfile/inline-stats.ll
  llvm/test/Transforms/SampleProfile/inline-topdown.ll
  llvm/test/Transforms/SampleProfile/inline.ll
  llvm/test/Transforms/SampleProfile/nolocinfo.ll
  llvm/test/Transforms/SampleProfile/offset.ll
  llvm/test/Transforms/SampleProfile/profile-format-compress.ll
  llvm/test/Transforms/SampleProfile/profile-format.ll
  llvm/test/Transforms/SampleProfile/profile-sample-accurate.ll
  llvm/test/Transforms/SampleProfile/propagate.ll
  llvm/test/Transforms/SampleProfile/remap.ll
  llvm/test/Transforms/SampleProfile/remarks.ll
  llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll
  llvm/test/Transforms/SampleProfile/syntax.ll
  llvm/test/Transforms/SampleProfile/use-sample-profile-attr.ll
  llvm/test/Transforms/SampleProfile/warm-inline-instance.ll

Index: llvm/test/Transforms/SampleProfile/warm-inline-instance.ll
===
--- llvm/test/Transforms/SampleProfile/warm-inline-instance.ll
+++ llvm/test/Transforms/SampleProfile/warm-inline-instance.ll
@@ -4,7 +4,7 @@
 @.str = private unnamed_addr constant [11 x i8] c"sum is %d\0A\00", align 1
 
 ; Function Attrs: nounwind uwtable
-define i32 @foo(i32 %x, i32 %y) !dbg !4 {
+define i32 @foo(i32 %x, i32 %y) #0 !dbg !4 {
 entry:
   %x.addr = alloca i32, align 4
   %y.addr = alloca i32, align 4
@@ -16,7 +16,7 @@
   ret i32 %add, !dbg !11
 }
 
-define i32 @goo(i32 %x, i32 %y) {
+define i32 @goo(i32 %x, i32 %y) #0 {
 entry:
   %x.addr = alloca i32, align 4
   %y.addr = alloca i32, align 4
@@ -29,7 +29,7 @@
 }
 
 ; Function Attrs: uwtable
-define i32 @main() !dbg !7 {
+define i32 @main() #0 !dbg !7 {
 entry:
   %retval = alloca i32, align 4
   %s = alloca i32, align 4
@@ -83,6 +83,8 @@
 
 declare i32 @printf(i8*, ...) #2
 
+attributes #0 = { "use-sample-profile" }
+
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!8, !9}
 !llvm.ident = !{!10}
Index: llvm/test/Transforms/SampleProfile/use-sample-profile-attr.ll
===
--- llvm/test/Transforms/SampleProfile/use-sample-profile-attr.ll
+++ llvm/test/Transforms/SampleProfile/use-sample-profile-attr.ll
@@ -1,26 +1,18 @@
-; RUN: opt < %s -sample-profile -sample-profile-file=%S/Inputs/inline.prof -S | FileCheck %s
-; RUN: opt < %s -passes=sample-profile -sample-profile-file=%S/Inputs/inline.prof -S | FileCheck %s
-
-; Original C++ test case
-;
-; #include 
-;
-; int sum(int x, int y) {
-;   return x + y;
-; }
-;
-; int main() {
-;   int s, i = 0;
-;   while (i++ < 2 * 2)
-; if (i != 100) s = sum(i, s); else s = 30;
-;   printf("sum is %d\n", s);
-;   return 0;
-; }
-;
+; RUN: opt < %s -sample-profile 

[clang] 7a6c894 - [SampleFDO] Add use-sample-profile function attribute.

2020-06-02 Thread Wei Mi via cfe-commits

Author: Wei Mi
Date: 2020-06-02T17:23:17-07:00
New Revision: 7a6c89427c9babc8e4a69e8a2b61bbf4a4b80c56

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

LOG: [SampleFDO] Add use-sample-profile function attribute.

When sampleFDO is enabled, people may expect they can use
-fno-profile-sample-use to opt-out using sample profile for a certain file.
That could be either for debugging purpose or for performance tuning purpose.
However, when thinlto is enabled, if a function in file A compiled with
-fno-profile-sample-use is imported to another file B compiled with
-fprofile-sample-use, the inlined copy of the function in file B may still
get its profile annotated.

The inconsistency may even introduce profile unused warning because if the
target is not compiled with explicit debug information flag, the function
in file A won't have its debug information enabled (debug information will
be enabled implicitly only when -fprofile-sample-use is used). After it is
imported into file B which is compiled with -fprofile-sample-use, profile
annotation for the outline copy of the function will fail because the
function has no debug information, and that will trigger  profile unused
warning.

We add a new attribute use-sample-profile to control whether a function
will use its sample profile no matter for its outline or inline copies.
That will make the behavior of -fno-profile-sample-use consistent.

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

Added: 
clang/test/CodeGen/use-sample-profile-attr.c
llvm/test/Transforms/Inline/inline-incompat-attrs.ll
llvm/test/Transforms/Inline/partial-inline-incompat-attrs.ll
llvm/test/Transforms/SampleProfile/Inputs/use-sample-profile-attr.prof
llvm/test/Transforms/SampleProfile/use-sample-profile-attr.ll

Modified: 
clang/lib/CodeGen/CodeGenFunction.cpp
llvm/include/llvm/IR/Attributes.td
llvm/lib/Transforms/IPO/SampleProfile.cpp
llvm/test/LTO/Resolution/X86/load-sample-prof-icp.ll
llvm/test/LTO/Resolution/X86/load-sample-prof-lto.ll
llvm/test/LTO/Resolution/X86/load-sample-prof.ll
llvm/test/Transforms/SampleProfile/Inputs/profile-symbol-list.ll
llvm/test/Transforms/SampleProfile/branch.ll
llvm/test/Transforms/SampleProfile/calls.ll
llvm/test/Transforms/SampleProfile/cold-indirect-call.ll
llvm/test/Transforms/SampleProfile/cov-zero-samples.ll
llvm/test/Transforms/SampleProfile/coverage-warning.ll
llvm/test/Transforms/SampleProfile/discriminator.ll
llvm/test/Transforms/SampleProfile/early-inline.ll
llvm/test/Transforms/SampleProfile/entry_counts.ll
llvm/test/Transforms/SampleProfile/entry_counts_cold.ll
llvm/test/Transforms/SampleProfile/entry_counts_missing_dbginfo.ll
llvm/test/Transforms/SampleProfile/fnptr.ll
llvm/test/Transforms/SampleProfile/function_metadata.ll
llvm/test/Transforms/SampleProfile/gcc-simple.ll
llvm/test/Transforms/SampleProfile/indirect-call-gcc.ll
llvm/test/Transforms/SampleProfile/indirect-call.ll
llvm/test/Transforms/SampleProfile/inline-callee-update.ll
llvm/test/Transforms/SampleProfile/inline-cold-callsite-samplepgo.ll
llvm/test/Transforms/SampleProfile/inline-cold.ll
llvm/test/Transforms/SampleProfile/inline-combine.ll
llvm/test/Transforms/SampleProfile/inline-coverage.ll
llvm/test/Transforms/SampleProfile/inline-mergeprof.ll
llvm/test/Transforms/SampleProfile/inline-stats.ll
llvm/test/Transforms/SampleProfile/inline-topdown.ll
llvm/test/Transforms/SampleProfile/inline.ll
llvm/test/Transforms/SampleProfile/nolocinfo.ll
llvm/test/Transforms/SampleProfile/offset.ll
llvm/test/Transforms/SampleProfile/profile-format-compress.ll
llvm/test/Transforms/SampleProfile/profile-format.ll
llvm/test/Transforms/SampleProfile/profile-sample-accurate.ll
llvm/test/Transforms/SampleProfile/propagate.ll
llvm/test/Transforms/SampleProfile/remap.ll
llvm/test/Transforms/SampleProfile/remarks.ll
llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll
llvm/test/Transforms/SampleProfile/syntax.ll
llvm/test/Transforms/SampleProfile/warm-inline-instance.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index d6622a435b53..0fa795d696e5 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -791,6 +791,9 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType 
RetTy,
   if (CGM.getCodeGenOpts().ProfileSampleAccurate)
 Fn->addFnAttr("profile-sample-accurate");
 
+  if (!CGM.getCodeGenOpts().SampleProfileFile.empty())
+Fn->addFnAttr("use-sample-profile");
+
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 

diff  --git 

[PATCH] D80876: [clang] Default to windows response files when running on windows

2020-06-02 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment.

Do you know how gcc handles this case when running on mingw?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80876



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


[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

2020-06-02 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D79744#2069786 , @arsenm wrote:

> In D79744#2069774 , @arsenm wrote:
>
> > I think this is converging to adding a new IR attribute that essentially 
> > just provides the pointee type for ABI purposes. I guess my name ideas for 
> > this would be  "indirect", "value", "memoryvalue", "abitype"?
>
>
> My main question for a new attribute is whether it needs to be explicitly 
> read only. I think the answer is no, since you can't ordinarily write to any 
> random pointer argument


I think this is "in practice" correct (assuming you don't see a write to that 
location in which case you can do some crazy stuff).


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

https://reviews.llvm.org/D79744



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


[PATCH] D80771: [MTE] Convert StackSafety into analysis

2020-06-02 Thread Vitaly Buka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG232d348c6eff: [MTE] Convert StackSafety into analysis 
(authored by vitalybuka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80771

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/Driver/memtag.c
  clang/test/Driver/memtag_lto.c
  llvm/include/llvm/Analysis/StackSafetyAnalysis.h
  llvm/lib/Analysis/StackSafetyAnalysis.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Target/AArch64/AArch64.h
  llvm/lib/Target/AArch64/AArch64StackTagging.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll
  llvm/test/CodeGen/AArch64/O3-pipeline.ll
  llvm/test/CodeGen/AArch64/stack-tagging.ll

Index: llvm/test/CodeGen/AArch64/stack-tagging.ll
===
--- llvm/test/CodeGen/AArch64/stack-tagging.ll
+++ llvm/test/CodeGen/AArch64/stack-tagging.ll
@@ -1,4 +1,5 @@
-; RUN: opt < %s -stack-tagging -S -o - | FileCheck %s
+; RUN: opt < %s -stack-tagging -S -o - | FileCheck %s --check-prefixes=CHECK,SSI
+; RUN: opt < %s -stack-tagging -stack-tagging-use-stack-safety=0 -S -o - | FileCheck %s --check-prefixes=CHECK,NOSSI
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64--linux-android"
@@ -8,6 +9,11 @@
 declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture)
 declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture)
 
+define dso_local void @noUse32(i32*) sanitize_memtag {
+entry:
+  ret void
+}
+
 define void @OneVar() sanitize_memtag {
 entry:
   %x = alloca i32, align 4
@@ -33,7 +39,7 @@
   %x1 = alloca i32, align 4
   %x2 = alloca i8, align 4
   %x3 = alloca i32, i32 11, align 4
-  %x4 = alloca i32, align 4, !stack-safe !0
+  %x4 = alloca i32, align 4
   call void @use32(i32* %x1)
   call void @use8(i8* %x2)
   call void @use32(i32* %x3)
@@ -50,9 +56,12 @@
 ; CHECK:  alloca { [11 x i32], [4 x i8] }, align 16
 ; CHECK:  call { [11 x i32], [4 x i8] }* @llvm.aarch64.tagp.{{.*}}({ [11 x i32], [4 x i8] }* {{.*}}, i64 2)
 ; CHECK:  call void @llvm.aarch64.settag(i8* {{.*}}, i64 48)
-; CHECK:  alloca i32, align 4
-; CHECK-NOT: @llvm.aarch64.tagp
-; CHECK-NOT: @llvm.aarch64.settag
+; SSI:alloca i32, align 4
+; NOSSI:  alloca { i32, [12 x i8] }, align 16
+; NOSSI: @llvm.aarch64.tagp.
+; NOSSI: call void @llvm.aarch64.settag(i8* {{.*}}, i64 16)
+; SSI-NOT: @llvm.aarch64.tagp
+; SSI-NOT: @llvm.aarch64.settag
 
 ; CHECK:  call void @use32(
 ; CHECK:  call void @use8(
@@ -61,6 +70,7 @@
 ; CHECK:  call void @llvm.aarch64.settag(i8* {{.*}}, i64 16)
 ; CHECK:  call void @llvm.aarch64.settag(i8* {{.*}}, i64 16)
 ; CHECK:  call void @llvm.aarch64.settag(i8* {{.*}}, i64 48)
+; NOSSI:  call void @llvm.aarch64.settag(i8* {{.*}}, i64 16)
 ; CHECK-NEXT:  ret void
 
 
@@ -161,12 +171,15 @@
 another_bb:
   call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %cz)
   store i32 7, i32* %z
+  call void @noUse32(i32* %z)
   call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %cz)
   call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %cz)
   store i32 7, i32* %z
   call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %cz)
   call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %cxcy)
   store i32 8, i32* %xy
+  call void @noUse32(i32* %x)
+  call void @noUse32(i32* %y)
   call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %cxcy)
   ret void
 }
@@ -179,15 +192,18 @@
 ; CHECK: alloca { i32, [12 x i8] }, align 16
 ; CHECK: call { i32, [12 x i8] }* @llvm.aarch64.tagp
 ; CHECK: call void @llvm.aarch64.settag(
-; CHECK: alloca { i32, [12 x i8] }, align 16
-; CHECK: call { i32, [12 x i8] }* @llvm.aarch64.tagp
-; CHECK: call void @llvm.aarch64.settag(
+; SSI: alloca i32, align 4
+; NOSSI: alloca { i32, [12 x i8] }, align 16
+; NOSSI: call { i32, [12 x i8] }* @llvm.aarch64.tagp
+; NOSSI: call void @llvm.aarch64.settag(
 ; CHECK: store i32
+; CHECK: call void @noUse32(i32*
 ; CHECK: store i32
 ; CHECK: store i32
+; CHECK: call void @noUse32(i32*
 ; CHECK: call void @llvm.aarch64.settag(
 ; CHECK: call void @llvm.aarch64.settag(
-; CHECK: call void @llvm.aarch64.settag(
+; NOSSI: call void @llvm.aarch64.settag(
 ; CHECK: ret void
 
 !0 = !{}
\ No newline at end of file
Index: llvm/test/CodeGen/AArch64/O3-pipeline.ll
===
--- llvm/test/CodeGen/AArch64/O3-pipeline.ll
+++ llvm/test/CodeGen/AArch64/O3-pipeline.ll
@@ -66,6 +66,14 @@
 ; CHECK-NEXT:   Interleaved Load Combine Pass
 ; CHECK-NEXT:   Dominator Tree Construction
 ; CHECK-NEXT:   Interleaved Access Pass
+; CHECK-NEXT: Stack Safety Analysis
+; CHECK-NEXT:   FunctionPass Manager
+; CHECK-NEXT: Dominator Tree Construction
+; CHECK-NEXT: Natural Loop Information
+; CHECK-NEXT: Scalar Evolution Analysis
+; CHECK-NEXT: Stack Safety Local 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

2020-06-02 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen added a comment.

In D79972#2069435 , @ABataev wrote:

> In D79972#2069366 , @cchen wrote:
>
> > In D79972#2069358 , @ABataev wrote:
> >
> > > In D79972#2069322 , @cchen wrote:
> > >
> > > > In D79972#2068976 , @ABataev 
> > > > wrote:
> > > >
> > > > > Still: Did you think about implementing it in the compiler instead of 
> > > > > the runtime?
> > > >
> > > >
> > > > I'm not sure I understand your question, which part of code are you 
> > > > asking?
> > > >  The main work compiler needs to do is to send the {offset, count, 
> > > > stride} struct to runtime.
> > >
> > >
> > > I mean did you think about calling `__tgt_target_data_update` function in 
> > > a loop in the compiler-generated code instead of putting it into the 
> > > runtime?
> >
> >
> > Oh, I would prefer to call `tgt_target_data_update` once in the compiler 
> > and I'm also doing it now.
>
>
> I was not quite correct. What I mean, is to generate the array with the array 
> section as VLA in the compiler, and fill it in the loop generated by the 
> compiler for non-contiguous sections but not in the runtime?
>  Say, we have the code:
>
>   int arr[3][3]
>   ...
>#pragma omp update to(arr[1:2][1:2]
>  
>
>
> In this case, we're going to transfer the next elements:
>
>   000
>   0xx
>   0xx
>
>
> In the compiler-generated code we emit something like this:
>
>   void *bptr[];
>   void *ptr[];
>   int64 sizes[];
>   int64 maptypes[];
>   for (int i = 0; i < ; ++i) {
> bptr[i] = [1+i][1];
> ptr[i] = [1+i][1];
> sizes[i] = ...;'
> maptypes[i] = ...;
>   }
>   call void @__tgt_target_data_update(i64 -1, i32 , bptr, ptr, sizes, 
> maptypes);
>
>
> With this solution, you won't need to modify the runtime and add a new 
> mapping flag.


For my current implementation, we have discussed in the bi-weekly meeting 
several weeks back, and there was a general consensus that it was an acceptable 
approach.

The major advantage of sending a descriptor to runtime can be elaborated in the 
following example:

  #define N 1
  int a[N][2];
  …
  #pragma amp target update to (a[0:N][0:1])

This would require passing through O(N) entries in the tgt_target_data_update 
call, or 1 entries. The current implementation only require a descriptor 
with 2 entries. I think this could be a real concern -
splitting out the transfers in compiler-generated code results in a list 
containing one entry per non-contiguous chunk (easily hitting scaling issues), 
while the descriptor approach is bounded by the number of dimensions.
That seems like a pretty compelling reason to use the descriptor - it’s much 
more space efficient.

Also, the descriptor idea is very similar to how Cray supported Fortran dope 
vectors for years (we send in a pointer to a dope vector rather than a pointer 
to the data, and a flag to indicate it’s a dope vector, and the runtime library 
handles it as a dope vector).
I think the runtime library changes will not be very extensive or difficult at 
all and we’re very willing to implement the runtime for non-contiguous.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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


[PATCH] D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8eda71616fec: [Clang][A32/T32][Linux] -O1 implies 
-fomit-frame-pointer (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828

Files:
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/frame-pointer-elim.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -91,6 +91,12 @@
 
 * Implemented C-language intrinsics  for the CDE instruction set.
 
+* Clang now defaults to ``-fomit-frame-pointer`` when targeting non-Android
+  Linux for arm and thumb when optimizations are enabled. Users that were
+  previously not specifying a value and relying on the implicit compiler
+  default may wish to specify ``-fno-omit-frame-pointer`` to get the old
+  behavior. This improves compatibility with GCC.
+
 Changes to the MIPS Target
 --
 
Index: clang/test/Driver/frame-pointer-elim.c
===
--- clang/test/Driver/frame-pointer-elim.c
+++ clang/test/Driver/frame-pointer-elim.c
@@ -103,5 +103,33 @@
 // RUN: %clang -### -target powerpc64 -S -O1 %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=KEEP-NONE %s
 
+// For AAarch32 (A32, T32) linux targets, default omit frame pointer when
+// optimizations are enabled.
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -marm -mbig-endian -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// RUN: %clang -### -target arm-linux-gnueabihf- -mthumb -mbig-endian -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-NONE %s
+// For Android, keep the framepointers always.
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -marm -mbig-endian -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+// RUN: %clang -### -target armv7a-linux-androideabi- -mthumb -mbig-endian -O1 -S %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEEP-ALL %s
+
 void f0() {}
 void f1() { f0(); }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "Clang.h"
+#include "AMDGPU.h"
 #include "Arch/AArch64.h"
 #include "Arch/ARM.h"
 #include "Arch/Mips.h"
@@ -15,11 +16,10 @@
 #include "Arch/Sparc.h"
 #include "Arch/SystemZ.h"
 #include "Arch/X86.h"
-#include "AMDGPU.h"
 #include "CommonArgs.h"
 #include "Hexagon.h"
-#include "MSP430.h"
 #include "InputInfo.h"
+#include "MSP430.h"
 #include "PS4CPU.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
@@ -35,6 +35,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -557,6 +558,13 @@
   Triple.isOSHurd()) {
 switch (Triple.getArch()) {
 // Don't use a frame pointer on linux if optimizing for certain targets.
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+  if (Triple.isAndroid())
+return true;
+  LLVM_FALLTHROUGH;
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:
Index: clang/lib/Basic/Targets/ARM.cpp
===
--- clang/lib/Basic/Targets/ARM.cpp
+++ clang/lib/Basic/Targets/ARM.cpp
@@ -314,7 +314,7 @@
 
   // Maximum alignment for ARM NEON data types should be 64-bits (AAPCS)
   // 

[PATCH] D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

thanks homies, committed as 
https://github.com/llvm/llvm-project/commit/8eda71616fecd098cbd7d2447859c8ac1315966f


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


RE: [PATCH] D80293: [clangd] Run PreambleThread in async mode behind a flag

2020-06-02 Thread Pieb, Wolfgang via cfe-commits
The test failed 7-8 times in a row on the same build machine and then started 
passing for about 10 runs, and then started failing again for 4 more runs, 
after which we disabled it.
The configuration is nothing special, simply a Release+Asserts build on 
Windows. We tried to reproduce it by hand on the same machine, but weren’t able 
to. The test ran in less than half a second when we tried it (the timeout is 
10s).

Sorry I can’t be of more help. Perhaps we can turn on some tracing output that 
would help diagnosing the problem?

  *   wolfgang

From: Kadir Çetinkaya 
Sent: Tuesday, June 2, 2020 1:20 PM
To: reviews+d80293+public+6cba1e506b8b4...@reviews.llvm.org
Cc: Sam McCall ; Pieb, Wolfgang ; 
Ilya Biryukov ; jav...@graphcore.ai; Fangrui Song 
; Jan Korous ; Aleksei Lorenz 
; jfbast...@apple.com; Utkarsh Saxena ; 
cfe-commits ; Theko Lekena ; 
Nicolas Lesser ; Han Shen ; Haojian 
Wu 
Subject: Re: [PATCH] D80293: [clangd] Run PreambleThread in async mode behind a 
flag

thanks for letting me know. it is annoying that the test is flaky, but I don't 
see that flakiness on any other buildbot I've access to.

Is there any chance that you are running in a custom build configuration that I 
can try to repro the failure? Also what's the falkiness ratio?

On Tue, Jun 2, 2020 at 1:43 AM Wolfgang Pieb via Phabricator 
mailto:revi...@reviews.llvm.org>> wrote:
wolfgangp added a comment.

Hi!

Shortly after this patch went in, we started to see intermittent failures with 
the unittest TUSchedulerTests.ManyUpdates. The build server (Windows) we're 
running it on is quite powerful and our own hand-run tests only take about 400 
ms, so we don't really understand what could be happening. Any help would be 
appreciated.

Here is an excerpt from the log:

Note: Google Test filter = TUSchedulerTests.ManyUpdates

[==] Running 1 test from 1 test case.

[--] Global test environment set-up.

[--] 1 test from TUSchedulerTests

[ RUN  ] TUSchedulerTests.ManyUpdates

C:\j\w\...\clang-tools-extra\clangd\unittests\TUSchedulerTests.cpp(508): error: 
Value of: S.blockUntilIdle(timeoutSeconds(10))

  Actual: false

Expected: true

[  FAILED  ] TUSchedulerTests.ManyUpdates (10576 ms)

[--] 1 test from TUSchedulerTests (10576 ms total)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80293


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


[clang] 232d348 - [MTE] Convert StackSafety into analysis

2020-06-02 Thread Vitaly Buka via cfe-commits

Author: Vitaly Buka
Date: 2020-06-02T16:08:14-07:00
New Revision: 232d348c6eff8493fc016b1ea8a99db0e5620d81

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

LOG: [MTE] Convert StackSafety into analysis

This lets us to remove !stack-safe metadata and
better controll when to perform StackSafety
analysis.

Reviewers: eugenis

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

Tags: #clang, #llvm

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

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp
clang/test/Driver/memtag.c
clang/test/Driver/memtag_lto.c
llvm/include/llvm/Analysis/StackSafetyAnalysis.h
llvm/lib/Analysis/StackSafetyAnalysis.cpp
llvm/lib/Passes/PassRegistry.def
llvm/lib/Target/AArch64/AArch64.h
llvm/lib/Target/AArch64/AArch64StackTagging.cpp
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
llvm/test/CodeGen/AArch64/O3-pipeline.ll
llvm/test/CodeGen/AArch64/stack-tagging.ll

Removed: 
llvm/test/Analysis/StackSafetyAnalysis/ipa-attr.ll



diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 056126dda445..9f28fed40ebc 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -352,11 +352,6 @@ static void addDataFlowSanitizerPass(const 
PassManagerBuilder ,
   PM.add(createDataFlowSanitizerPass(LangOpts.SanitizerBlacklistFiles));
 }
 
-static void addMemTagOptimizationPasses(const PassManagerBuilder ,
-legacy::PassManagerBase ) {
-  PM.add(createStackSafetyGlobalInfoWrapperPass());
-}
-
 static TargetLibraryInfoImpl *createTLII(llvm::Triple ,
  const CodeGenOptions ) {
   TargetLibraryInfoImpl *TLII = new TargetLibraryInfoImpl(TargetTriple);
@@ -727,11 +722,6 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager 
,
addDataFlowSanitizerPass);
   }
 
-  if (LangOpts.Sanitize.has(SanitizerKind::MemTag)) {
-PMBuilder.addExtension(PassManagerBuilder::EP_OptimizerLast,
-   addMemTagOptimizationPasses);
-  }
-
   // Set up the per-function pass manager.
   FPM.add(new TargetLibraryInfoWrapperPass(*TLII));
   if (CodeGenOpts.VerifyModule)
@@ -1384,11 +1374,6 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   /*CompileKernel=*/true, /*Recover=*/true));
 }
 
-if (CodeGenOpts.OptimizationLevel > 0 &&
-LangOpts.Sanitize.has(SanitizerKind::MemTag)) {
-  MPM.addPass(StackSafetyGlobalAnnotatorPass());
-}
-
 if (CodeGenOpts.OptimizationLevel == 0) {
   addCoroutinePassesAtO0(MPM, LangOpts, CodeGenOpts);
   addSanitizersAtO0(MPM, TargetTriple, LangOpts, CodeGenOpts);

diff  --git a/clang/test/Driver/memtag.c b/clang/test/Driver/memtag.c
index bfe453beef56..0de5aaa35fb7 100644
--- a/clang/test/Driver/memtag.c
+++ b/clang/test/Driver/memtag.c
@@ -1,23 +1,27 @@
 // REQUIRES: aarch64-registered-target
 
 // Old pass manager.
-// RUN: %clang -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-NO-SAFETY
-// RUN: %clang -O1 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
-// RUN: %clang -O2 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
-// RUN: %clang -O3 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag %s -S -emit-llvm -o 
- | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -mllvm 
-stack-safety-print=1 %s -S -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-NO-SAFETY
+// RUN: %clang -O1 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -mllvm 
-stack-safety-print=1 %s -S -o - 2>&1 | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -O2 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -mllvm 
-stack-safety-print=1 %s -S -o - 2>&1 | FileCheck %s --check-prefix=CHECK-SAFETY
+// RUN: %clang -O3 -fno-experimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag -mllvm 
-stack-safety-print=1 %s -S -o - 2>&1 | FileCheck %s --check-prefix=CHECK-SAFETY
 
 // New pass manager.
-// RUN: %clang -fexperimental-new-pass-manager -target 
aarch64-unknown-linux -march=armv8+memtag -fsanitize=memtag 

[clang] 39fa431 - [Analyzer][NFC] Fix markup in WebKit checkers documentation

2020-06-02 Thread Jan Korous via cfe-commits

Author: Jan Korous
Date: 2020-06-02T16:04:23-07:00
New Revision: 39fa431c8ccad45de9ec67e8681da923d0cd28c7

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

LOG: [Analyzer][NFC] Fix markup in WebKit checkers documentation

Added: 


Modified: 
clang/docs/analyzer/checkers.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index c977dde8c52f..34018047d57f 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1381,10 +1381,11 @@ WebKit is an open-source web browser engine available 
for macOS, iOS and Linux.
 This section describes checkers that can find issues in WebKit codebase.
 
 Most of the checkers focus on memory management for which WebKit uses custom 
implementation of reference counted smartpointers.
-Checker are formulated in terms related to ref-counting:
-* *Ref-counted type* is either ``Ref`` or ``RefPtr``.
-* *Ref-countable type* is any type that implements ``ref()`` and ``deref()`` 
methods as ``RefPtr<>`` is a template (i. e. relies on duck typing).
-* *Uncounted type* is ref-countable but not ref-counted type.
+
+Checkers are formulated in terms related to ref-counting:
+ - *Ref-counted type* is either ``Ref`` or ``RefPtr``.
+ - *Ref-countable type* is any type that implements ``ref()`` and ``deref()`` 
methods as ``RefPtr<>`` is a template (i. e. relies on duck typing).
+ - *Uncounted type* is ref-countable but not ref-counted type.
 
 .. _webkit-RefCntblBaseVirtualDtor:
 
@@ -1410,6 +1411,7 @@ webkit.WebKitNoUncountedMemberChecker
 Raw pointers and references to uncounted types can't be used as class members. 
Only ref-counted types are allowed.
 
 .. code-block:: cpp
+
  struct RefCntbl {
void ref() {}
void deref() {}



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


[clang] 8eda716 - [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Nick Desaulniers via cfe-commits

Author: Nick Desaulniers
Date: 2020-06-02T15:54:14-07:00
New Revision: 8eda71616fecd098cbd7d2447859c8ac1315966f

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

LOG: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

Summary:
An upgrade of LLVM for CrOS [0] containing [1] triggered a bunch of
errors related to writing to reserved registers for a Linux kernel's
arm64 compat vdso (which is a aarch32 image).

After a discussion on LKML [2], it was determined that
-f{no-}omit-frame-pointer was not being specified. Comparing GCC and
Clang [3], it becomes apparent that GCC defaults to omitting the frame
pointer implicitly when optimizations are enabled, and Clang does not.
ie. setting -O1 (or above) implies -fomit-frame-pointer. Clang was
defaulting to -fno-omit-frame-pointer implicitly unless -fomit-frame-pointer
was set explicitly.

Why this becomes a problem is that the Linux kernel's arm64 compat vdso
contains code that uses r7. r7 is used sometimes for the frame pointer
(for example, when targeting thumb (-mthumb)). See useR7AsFramePointer()
in llvm/llvm-project/llvm/lib/Target/ARM/ARMSubtarget.h. This is mostly
for legacy/compatibility reasons, and the 2019 Q4 revision of the ARM
AAPCS looks to standardize r11 as the frame pointer for aarch32, though
this is not yet implemented in LLVM.

Users that are reliant on the implicit value if unspecified when
optimizations are enabled should explicitly choose -fomit-frame-pointer
(new behavior) or -fno-omit-frame-pointer (old behavior).

[0] https://bugs.chromium.org/p/chromium/issues/detail?id=1084372
[1] https://reviews.llvm.org/D76848
[2] 
https://lore.kernel.org/lkml/20200526173117.155339-1-ndesaulni...@google.com/
[3] https://godbolt.org/z/0oY39t

Reviewers: kristof.beyls, psmith, danalbert, srhines, MaskRay, ostannard, 
efriedma

Reviewed By: psmith, danalbert, srhines, MaskRay, efriedma

Subscribers: efriedma, olista01, MaskRay, vhscampos, cfe-commits, llvm-commits, 
manojgupta, llozano, glider, hctim, eugenis, pcc, peter.smith, srhines

Tags: #clang, #llvm

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

Added: 


Modified: 
clang/lib/Basic/Targets/ARM.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/frame-pointer-elim.c
llvm/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/lib/Basic/Targets/ARM.cpp b/clang/lib/Basic/Targets/ARM.cpp
index f02a9d373609..08b6a9c0b917 100644
--- a/clang/lib/Basic/Targets/ARM.cpp
+++ b/clang/lib/Basic/Targets/ARM.cpp
@@ -314,7 +314,7 @@ ARMTargetInfo::ARMTargetInfo(const llvm::Triple ,
 
   // Maximum alignment for ARM NEON data types should be 64-bits (AAPCS)
   // as well the default alignment
-  if (IsAAPCS && (Triple.getEnvironment() != llvm::Triple::Android))
+  if (IsAAPCS && !Triple.isAndroid())
 DefaultAlignForAttributeAligned = MaxVectorAlign = 64;
 
   // Do force alignment of members that follow zero length bitfields.  If

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 4dfc41c6d3d2..d86fe499d69e 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "Clang.h"
+#include "AMDGPU.h"
 #include "Arch/AArch64.h"
 #include "Arch/ARM.h"
 #include "Arch/Mips.h"
@@ -15,11 +16,10 @@
 #include "Arch/Sparc.h"
 #include "Arch/SystemZ.h"
 #include "Arch/X86.h"
-#include "AMDGPU.h"
 #include "CommonArgs.h"
 #include "Hexagon.h"
-#include "MSP430.h"
 #include "InputInfo.h"
+#include "MSP430.h"
 #include "PS4CPU.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
@@ -35,6 +35,7 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -557,6 +558,13 @@ static bool useFramePointerForTargetByDefault(const 
ArgList ,
   Triple.isOSHurd()) {
 switch (Triple.getArch()) {
 // Don't use a frame pointer on linux if optimizing for certain targets.
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+  if (Triple.isAndroid())
+return true;
+  LLVM_FALLTHROUGH;
 case llvm::Triple::mips64:
 case llvm::Triple::mips64el:
 case llvm::Triple::mips:

diff  --git a/clang/test/Driver/frame-pointer-elim.c 
b/clang/test/Driver/frame-pointer-elim.c
index 47c1c0549212..fd74da7768eb 100644
--- a/clang/test/Driver/frame-pointer-elim.c
+++ b/clang/test/Driver/frame-pointer-elim.c
@@ -103,5 +103,33 @@
 // RUN: %clang -### -target powerpc64 -S -O1 %s 

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-02 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

The change description says something about PWD (which makes me worried about 
build determinism, like amccarth), but I don't see anything about that in the 
diff.

The diff as-is looks fine to me with hans's comment about duplication resolved 
in some form. Please don't add code that stores absolute paths, but at the 
moment this doesn't do that as far as I can tell :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833



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


[PATCH] D80954: [NFC,MTE] Drop unneeded attribute from test

2020-06-02 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis accepted this revision.
eugenis added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80954



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


[PATCH] D80887: [clang-tidy] ignore builtin varargs from pro-type-vararg-check

2020-06-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:21
 
+// FIXME: Add any more builtin variadics that shouldn't trigger this
+static constexpr StringRef AllowedVariadics[] = {

njames93 wrote:
> aaron.ballman wrote:
> > njames93 wrote:
> > > aaron.ballman wrote:
> > > > How would we know which builtins should or should not trigger this? Can 
> > > > we add that information to this comment (or, better yet, just fix the 
> > > > fixme up front)?
> > > Good point, its more a case that I don't know all the variadic builtins 
> > > clang supports. Those ones I included are just the ones you find in the 
> > > gcc documentation. Shall I just remove the Fixme completely, If there is 
> > > another one that's been missed we can address that later
> > Taking a look at Builtins.def shows quite a few more that likely should be 
> > added to the list. I think we should probably have the initial commit 
> > covering anything that's a C standard library function/macro that ends with 
> > "." in its builtin type signature where the C API isn't variadic. For 
> > instance, `__builtin_isfinite`, `__builtin_isinf`, etc. I'm fine if we 
> > don't vet every builtin we support (that's a large amount of work), but we 
> > should be able to cover the most common cases where there's a specification 
> > to compare against.
> ```
> BUILTIN(__builtin_isgreater , "i.", "Fnct")
> BUILTIN(__builtin_isgreaterequal, "i.", "Fnct")
> BUILTIN(__builtin_isless, "i.", "Fnct")
> BUILTIN(__builtin_islessequal   , "i.", "Fnct")
> BUILTIN(__builtin_islessgreater , "i.", "Fnct")
> BUILTIN(__builtin_isunordered   , "i.", "Fnct")
> BUILTIN(__builtin_fpclassify, "ii.", "Fnct")
> BUILTIN(__builtin_isfinite,   "i.", "Fnct")
> BUILTIN(__builtin_isinf,  "i.", "Fnct")
> BUILTIN(__builtin_isinf_sign, "i.", "Fnct")
> BUILTIN(__builtin_isnan,  "i.", "Fnct")
> BUILTIN(__builtin_isnormal,   "i.", "Fnct")
> BUILTIN(__builtin_signbit, "i.", "Fnct")
> BUILTIN(__builtin_constant_p, "i.", "nctu")
> BUILTIN(__builtin_classify_type, "i.", "nctu")
> BUILTIN(__builtin_va_start, "vA.", "nt")
> BUILTIN(__builtin_stdarg_start, "vA.", "nt")
> BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
> BUILTIN(__builtin_fprintf, "iP*cC*.", "Fp:1:")
> BUILTIN(__builtin_printf, "icC*.", "Fp:0:")
> BUILTIN(__builtin_snprintf, "ic*zcC*.", "nFp:2:")
> BUILTIN(__builtin___snprintf_chk, "ic*zizcC*.", "Fp:4:")
> BUILTIN(__builtin___sprintf_chk, "ic*izcC*.", "Fp:3:")
> BUILTIN(__builtin___fprintf_chk, "iP*icC*.", "Fp:2:")
> BUILTIN(__builtin___printf_chk, "iicC*.", "Fp:1:")
> BUILTIN(__builtin_prefetch, "vvC*.", "nc")
> BUILTIN(__builtin_shufflevector, "v."   , "nct")
> BUILTIN(__builtin_convertvector, "v."   , "nct")
> BUILTIN(__builtin_call_with_static_chain, "v.", "nt")
> BUILTIN(__builtin_annotation, "v.", "tn")
> BUILTIN(__builtin_add_overflow, "b.", "nt")
> BUILTIN(__builtin_sub_overflow, "b.", "nt")
> BUILTIN(__builtin_mul_overflow, "b.", "nt")
> BUILTIN(__builtin_preserve_access_index, "v.", "t")
> BUILTIN(__builtin_nontemporal_store, "v.", "t")
> BUILTIN(__builtin_nontemporal_load, "v.", "t")
> BUILTIN(__builtin_os_log_format_buffer_size, "zcC*.", "p:0:nut")
> BUILTIN(__builtin_os_log_format, "v*v*cC*.", "p:0:nt")
> BUILTIN(__builtin_ms_va_start, "vc*&.", "nt")```
> That's all the variadics inside `Builtin.def` that are called `__builtin...`.
> Would you suggest including all those apart from the va, print or format 
> related ones
I would include all of those that are not defined as being variadic in the C, 
C++, or POSIX standards. So things like `__builtin_isgreater` make sense to 
include, but `__builtin_isinf_sign` is a bit less clear to me. So a large part 
of that list seems like stuff we'd want to include, but not all of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80887



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


[PATCH] D81040: Split syntax tree tests into more granular ones

2020-06-02 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
gribozavr2 added reviewers: hlopko, eduucaldas.

Doing so allows us to increase test coverage by removing unnecessary
language restrictions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81040

Files:
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -64,6 +64,11 @@
Language == Lang_CXX17 || Language == Lang_CXX20;
   }
 
+  bool supportsCXXDynamicExceptionSpecification() const {
+return Language == Lang_CXX03 || Language == Lang_CXX11 ||
+   Language == Lang_CXX14;
+  }
+
   bool hasDelayedTemplateParsing() const {
 return Target == "x86_64-pc-win32-msvc";
   }
@@ -851,10 +856,6 @@
 }
 
 TEST_P(SyntaxTreeTest, BinaryOperator) {
-  if (!GetParam().isCXX()) {
-// TODO: Split parts that depend on C++ into a separate test.
-return;
-  }
   expectTreeDumpEqual(
   R"cpp(
 void test(int a) {
@@ -862,15 +863,9 @@
   1 == 2;
   a = 1;
   a <<= 1;
-
-  true || false;
-  true or false;
-
+  1 || 0;
   1 & 2;
-  1 bitand 2;
-
   a ^= 3;
-  a xor_eq 3;
 }
 )cpp",
   R"txt(
@@ -923,42 +918,82 @@
 |-ExpressionStatement
 | |-BinaryOperatorExpression
 | | |-UnknownExpression
-| | | `-true
+| | | `-1
 | | |-||
 | | `-UnknownExpression
-| |   `-false
+| |   `-0
 | `-;
 |-ExpressionStatement
 | |-BinaryOperatorExpression
 | | |-UnknownExpression
-| | | `-true
-| | |-or
+| | | `-1
+| | |-&
 | | `-UnknownExpression
-| |   `-false
+| |   `-2
 | `-;
 |-ExpressionStatement
 | |-BinaryOperatorExpression
 | | |-UnknownExpression
-| | | `-1
-| | |-&
+| | | `-a
+| | |-^=
 | | `-UnknownExpression
-| |   `-2
+| |   `-3
 | `-;
+`-}
+)txt");
+}
+
+TEST_P(SyntaxTreeTest, BinaryOperatorCxx) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  expectTreeDumpEqual(
+  R"cpp(
+void test(int a) {
+  true || false;
+  true or false;
+  1 bitand 2;
+  a xor_eq 3;
+}
+)cpp",
+  R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   `-a
+  |   `-)
+  `-CompoundStatement
+|-{
 |-ExpressionStatement
 | |-BinaryOperatorExpression
 | | |-UnknownExpression
-| | | `-1
-| | |-bitand
+| | | `-true
+| | |-||
 | | `-UnknownExpression
-| |   `-2
+| |   `-false
 | `-;
 |-ExpressionStatement
 | |-BinaryOperatorExpression
 | | |-UnknownExpression
-| | | `-a
-| | |-^=
+| | | `-true
+| | |-or
 | | `-UnknownExpression
-| |   `-3
+| |   `-false
+| `-;
+|-ExpressionStatement
+| |-BinaryOperatorExpression
+| | |-UnknownExpression
+| | | `-1
+| | |-bitand
+| | `-UnknownExpression
+| |   `-2
 | `-;
 |-ExpressionStatement
 | |-BinaryOperatorExpression
@@ -2017,21 +2052,202 @@
   `-;   )txt");
 }
 
-TEST_P(SyntaxTreeTest, ParameterListsInDeclarators) {
+TEST_P(SyntaxTreeTest, ParametersAndQualifiersInFreeFunctions) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  expectTreeDumpEqual(
+  R"cpp(
+int func1();
+int func2a(int a);
+int func2b(int);
+int func3a(int *ap);
+int func3b(int *);
+int func4(int a, float b);
+int func4a(int, float);
+  )cpp",
+  R"txt(
+*: TranslationUnit
+|-SimpleDeclaration
+| |-int
+| |-SimpleDeclarator
+| | |-func1
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   `-)
+| `-;
+|-SimpleDeclaration
+| |-int
+| |-SimpleDeclarator
+| | |-func2a
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   |-SimpleDeclaration
+| |   | |-int
+| |   | `-SimpleDeclarator
+| |   |   `-a
+| |   `-)
+| `-;
+|-SimpleDeclaration
+| |-int
+| |-SimpleDeclarator
+| | |-func2b
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   |-SimpleDeclaration
+| |   | `-int
+| |   `-)
+| `-;
+|-SimpleDeclaration
+| |-int
+| |-SimpleDeclarator
+| | |-func3a
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   |-SimpleDeclaration
+| |   | |-int
+| |   | `-SimpleDeclarator
+| |   |   |-*
+| |   |   `-ap
+| |   `-)
+| `-;
+|-SimpleDeclaration
+| |-int
+| |-SimpleDeclarator
+| | |-func3b
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   |-SimpleDeclaration
+| |   | |-int
+| |   | `-SimpleDeclarator
+| |   |   `-*
+| |   `-)
+| `-;
+|-SimpleDeclaration
+| |-int
+| |-SimpleDeclarator
+| | |-func4
+| | `-ParametersAndQualifiers
+| |   |-(
+| |   |-SimpleDeclaration
+| |   | |-int
+| |   | `-SimpleDeclarator
+| |   |   `-a
+| |   |-,
+| |   |-SimpleDeclaration
+| |   | |-float
+| |   | `-SimpleDeclarator
+| |   |   `-b
+| |   `-)
+| `-;
+`-SimpleDeclaration
+  |-int
+  

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

2020-06-02 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen updated this revision to Diff 268009.
cchen added a comment.

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_update_ast_print.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  clang/test/OpenMP/target_update_messages.cpp
  clang/test/OpenMP/target_update_to_messages.cpp

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}} expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
@@ -140,6 +144,7 @@
   {
 #pragma omp target update to(s7.x)
   }
+
   return 0;
 }
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -36,5 +38,21 @@
   {
 foo();
   }
+
+  double marr[10][5][10];
+#pragma omp target update to(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr [0:] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr [0:2] [2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr [0:2] [2:4] [1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
   return tmain(argc, argv);
 }
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -1059,5 +1059,283 @@
   #pragma omp target update from(([sa][5])f)
 }
 
+#endif
+
+///==///
+// RUN: %clang_cc1 -DCK19 -verify -fopenmp -fopenmp-version=50 -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s 

[PATCH] D81041: Use existing path sep style in clang::FileManager::FixupRelativePath

2020-06-02 Thread Christopher Tetreault via Phabricator via cfe-commits
ctetreau created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
ctetreau added reviewers: efriedma, apazos, zzheng, dexonsmith, rnk.

Modify clang::FileManager::FixupRelativePath to use the existing path
separator style, if one exists. Prior to this change, if a path is set
for the FileSystemOpts workingDir that does not match the native path
separator style, this can result in a path with mixed separators. This
was causing an assertion failure in
clang::test/ClangScanDeps/vfsoverlay.cpp on my machine.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81041

Files:
  clang/lib/Basic/FileManager.cpp


Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -423,6 +423,8 @@
 }
 
 bool FileManager::FixupRelativePath(SmallVectorImpl ) const {
+  using namespace llvm::sys::path;
+
   StringRef pathRef(path.data(), path.size());
 
   if (FileSystemOpts.WorkingDir.empty()
@@ -430,7 +432,19 @@
 return false;
 
   SmallString<128> NewPath(FileSystemOpts.WorkingDir);
-  llvm::sys::path::append(NewPath, pathRef);
+  Style sepStyle = Style::native;
+
+  for (char c : NewPath) {
+if (is_separator(c, Style::windows)) {
+  sepStyle = Style::windows;
+  break;
+} else if (is_separator(c, Style::posix)) {
+  sepStyle = Style::posix;
+  break;
+}
+  }
+
+  append(NewPath, sepStyle, pathRef);
   path = NewPath;
   return true;
 }


Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -423,6 +423,8 @@
 }
 
 bool FileManager::FixupRelativePath(SmallVectorImpl ) const {
+  using namespace llvm::sys::path;
+
   StringRef pathRef(path.data(), path.size());
 
   if (FileSystemOpts.WorkingDir.empty()
@@ -430,7 +432,19 @@
 return false;
 
   SmallString<128> NewPath(FileSystemOpts.WorkingDir);
-  llvm::sys::path::append(NewPath, pathRef);
+  Style sepStyle = Style::native;
+
+  for (char c : NewPath) {
+if (is_separator(c, Style::windows)) {
+  sepStyle = Style::windows;
+  break;
+} else if (is_separator(c, Style::posix)) {
+  sepStyle = Style::posix;
+  break;
+}
+  }
+
+  append(NewPath, sepStyle, pathRef);
   path = NewPath;
   return true;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D80877: [clang-tidy] Added MacroDefiniton docs for readability-identifier-naming

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

LGTM! We can add the `ObjCIVar` docs in a future patch if someone has the 
expertise there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80877



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


[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

2020-06-02 Thread Ten Tzen via Phabricator via cfe-commits
tentzen added a comment.

Is there any feedback? thanks,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344



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


[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

2020-06-02 Thread Ten Tzen via Phabricator via cfe-commits
tentzen added a comment.

Hi, does this look good? or is there any other concern?
thanks,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77982



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


[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

2020-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 268002.
nickdesaulniers added a comment.

- handle non top level decls, add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80242

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/CommandGuide/clang.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-unused-types.c
  clang/test/CodeGen/debug-info-unused-types.cpp

Index: clang/test/CodeGen/debug-info-unused-types.cpp
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-unused-types.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang++ -fno-eliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang++ -feliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang++ -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang++ -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+using foo = int;
+class bar {};
+enum class baz { BAZ };
+
+void quux() {
+  using x = int;
+  class y {};
+  enum class z { Z };
+}
+
+// CHECK: !3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz", file: !4, line: 7, baseType: !5, size: 32, flags: DIFlagEnumClass, elements: !6, identifier: "_ZTS3baz")
+// CHECK: !7 = !DIEnumerator(name: "BAZ", value: 0)
+// CHECK: !8 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z", scope: !9, file: !4, line: 12, baseType: !5, size: 32, flags: DIFlagEnumClass, elements: !13)
+// CHECK: !14 = !DIEnumerator(name: "Z", value: 0)
+// CHECK: !16 = !DIDerivedType(tag: DW_TAG_typedef, name: "foo", file: !4, line: 5, baseType: !5)
+// CHECK: !17 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "bar", file: !4, line: 6, size: 8, flags: DIFlagTypePassByValue, elements: !12, identifier: "_ZTS3bar")
+// CHECK: !18 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "y", scope: !9, file: !4, line: 11, size: 8, flags: DIFlagTypePassByValue, elements: !12)
+
+// NODBG-NOT: !3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz", file: !4, line: 7, baseType: !5, size: 32, flags: DIFlagEnumClass, elements: !6, identifier: "_ZTS3baz")
+// NODBG-NOT: !7 = !DIEnumerator(name: "BAZ", value: 0)
+// NODBG-NOT: !8 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z", scope: !9, file: !4, line: 12, baseType: !5, size: 32, flags: DIFlagEnumClass, elements: !13)
+// NODBG-NOT: !14 = !DIEnumerator(name: "Z", value: 0)
+// NODBG-NOT: !16 = !DIDerivedType(tag: DW_TAG_typedef, name: "foo", file: !4, line: 5, baseType: !5)
+// NODBG-NOT: !17 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "bar", file: !4, line: 6, size: 8, flags: DIFlagTypePassByValue, elements: !12, identifier: "_ZTS3bar")
+// NODBG-NOT: !18 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "y", scope: !9, file: !4, line: 11, size: 8, flags: DIFlagTypePassByValue, elements: !12)
Index: clang/test/CodeGen/debug-info-unused-types.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-unused-types.c
@@ -0,0 +1,44 @@
+// RUN: %clang -fno-eliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang -feliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+typedef int my_int;
+struct foo {};
+enum bar { BAR };
+union baz {};
+
+void quux(void) {
+  typedef int x;
+  struct y {};
+  enum z { Z };
+  union w {};
+}
+
+// Check that debug info is emitted for the typedef, struct, enum, and union
+// when -fno-eliminate-unused-debug-types and -g are set.
+
+// CHECK: !3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "bar", file: !4, line: 7, baseType: !5, size: 32, elements: !6)
+// CHECK: !7 = !DIEnumerator(name: "BAR", value: 0, isUnsigned: true)
+// CHECK: !8 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z", scope: !9, file: !4, line: 13, baseType: !5, size: 32, elements: !13)
+// CHECK: !14 = !DIEnumerator(name: "Z", value: 0, isUnsigned: true)
+// CHECK: !16 = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int", file: !4, line: 5, baseType: !17)
+// CHECK: !17 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// CHECK: !18 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo", file: !4, line: 6, elements: !12)
+// CHECK: !19 = distinct !DICompositeType(tag: DW_TAG_union_type, name: "baz", file: !4, line: 8, elements: !12)
+// CHECK: !20 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "y", 

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

2020-06-02 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

In D79744#2069774 , @arsenm wrote:

> I think this is converging to adding a new IR attribute that essentially just 
> provides the pointee type for ABI purposes. I guess my name ideas for this 
> would be  "indirect", "value", "memoryvalue", "abitype"?


My main question for a new attribute is whether it needs to be explicitly read 
only. I think the answer is no, since you can't ordinarily write to any random 
pointer argument


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

https://reviews.llvm.org/D79744



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


[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-06-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Do we have a mechanism bisecting pragmas? If yes, we can let that tool add 
`#pragma clang attribute push([[clang::uninitialized]], apply_to = variable)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77168



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


[PATCH] D74746: [clang][doxygen] Fix -Wdocumentation warning for tag typedefs

2020-06-02 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment.

I reported this issue back in November, as 
https://bugs.llvm.org/show_bug.cgi?id=44143.  This change fixes the bug -- I'll 
mark it as such.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74746



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


[PATCH] D79721: [Clang][AArch64] Capturing proper pointer alignment for Neon vld1 intrinsicts

2020-06-02 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79721



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


[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

2020-06-02 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert marked an inline comment as done.
jdoerfert added a comment.

I tried to determine why we don't emit such calls for c++11 and stdc++ but I 
was not successful :( Tracking back from the emission lead to the generic 
expression codegen without any (obvious) check of the runtime library or std 
versions.




Comment at: clang/lib/Headers/__clang_cuda_complex_builtins.h:136-137
+  __d = _COPYSIGNf(_ISINFf(__d) ? 1 : 0, __d);
+  if (_ISNANf(__a))
+__a = _COPYSIGNf(0, __a);
+  if (_ISNANf(__b))

arsenm wrote:
> Why does this try to preserve the sign of a nan? They are meaningless
Idk [I only work here... ;)]

I guess the algorithm was once copied from libc++, unclear if the one in there 
is still the same, we could check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80897



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


[PATCH] D81037: [OPENMP]Fix PR46170: partial mapping for array sections of data members.

2020-06-02 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for the quick fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81037



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


[PATCH] D79052: [clang codegen] Fix alignment of "Address" for incomplete array pointer.

2020-06-02 Thread Eli Friedman via Phabricator via cfe-commits
efriedma updated this revision to Diff 267999.
efriedma added a comment.

This should work correctly, now, I think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79052

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/alignment.cpp

Index: clang/test/CodeGenCXX/alignment.cpp
===
--- clang/test/CodeGenCXX/alignment.cpp
+++ clang/test/CodeGenCXX/alignment.cpp
@@ -308,4 +308,20 @@
 D d;
 AlignedArray result = d.bArray;
   }
+
+  // CHECK-LABEL: @_ZN5test11hEPA_NS_1BE
+  void h(B (*b)[]) {
+// CHECK: [[RESULT:%.*]] = alloca [[ARRAY]], align 64
+// CHECK: [[B_P:%.*]] = load [0 x [[B]]]*, [0 x [[B]]]**
+// CHECK: [[ELEMENT_P:%.*]] = getelementptr inbounds [0 x [[B]]], [0 x [[B]]]* [[B_P]], i64 0
+// CHECK: [[ARRAY_P:%.*]] = getelementptr inbounds [[B]], [[B]]* [[ELEMENT_P]], i32 0, i32 2
+// CHECK: [[T0:%.*]] = bitcast [[ARRAY]]* [[RESULT]] to i8*
+// CHECK: [[T1:%.*]] = bitcast [[ARRAY]]* [[ARRAY_P]] to i8*
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 64 [[T0]], i8* align 16 [[T1]], i64 16, i1 false)
+AlignedArray result = (*b)->bArray;
+  }
 }
+
+// CHECK-LABEL: @_Z22incomplete_array_derefPA_i
+// CHECK: load i32, i32* {{%.*}}, align 4
+int incomplete_array_deref(int (*p)[]) { return (*p)[2]; }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5982,6 +5982,9 @@
   if (TBAAInfo)
 *TBAAInfo = getTBAAAccessInfo(T);
 
+  // FIXME: This duplicates logic in ASTContext::getTypeAlignIfKnown. But
+  // that doesn't return the information we need to compute BaseInfo.
+
   // Honor alignment typedef attributes even on incomplete types.
   // We also honor them straight for C++ class types, even as pointees;
   // there's an expressivity gap here.
@@ -5993,32 +5996,46 @@
 }
   }
 
+  bool AlignForArray = T->isArrayType();
+
+  // Analyze the base element type, so we don't get confused by incomplete
+  // array types.
+  T = getContext().getBaseElementType(T);
+
+  if (T->isIncompleteType()) {
+// We could try to replicate the logic from
+// ASTContext::getTypeAlignIfKnown, but nothing uses the alignment if the
+// type is incomplete, so it's impossible to test. We could try to reuse
+// getTypeAlignIfKnown, but that doesn't return the information we need
+// to set BaseInfo.  So just ignore the possibility that the alignment is
+// greater than one.
+if (BaseInfo)
+  *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
+return CharUnits::One();
+  }
+
   if (BaseInfo)
 *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
 
   CharUnits Alignment;
-  if (T->isIncompleteType()) {
-Alignment = CharUnits::One(); // Shouldn't be used, but pessimistic is best.
+  // For C++ class pointees, we don't know whether we're pointing at a
+  // base or a complete object, so we generally need to use the
+  // non-virtual alignment.
+  const CXXRecordDecl *RD;
+  if (forPointeeType && !AlignForArray && (RD = T->getAsCXXRecordDecl())) {
+Alignment = getClassPointerAlignment(RD);
   } else {
-// For C++ class pointees, we don't know whether we're pointing at a
-// base or a complete object, so we generally need to use the
-// non-virtual alignment.
-const CXXRecordDecl *RD;
-if (forPointeeType && (RD = T->getAsCXXRecordDecl())) {
-  Alignment = getClassPointerAlignment(RD);
-} else {
-  Alignment = getContext().getTypeAlignInChars(T);
-  if (T.getQualifiers().hasUnaligned())
-Alignment = CharUnits::One();
-}
+Alignment = getContext().getTypeAlignInChars(T);
+if (T.getQualifiers().hasUnaligned())
+  Alignment = CharUnits::One();
+  }
 
-// Cap to the global maximum type alignment unless the alignment
-// was somehow explicit on the type.
-if (unsigned MaxAlign = getLangOpts().MaxTypeAlign) {
-  if (Alignment.getQuantity() > MaxAlign &&
-  !getContext().isAlignmentRequired(T))
-Alignment = CharUnits::fromQuantity(MaxAlign);
-}
+  // Cap to the global maximum type alignment unless the alignment
+  // was somehow explicit on the type.
+  if (unsigned MaxAlign = getLangOpts().MaxTypeAlign) {
+if (Alignment.getQuantity() > MaxAlign &&
+!getContext().isAlignmentRequired(T))
+  Alignment = CharUnits::fromQuantity(MaxAlign);
   }
   return Alignment;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

2020-06-02 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

In D79744#2069620 , @rjmccall wrote:

> In D79744#2069324 , @arsenm wrote:
>
> > In D79744#2050498 , @rjmccall 
> > wrote:
> >
> > > > For the purpose here, only the callee exists. This is essentially a 
> > > > freestanding function, the entry point to the program.
> > >
> > > I'm definitely not going to let you add a new "generic" argument-passing 
> > > convention and then only implement exactly the parts you need; that's 
> > > just adding technical debt.
> >
> >
> > I'm not sure what you mean here. I don't really want or need a totally new 
> > generic argument passing convention. Not every IR feature is expected to be 
> > implemented by every backend. For instance, inalloca/preallocated exist 
> > solely to implement the x86 windows ABI and no other target tries to handle 
> > them. This is just another flavor of the same fundamental concept of a 
> > parameter attribute needed for a target specific calling convention.
>
>
> I mean the Clang code for supporting this new convention, not the IR support. 
>  Of course LLVM has target-specific attributes.


I think this is converging to adding a new IR attribute that essentially just 
provides the pointee type for ABI purposes. I guess my name ideas for this 
would be  "indirect", "value", "memoryvalue", "abitype"?

I forget that frontends exist sometimes, so I'm not sure I understand your 
clang side objection.


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

https://reviews.llvm.org/D79744



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


[PATCH] D81037: [OPENMP]Fix PR46170: partial mapping for array sections of data members.

2020-06-02 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev created this revision.
ABataev added a reviewer: jdoerfert.
Herald added subscribers: sstefan1, guansong, yaxunl.
Herald added a project: clang.
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for the quick fix!


If the data member is mapped as an array section, need to emit the
pointer to the last element of this array section and use this pointer
as the highest element in partial struct data.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81037

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

Index: clang/test/OpenMP/target_map_member_expr_array_section_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_map_member_expr_array_section_codegen.cpp
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+// 32 = 0x20 = OMP_MAP_TARGET_PARAM
+// 281474976710656 = 0x1 = OMP_MAP_MEMBER_OF of 1-st element
+// CHECK: [[MAP_ENTER:@.+]] = private unnamed_addr constant [2 x i64] [i64 32, i64 281474976710656]
+// 281474976710664 = 0x10008 = OMP_MAP_MEMBER_OF of 1-st element | OMP_MAP_DELETE
+// CHECK: [[MAP_EXIT:@.+]] = private unnamed_addr constant [2 x i64] [i64 32, i64 281474976710664]
+template 
+struct S {
+  constexpr static int size = 6;
+  T data[size];
+};
+
+template 
+struct maptest {
+  S s;
+  maptest() {
+// CHECK: [[BPTRS:%.+]] = alloca [2 x i8*],
+// CHECK: [[PTRS:%.+]] = alloca [2 x i8*],
+// CHECK: [[SIZES:%.+]] = alloca [2 x i64],
+// CHECK: getelementptr inbounds
+// CHECK: [[S_ADDR:%.+]] = getelementptr inbounds %struct.maptest, %struct.maptest* [[THIS:%.+]], i32 0, i32 0
+// CHECK: [[S_DATA_ADDR:%.+]] = getelementptr inbounds %struct.S, %struct.S* [[S_ADDR]], i32 0, i32 0
+// CHECK: [[S_DATA_0_ADDR:%.+]] = getelementptr inbounds [6 x float], [6 x float]* [[S_DATA_ADDR]], i64 0, i64 0
+
+// SZ = >s.data[6]->s.data[0]
+// CHECK: [[S_ADDR:%.+]] = getelementptr inbounds %struct.maptest, %struct.maptest* [[THIS]], i32 0, i32 0
+// CHECK: [[S_DATA_ADDR:%.+]] = getelementptr inbounds %struct.S, %struct.S* [[S_ADDR]], i32 0, i32 0
+// CHECK: [[S_DATA_5_ADDR:%.+]] = getelementptr inbounds [6 x float], [6 x float]* [[S_DATA_ADDR]], i64 0, i64 5
+// CHECK: [[S_DATA_6_ADDR:%.+]] = getelementptr float, float* [[S_DATA_5_ADDR]], i32 1
+// CHECK: [[BEG:%.+]] = bitcast float* [[S_DATA_0_ADDR]] to i8*
+// CHECK: [[END:%.+]] = bitcast float* [[S_DATA_6_ADDR]] to i8*
+// CHECK: [[END_BC:%.+]] = ptrtoint i8* [[END]] to i64
+// CHECK: [[BEG_BC:%.+]] = ptrtoint i8* [[BEG]] to i64
+// CHECK: [[DIFF:%.+]] = sub i64 [[END_BC]], [[BEG_BC]]
+// CHECK: [[SZ:%.+]] = sdiv exact i64 [[DIFF]], ptrtoint (i8* getelementptr (i8, i8* null, i32 1) to i64)
+
+// Fill mapping arrays
+// CHECK: [[BPTR0:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[BPTRS]], i32 0, i32 0
+// CHECK: [[BPTR0_THIS:%.+]] = bitcast i8** [[BPTR0]] to %struct.maptest**
+// CHECK: store %struct.maptest* [[THIS]], %struct.maptest** [[BPTR0_THIS]],
+// CHECK: [[PTR0:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[PTRS]], i32 0, i32 0
+// CHECK: [[PTR0_DATA:%.+]] = bitcast i8** [[PTR0]] to float**
+// CHECK: store float* [[S_DATA_0_ADDR]], float** [[PTR0_DATA]],
+// CHECK: [[SIZE0:%.+]] = getelementptr inbounds [2 x i64], [2 x i64]* [[SIZES]], i32 0, i32 0
+// CHECK: store i64 [[SZ]], i64* [[SIZE0]],
+// CHECK: [[BPTR1:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[BPTRS]], i32 0, i32 1
+// CHECK: [[BPTR1_THIS:%.+]] = bitcast i8** [[BPTR1]] to %struct.maptest**
+// CHECK: store %struct.maptest* [[THIS]], %struct.maptest** [[BPTR1_THIS]],
+// CHECK: [[PTR1:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[PTRS]], i32 0, i32 1
+// CHECK: [[PTR1_DATA:%.+]] = bitcast i8** [[PTR1]] to float**
+// CHECK: store float* [[S_DATA_0_ADDR]], float** [[PTR1_DATA]],
+// CHECK: [[SIZE1:%.+]] = getelementptr inbounds [2 x i64], [2 x i64]* [[SIZES]], i32 0, i32 1
+// CHECK: store i64 24, i64* [[SIZE1]],
+// CHECK: [[BPTR:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[BPTRS]], i32 0, i32 0
+// CHECK: [[PTR:%.+]] = getelementptr inbounds [2 x i8*], [2 x i8*]* [[PTRS]], i32 0, i32 0
+// CHECK: [[SIZE:%.+]] = getelementptr inbounds [2 x 

[PATCH] D80933: [clang-format] [PR46157] Wrong spacing of negative literals with use of operator

2020-06-02 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:981
 consumeToken();
+if (CurrentToken->is(tok::comma) &&
+CurrentToken->Previous->isNot(tok::kw_operator))

Shouldn't you check `CurrentToken` for not being null after a call to 
`consumeToken`?? It may happen if `operator` is at the end of input (even if 
it's a degenerated case).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80933



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


[PATCH] D80950: [clang-format] [PR44542,38872] String << String always get a forced newline.

2020-06-02 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

The change seems to me technically sound, but I'm not sure of the scope of its 
effects. There might be users that rely on this behaviour. On the other hand, 
adding an option to keep the old behaviour doesn't seem appropriate, and 
personally I consider the old behaviour as a bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80950



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


[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

2020-06-02 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

In D80961#2068902 , @aaron.ballman 
wrote:

> My experience with clang-tidy has been that template instantiations are a 
> double-edged sword. The instantiation is the only place at which you have 
> sufficient information to perform many kinds of analyses, but it's also often 
> not plausible to modify the templated code because it's not clear from the 
> instantiation context how changes may impact other instantiations. The result 
> is that most of the clang-tidy checks wind up punting on template 
> instantiations similar to what they do for macros. Based on that experience, 
> I think it makes sense to continue in the proposed direction.


Great. I also think that given that the default has just been changed to 
`TK_IgnoreUnlessSpelledInSource`, we should change this soon. The facts would 
still be the same if we do this at some future date, but it would be needlessly 
more disruptive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



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


[PATCH] D79052: [clang codegen] Fix alignment of "Address" for incomplete array pointer.

2020-06-02 Thread Eli Friedman via Phabricator via cfe-commits
efriedma updated this revision to Diff 267994.
efriedma added a comment.

"Address" the review comments.  Not really happy with this, but not sure what 
else to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79052

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/alignment.cpp


Index: clang/test/CodeGenCXX/alignment.cpp
===
--- clang/test/CodeGenCXX/alignment.cpp
+++ clang/test/CodeGenCXX/alignment.cpp
@@ -309,3 +309,7 @@
 AlignedArray result = d.bArray;
   }
 }
+
+// CHECK-LABEL: @_Z22incomplete_array_derefPA_i
+// CHECK: load i32, i32* {{%.*}}, align 4
+int incomplete_array_deref(int (*p)[]) { return (*p)[2]; }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5993,32 +5993,58 @@
 }
   }
 
+  bool AlignForArray = T->isArrayType();
+
+  // Analyze the base element type, so we don't get confused by incomplete
+  // array types.
+  T = getBaseElementType(T);
+
+  if (T->isIncompleteType()) {
+// FIXME: This duplicates logic in ASTContext::getTypeAlignIfKnown. But we
+// that doesn't return whether we got the alignment from an attribute.
+if (const auto *TT = T->getAs()) {
+  if (unsigned Align = TT->getDecl()->getMaxAlignment()) {
+if (BaseInfo)
+  *BaseInfo = LValueBaseInfo(AlignmentSource::AttributedType);
+return getContext().toCharUnitsFromBits(Align);
+  }
+}
+
+// Otherwise, see if the declaration of the type had an attribute.
+if (const auto *TT = T->getAs()) {
+  if (BaseInfo)
+*BaseInfo = LValueBaseInfo(AlignmentSource::AttributedType);
+  return 
getContext().toCharUnitsFromBits(TT->getDecl()->getMaxAlignment());
+}
+
+// The alignment is completely unknown; just return one.
+if (BaseInfo)
+  *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
+return CharUnits::One();
+  }
+
   if (BaseInfo)
 *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
 
   CharUnits Alignment;
-  if (T->isIncompleteType()) {
-Alignment = CharUnits::One(); // Shouldn't be used, but pessimistic is 
best.
+  // For C++ class pointees, we don't know whether we're pointing at a
+  // base or a complete object, so we generally need to use the
+  // non-virtual alignment.
+  const CXXRecordDecl *RD;
+  if (forPointeeType && !AlignForArray && (RD = T->getAsCXXRecordDecl())) {
+Alignment = getClassPointerAlignment(RD);
   } else {
-// For C++ class pointees, we don't know whether we're pointing at a
-// base or a complete object, so we generally need to use the
-// non-virtual alignment.
-const CXXRecordDecl *RD;
-if (forPointeeType && (RD = T->getAsCXXRecordDecl())) {
-  Alignment = getClassPointerAlignment(RD);
-} else {
-  Alignment = getContext().getTypeAlignInChars(T);
-  if (T.getQualifiers().hasUnaligned())
-Alignment = CharUnits::One();
-}
+Alignment = getContext().getTypeAlignInChars(T);
+if (T.getQualifiers().hasUnaligned())
+  Alignment = CharUnits::One();
+  }
 
-// Cap to the global maximum type alignment unless the alignment
-// was somehow explicit on the type.
-if (unsigned MaxAlign = getLangOpts().MaxTypeAlign) {
-  if (Alignment.getQuantity() > MaxAlign &&
-  !getContext().isAlignmentRequired(T))
-Alignment = CharUnits::fromQuantity(MaxAlign);
-}
+  // Cap to the global maximum type alignment unless the alignment
+  // was somehow explicit on the type.
+  if (unsigned MaxAlign = getLangOpts().MaxTypeAlign) {
+if (Alignment.getQuantity() > MaxAlign &&
+!getContext().isAlignmentRequired(T))
+  Alignment = CharUnits::fromQuantity(MaxAlign);
   }
   return Alignment;
 }


Index: clang/test/CodeGenCXX/alignment.cpp
===
--- clang/test/CodeGenCXX/alignment.cpp
+++ clang/test/CodeGenCXX/alignment.cpp
@@ -309,3 +309,7 @@
 AlignedArray result = d.bArray;
   }
 }
+
+// CHECK-LABEL: @_Z22incomplete_array_derefPA_i
+// CHECK: load i32, i32* {{%.*}}, align 4
+int incomplete_array_deref(int (*p)[]) { return (*p)[2]; }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5993,32 +5993,58 @@
 }
   }
 
+  bool AlignForArray = T->isArrayType();
+
+  // Analyze the base element type, so we don't get confused by incomplete
+  // array types.
+  T = getBaseElementType(T);
+
+  if (T->isIncompleteType()) {
+// FIXME: This duplicates logic in ASTContext::getTypeAlignIfKnown. But we
+// that doesn't return whether we got the alignment from an attribute.
+if (const auto *TT = 

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-06-02 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment.

@jfb Does the current implementation look good? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77168



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


[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

2020-06-02 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

In D80961#2067638 , @gribozavr2 wrote:

> If IgnoreUnlessSpelledInSource is indeed for novice users (and not to be 
> strictly interpreted as "it does what it says") we should think about whether 
> it more useful to ignore instantiations or to match in instantiations.


Tools should generally follow the Hippocratic: First do no harm.

As I demonstrated, simple tools currently accidentally make unintended and 
incorrect changes to code. That is wrong.

Do you have a standing objection to this being changed?

I can't really work on this if I know or assume that you're going to prevent it 
going in (ever or for a significant time which results in this being harder 
than it should be).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



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


[PATCH] D79052: [clang codegen] Fix alignment of "Address" for incomplete array pointer.

2020-06-02 Thread Eli Friedman via Phabricator via cfe-commits
efriedma planned changes to this revision.
efriedma added a comment.

Hang on, I submitted this too early.  Need to look a bit more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79052



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


[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

2020-06-02 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

In D80961#2068865 , @ymandel wrote:

> Thank you for bringing up this issue. I think it highlights an underlying 
> problem -- editing templates is quite difficult -- that neither setting will 
> address, as Dmitri expanded on above. Given the parallel to macros, I'd say 
> your change is better than the status quo. Most clang tidies and other 
> rewriting tools that I've encountered simply skip code in macro expansions, 
> rather than reason about how to update the macro definition or whatnot. So, 
> by that reasoning, we should skip template instantations.


Yes, it's generally true for tools that given the choice of

1. possibly making a change which is definitely incorrect (as is currently done 
by default and as demonstrated in the Transformer test case)
2. not making a particular change meaning the overall change is incomplete (as 
in Dmitris response)

number (2) would always be way to go. I don't think Dmitris objection makes any 
sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



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


[PATCH] D80752: [AArch64]: BFloat MatMul Intrinsics

2020-06-02 Thread Mikhail Maltsev via Phabricator via cfe-commits
miyuki added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:16160
   }
   case WebAssembly::BI__builtin_wasm_narrow_s_i8x16_i16x8:
   case WebAssembly::BI__builtin_wasm_narrow_u_i8x16_i16x8:

miyuki wrote:
> This chunk does not belong to the patch
Oops, ignore the previous comment, please.


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

https://reviews.llvm.org/D80752



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


[clang] 1643799 - Undo removal of test for dr777.

2020-06-02 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-06-02T14:19:42-07:00
New Revision: 16437992cac249f6fe1efd392d20e3469b47e39e

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

LOG: Undo removal of test for dr777.

Added: 


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

Removed: 




diff  --git a/clang/test/CXX/drs/dr7xx.cpp b/clang/test/CXX/drs/dr7xx.cpp
index dc1d7e86c58b..147560d3037b 100644
--- a/clang/test/CXX/drs/dr7xx.cpp
+++ b/clang/test/CXX/drs/dr7xx.cpp
@@ -219,4 +219,16 @@ namespace dr727 { // dr727: partial
   Collision c; // expected-note {{in instantiation of}}
 }
 
-// dr777 superseded by dr2233
+namespace dr777 { // dr777: 3.7
+#if __cplusplus >= 201103L
+template 
+void f(int i = 0, T ...args) {}
+void ff() { f(); }
+
+template 
+void g(int i = 0, T ...args, T ...args2) {}
+
+template 
+void h(int i = 0, T ...args, int j = 1) {}
+#endif
+}

diff  --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 1a06a85d0912..4abdc0fa2639 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -4681,7 +4681,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg777;>777
 CD2
 Default arguments and parameter packs
-Superseded by 2233
+Clang 3.7
   
   
 https://wg21.link/cwg778;>778
@@ -13213,7 +13213,7 @@ C++ defect report implementation 
status
 https://wg21.link/cwg2233;>2233
 DRWP
 Function parameter packs following default arguments
-Clang 11
+Clang 11
   
   
 https://wg21.link/cwg2234;>2234



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


[PATCH] D80876: [clang] Default to windows response files when running on windows

2020-06-02 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a subscriber: mati865.
mstorsjo added a comment.

In D80876#2069418 , @sbc100 wrote:

> If we want want to have the default on windows be dependent on mingw vs 
> not-mingw then we should do it across the board so it applies to llvm-ar, 
> lld, and other tools.  Right now I don't think any of those other tools have 
> any special mingw handling.


That's a fair point

> Would it make sense to expect mingw users to explicitly pass 
> `--rsp-quoting=gnu`.  I'm not very familiar with mingw use cases so I don't 
> know if that is an unreadable request?

It'd be quite a bit of a hassle - one of the main points is to be able to build 
the vast majority of projects with unix style build systems, so anything that 
requires changes to the build systems is a hassle. That'd require either adding 
wrapper executables that pass such defaults (msys2 currently don't use anything 
such), or being able to configure custom defaults in the build (like one can 
set the default target or default linker etc).

If the practical failure cases only show up with response files with "tricky" 
filenames containing single quotes or similar, I guess it might be an option to 
just change the defaults (as you say, other tools already do it this way) and 
require changes only in projects that use problematic file names.

@mati865 What do you think, from the msys2 point of view?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80876



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


[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

2020-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers planned changes to this revision.
nickdesaulniers added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5317
 /// EmitTopLevelDecl - Emit code for a single top level declaration.
 void CodeGenModule::EmitTopLevelDecl(Decl *D) {
   // Ignore dependent declarations.

dblaikie wrote:
> Since this is only for top-level decls, have you checked this works for types 
> inside namespaces, local types in functions (well, that one might not be 
> supported by GCC - so probably good to test/compare GCC's behavior here too), 
> etc?
class declared in namespace works with this patch.  I'll add a test.

local type in function, ex.
```
void foo(void) {
  struct bar {};
}
```
is not in the current diff, but is supported by gcc.  Let me fix that and add a 
test.  Great suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80242



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


[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-06-02 Thread Konrad Wilhelm Kleine via Phabricator via cfe-commits
kwk added a comment.

In D80531#2069383 , @njames93 wrote:

> LGTM with 2 small nits, but I'd still give a few days see if anyone else 
> wants to weight in.


I'm okay with this but I'd like to understand when it's time to wait for 
others? Only when a patch is approved or from the beginning of patch's life? I 
mean who looks at patches that are approved unless you filter for the amount of 
approvers? How many approvers must there be?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80531



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


[PATCH] D80660: clang: Add support for relative linker paths with -fuse-ld

2020-06-02 Thread Keith Smiley via Phabricator via cfe-commits
keith updated this revision to Diff 267991.
keith added a comment.

Fix test on windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80660

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/fuse-ld.c


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -4,7 +4,6 @@
 // RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 
-
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-FREEBSD-LD
@@ -94,3 +93,18 @@
 // RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-BFD
 // CHECK-WINDOWS-MSVC-BFD: "{{.*}}ld.bfd"
 // CHECK-WINDOWS-MSVC-BFD-SAME: "-o"
+
+// RUN: %clang %s -### \
+// RUN: -target x86_64-unknown-linux \
+// RUN: -working-directory "%S/Inputs" \
+// RUN: -fuse-ld=fuse_ld_windows/ld.foo.exe 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-RELATIVE-WORKDIR-LD
+// CHECK-RELATIVE-WORKDIR-LD-NOT: error: invalid linker name
+// CHECK-RELATIVE-WORKDIR-LD: 
test{{/|\\+}}Driver{{/|\\+}}Inputs{{/|\\+}}fuse_ld_windows/ld.foo.exe
+
+// RUN: cd "%S" && %clang %s -### \
+// RUN: -target x86_64-unknown-linux \
+// RUN: -fuse-ld=Inputs/fuse_ld_windows/ld.foo.exe 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-RELATIVE-LD
+// CHECK-RELATIVE-LD-NOT: error: invalid linker name
+// CHECK-RELATIVE-LD: 
test{{/|\\+}}Driver{{/|\\+}}Inputs{{/|\\+}}fuse_ld_windows/ld.foo.exe
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -557,6 +557,18 @@
 std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
 if (llvm::sys::fs::can_execute(LinkerPath))
   return LinkerPath;
+
+const Arg *WorkingDir = Args.getLastArg(options::OPT_working_directory);
+SmallString<128> ResolvedPath(UseLinker);
+if (WorkingDir) {
+  sys::fs::make_absolute(WorkingDir->getValue(), ResolvedPath);
+  if (llvm::sys::fs::can_execute(ResolvedPath))
+return std::string(ResolvedPath);
+}
+
+if (!sys::fs::make_absolute(ResolvedPath) &&
+llvm::sys::fs::can_execute(ResolvedPath))
+  return std::string(ResolvedPath);
   }
 
   if (A)


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -4,7 +4,6 @@
 // RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 
-
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-FREEBSD-LD
@@ -94,3 +93,18 @@
 // RUN:   | FileCheck %s --check-prefix CHECK-WINDOWS-MSVC-BFD
 // CHECK-WINDOWS-MSVC-BFD: "{{.*}}ld.bfd"
 // CHECK-WINDOWS-MSVC-BFD-SAME: "-o"
+
+// RUN: %clang %s -### \
+// RUN: -target x86_64-unknown-linux \
+// RUN: -working-directory "%S/Inputs" \
+// RUN: -fuse-ld=fuse_ld_windows/ld.foo.exe 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-RELATIVE-WORKDIR-LD
+// CHECK-RELATIVE-WORKDIR-LD-NOT: error: invalid linker name
+// CHECK-RELATIVE-WORKDIR-LD: test{{/|\\+}}Driver{{/|\\+}}Inputs{{/|\\+}}fuse_ld_windows/ld.foo.exe
+
+// RUN: cd "%S" && %clang %s -### \
+// RUN: -target x86_64-unknown-linux \
+// RUN: -fuse-ld=Inputs/fuse_ld_windows/ld.foo.exe 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-RELATIVE-LD
+// CHECK-RELATIVE-LD-NOT: error: invalid linker name
+// CHECK-RELATIVE-LD: test{{/|\\+}}Driver{{/|\\+}}Inputs{{/|\\+}}fuse_ld_windows/ld.foo.exe
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -557,6 +557,18 @@
 std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
 if (llvm::sys::fs::can_execute(LinkerPath))
   return LinkerPath;
+
+const Arg *WorkingDir = Args.getLastArg(options::OPT_working_directory);
+SmallString<128> ResolvedPath(UseLinker);
+if (WorkingDir) {
+  sys::fs::make_absolute(WorkingDir->getValue(), ResolvedPath);
+  if (llvm::sys::fs::can_execute(ResolvedPath))
+return std::string(ResolvedPath);
+}
+
+if (!sys::fs::make_absolute(ResolvedPath) &&
+llvm::sys::fs::can_execute(ResolvedPath))
+  return std::string(ResolvedPath);
   }
 
   if (A)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.

LGTM.  For non-Android, I think it makes sense to align with gcc as much as 
possible.

> This is mostly
>  for legacy/compatibility reasons, and the 2019 Q4 revision of the ARM
>  AAPCS looks to standardize r11 as the frame pointer for aarch32, though
>  this is not yet implemented in LLVM.

It's also really expensive to use r11 as a frame pointer in Thumb1.  I guess 
it's not impossible, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

2020-06-02 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D79744#2069324 , @arsenm wrote:

> In D79744#2050498 , @rjmccall wrote:
>
> > > For the purpose here, only the callee exists. This is essentially a 
> > > freestanding function, the entry point to the program.
> >
> > I'm definitely not going to let you add a new "generic" argument-passing 
> > convention and then only implement exactly the parts you need; that's just 
> > adding technical debt.
>
>
> I'm not sure what you mean here. I don't really want or need a totally new 
> generic argument passing convention. Not every IR feature is expected to be 
> implemented by every backend. For instance, inalloca/preallocated exist 
> solely to implement the x86 windows ABI and no other target tries to handle 
> them. This is just another flavor of the same fundamental concept of a 
> parameter attribute needed for a target specific calling convention.


I mean the Clang code for supporting this new convention, not the IR support.  
Of course LLVM has target-specific attributes.

>>> The load-from-constant nature needs to be exposed earlier, so I think this 
>>> necessarily involves changing the convention lowering in some way and it's 
>>> just a question of what it looks like. To summarize the 2.5 options I've 
>>> come up with are
>>> 
>>> 1. Use constant byval parameters, as this patch does. This requires the 
>>> least implementation effort but doesn't exactly fit in with byval as 
>>> defined.
>> 
>> I assume you generate a `byval` caller in some later pass, which will then 
>> implicitly copy.  Or do you lower `byval` in a nonstandard way in your 
>> backend?
> 
> No, the caller is only an external driver of some kind. Since the address 
> spaces don't match (and the source address space is read only), anything that 
> behaves like a stack byval doesn't really make sense from the beginning which 
> is why this patch changes the address space. If we were to leave this as a 
> stack address space, we would have to add an alloca and insert a memcpy 
> anyway. This would still leave an unusable byval argument around a pass could 
> still theoretically try to write into.
> 
> D79630  adds the lowering that uses this. 
> Because reasons (some of which I referenced in the previous comments), we 
> have 3 different implementations of the ABI. The byval pointer value is 
> simply replaced with a new pointer derived from a constant offset from an 
> intrinsic call (this is most obvious in the AMDGPULowerKernelArguments IR 
> version)
> 
>>> 1. Replace all IR argument uses with loads from a constant offset from an 
>>> intrinsic call. This still needs to leave the IR arguments in place because 
>>> we do need to know the original argument sizes and offsets, but they would 
>>> never have a use (or I would need to invent some other method of tracking 
>>> this information)
>> 
>> I guess passing aggregate arguments using a normal indirect convention has 
>> this same tracking problem.  You'd also have to copy on the caller side to 
>> maintain semantics, which is probably hard to eliminate, but I would guess 
>> `byval` has the same problem?
> 
> Isn't using byval the normal indirect convention? Really the only properties 
> I want that byval gives me is a pointee size and alignment. The most abstract 
> attribute I could probably come up with is a pointee(type) annotation that I 
> could use, without the stack copying implications of byval

`byval` is not an indirect convention.  It looks like one in IR, but it means 
"it's passed in the argument area of the stack", which is essentially more like 
being passed directly than otherwise.

In IR, the normal indirect convention is just to pass a pointer without any 
extra treatment.  Clang does set `nonnull dereferenceable(size) align 4` for 
optimization purposes, though.


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

https://reviews.llvm.org/D79744



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


[PATCH] D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Peter Smith via Phabricator via cfe-commits
psmith accepted this revision.
psmith added a comment.

LGTM from an Arm person now that the Android changes have been made.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:22
 #include "InputInfo.h"
+#include "MSP430.h"
 #include "PS4CPU.h"

nickdesaulniers wrote:
> Note to reviewers: the linter really wanted to touch this header inclusion 
> list, since I add "llvm/Support/Compiler.h" below.  Hopefully it's ok to just 
> include with this patch?
One possible solution here is to commit the reordering separately using an 
[NFC] refactoring commit, no separate review required, then commit the rest of 
the patch after that. Anyone doing a source code search will only pick up 
significant changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[clang] b5f2c4e - PR23029 / C++ DR2233: Allow expanded parameter packs to follow

2020-06-02 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-06-02T13:48:59-07:00
New Revision: b5f2c4e45b8d54063051e6955cef0bbb7b6ab0f8

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

LOG: PR23029 / C++ DR2233: Allow expanded parameter packs to follow
parameters with default arguments.

Directly follow the wording by relaxing the AST invariant that all
parameters after one with a default arguemnt also have default
arguments, and removing the diagnostic on missing default arguments
on a pack-expanded parameter following a parameter with a default
argument.

Testing also revealed that we need to special-case explicit
specializations of templates with a pack following a parameter with a
default argument, as such explicit specializations are otherwise
impossible to write. The standard wording doesn't address this case; a
issue has been filed.

This exposed a bug where we would briefly consider a parameter to have
no default argument while we parse a delay-parsed default argument for
that parameter, which is also fixed.

Partially incorporates a patch by Raul Tambre.

Added: 
clang/test/CXX/expr/expr.post/expr.call/p4.cpp

Modified: 
clang/include/clang/AST/Decl.h
clang/include/clang/Sema/Template.h
clang/lib/AST/Decl.cpp
clang/lib/AST/DeclCXX.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx03-extra-copy.cpp
clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-cxx0x-no-extra-copy.cpp
clang/test/CXX/drs/dr0xx.cpp
clang/test/CXX/drs/dr1xx.cpp
clang/test/CXX/drs/dr22xx.cpp
clang/test/CXX/drs/dr7xx.cpp
clang/test/SemaCXX/abstract.cpp
clang/test/SemaCXX/constant-expression-cxx11.cpp
clang/test/SemaCXX/decl-init-ref.cpp
clang/test/SemaCXX/implicit-exception-spec.cpp
clang/test/SemaCXX/warn-bool-conversion.cpp
clang/www/cxx_dr_status.html

Removed: 




diff  --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 2e1630827cce..185ba2f4b4c1 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2439,6 +2439,14 @@ class FunctionDecl : public DeclaratorDecl,
   /// parameters have default arguments (in C++).
   unsigned getMinRequiredArguments() const;
 
+  /// Determine whether this function has a single parameter, or multiple
+  /// parameters where all but the first have default arguments.
+  ///
+  /// This notion is used in the definition of copy/move constructors and
+  /// initializer list constructors. Note that, unlike getMinRequiredArguments,
+  /// parameter packs are not treated specially here.
+  bool hasOneParamOrDefaultArgs() const;
+
   /// Find the source location information for how the type of this function
   /// was written. May be absent (for example if the function was declared via
   /// a typedef) and may contain a 
diff erent type from that of the function

diff  --git a/clang/include/clang/Sema/Template.h 
b/clang/include/clang/Sema/Template.h
index 47d6143bdc9a..8166b6cf9b6f 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -422,6 +422,9 @@ class VarDecl;
 NamedDecl *
 getPartiallySubstitutedPack(const TemplateArgument **ExplicitArgs = 
nullptr,
 unsigned *NumExplicitArgs = nullptr) const;
+
+/// Determine whether D is a pack expansion created in this scope.
+bool isLocalPackExpansion(const Decl *D);
   };
 
   class TemplateDeclInstantiator

diff  --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index e6800073ee58..23547933b88b 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3264,13 +3264,27 @@ unsigned FunctionDecl::getMinRequiredArguments() const {
   if (!getASTContext().getLangOpts().CPlusPlus)
 return getNumParams();
 
+  // Note that it is possible for a parameter with no default argument to
+  // follow a parameter with a default argument.
   unsigned NumRequiredArgs = 0;
-  for (auto *Param : parameters())
-if (!Param->isParameterPack() && !Param->hasDefaultArg())
-  ++NumRequiredArgs;
+  unsigned MinParamsSoFar = 0;
+  for (auto *Param : parameters()) {
+if (!Param->isParameterPack()) {
+  ++MinParamsSoFar;
+  if (!Param->hasDefaultArg())
+NumRequiredArgs = MinParamsSoFar;
+}
+  }
   return NumRequiredArgs;
 }
 
+bool FunctionDecl::hasOneParamOrDefaultArgs() const {
+  return getNumParams() == 1 ||
+ (getNumParams() > 1 &&
+  std::all_of(param_begin() + 1, param_end(),
+  [](ParmVarDecl *P) { return P->hasDefaultArg(); }));
+}
+
 /// The combination of the extern and inline 

[PATCH] D80883: [Driver] Add multiclass OptInFlag and OptOutFlag to simplify boolean option definition

2020-06-02 Thread Fangrui Song via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7694b571d9fd: [Driver] Add multiclass OptInFlag and 
OptOutFlag to simplify boolean option… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80883

Files:
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -66,6 +66,26 @@
 // GCC compatibility.
 class IgnoredGCCCompat : Flags<[HelpHidden]> {}
 
+// A boolean option which is opt-in in CC1. The positive option exists in CC1 
and
+// Args.hasArg(OPT_ffoo) is used to check that the flag is enabled.
+// This is useful if the option is usually disabled.
+multiclass OptInFFlag flags=[]> {
+  def f#NAME : Flag<["-"], "f"#name>, Flags,
+   HelpText;
+  def fno_#NAME : Flag<["-"], "fno-"#name>, Flags,
+   HelpText;
+}
+
+// A boolean option which is opt-out in CC1. The negative option exists in CC1 
and
+// Args.hasArg(OPT_fno_foo) is used to check that the flag is disabled.
+multiclass OptOutFFlag flags=[]> {
+  def f#NAME : Flag<["-"], "f"#name>, Flags, 
HelpText;
+  def fno_ #NAME : Flag<["-"], "fno-"#name>, Flags,
+   HelpText;
+}
+
 /
 // Groups
 
@@ -824,10 +844,7 @@
 Group, Flags<[CC1Option, CoreOption]>,
 HelpText<"Generate instrumented code to collect order file into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
 
-def faddrsig : Flag<["-"], "faddrsig">, Group, Flags<[CoreOption, 
CC1Option]>,
-  HelpText<"Emit an address-significance table">;
-def fno_addrsig : Flag<["-"], "fno-addrsig">, Group, 
Flags<[CoreOption]>,
-  HelpText<"Don't emit an address-significance table">;
+defm addrsig : OptInFFlag<"addrsig", "Emit", "Don't emit", " an 
address-significance table", [CoreOption]>;
 def fblocks : Flag<["-"], "fblocks">, Group, Flags<[CoreOption, 
CC1Option]>,
   HelpText<"Enable the 'blocks' language feature">;
 def fbootclasspath_EQ : Joined<["-"], "fbootclasspath=">, Group;
@@ -969,15 +986,8 @@
 def fbracket_depth_EQ : Joined<["-"], "fbracket-depth=">, Group, 
Flags<[CoreOption]>;
 def fsignaling_math : Flag<["-"], "fsignaling-math">, Group;
 def fno_signaling_math : Flag<["-"], "fno-signaling-math">, Group;
-def fjump_tables : Flag<["-"], "fjump-tables">, Group;
-def fno_jump_tables : Flag<["-"], "fno-jump-tables">, Group, 
Flags<[CC1Option]>,
-  HelpText<"Do not use jump tables for lowering switches">;
-def fforce_enable_int128 : Flag<["-"], "fforce-enable-int128">,
-  Group, Flags<[CC1Option]>,
-  HelpText<"Enable support for int128_t type">;
-def fno_force_enable_int128 : Flag<["-"], "fno-force-enable-int128">,
-  Group, Flags<[CC1Option]>,
-  HelpText<"Disable support for int128_t type">;
+defm jump_tables : OptOutFFlag<"jump-tables", "Use", "Do not use", " jump 
tables for lowering switches">;
+defm force_enable_int128 : OptInFFlag<"force-enable-int128", "Enable", 
"Disable", " support for int128_t type">;
 def fkeep_static_consts : Flag<["-"], "fkeep-static-consts">, Group, 
Flags<[CC1Option]>,
   HelpText<"Keep static const variables even if unused">;
 def ffixed_point : Flag<["-"], "ffixed-point">, Group,


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -66,6 +66,26 @@
 // GCC compatibility.
 class IgnoredGCCCompat : Flags<[HelpHidden]> {}
 
+// A boolean option which is opt-in in CC1. The positive option exists in CC1 and
+// Args.hasArg(OPT_ffoo) is used to check that the flag is enabled.
+// This is useful if the option is usually disabled.
+multiclass OptInFFlag flags=[]> {
+  def f#NAME : Flag<["-"], "f"#name>, Flags,
+   HelpText;
+  def fno_#NAME : Flag<["-"], "fno-"#name>, Flags,
+   HelpText;
+}
+
+// A boolean option which is opt-out in CC1. The negative option exists in CC1 and
+// Args.hasArg(OPT_fno_foo) is used to check that the flag is disabled.
+multiclass OptOutFFlag flags=[]> {
+  def f#NAME : Flag<["-"], "f"#name>, Flags, HelpText;
+  def fno_ #NAME : Flag<["-"], "fno-"#name>, Flags,
+   HelpText;
+}
+
 /
 // Groups
 
@@ -824,10 +844,7 @@
 Group, Flags<[CC1Option, CoreOption]>,
 HelpText<"Generate instrumented code to collect order file into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">;
 
-def faddrsig : Flag<["-"], "faddrsig">, Group, Flags<[CoreOption, CC1Option]>,
-  HelpText<"Emit an address-significance table">;
-def fno_addrsig : Flag<["-"], "fno-addrsig">, Group, Flags<[CoreOption]>,
-  HelpText<"Don't emit an address-significance table">;
+defm addrsig : OptInFFlag<"addrsig", "Emit", "Don't emit", " 

[PATCH] D80900: [clangd] Use different FS in PreambleThread

2020-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

In D80900#2066327 , @sammccall wrote:

> TL;DR: I think there are three viable paths that we should consider:
>  A) minimal change: put the FSProvider in ParseInputs
>  B) this patch, but with more documentation and safety
>  C) fight to clean up VFS multithreading semantics
>
> Assuming we don't want to block on C, I'm not sure whether A or B is 
> conceptually better. But A is a smaller change and brings side benefits, so 
> wondering if you see advantages in B.


I didn't want to go for A) as I was afraid of satisfying the contract on 
`FileSystemProvider::getFileSystem`

  /// Context::current() will be the context passed to the clang entrypoint,
  /// such as addDocument(), and will also be propagated to result callbacks.

As this patch keep the `FileSystemProvider` inside TUScheduler.cpp and we need 
to ensure context is always the one we received in the entrypoint. Whereas 
`ParseInputs` is widely
passed around, and ensuring that assumption holds might be hard.

> You're right about the race condition of course and I'm sure this patch is 
> correct.
>  But this rubs me the wrong way a bit - the API change doesn't seem clearly 
> motivated either from a high level (we conceptually *want* a single FS to 
> build a single version of code) or from a low level (what are we actually 
> doing in parallel that isn't threadsafe?).
> 
> Looking at the low level perspective first. VFS has three classes 
> threadsafety issues:
> 
> - it contains the "working directory" as mutable state, which isn't 
> multi-threading friendly. We do set working directory concurrently with other 
> work, but always to the same thing. We could factor our way around this by 
> setting the working directory prior to any concurrency (to CDB.Directory) and 
> ensuring we never set it again.
> - the RealFilesystem in real-working-directory mode is a singleton so its use 
> is thread-hostile. This isn't really relevant to us because we use 
> emulated-working-directory mode and have distinct instances. But it probably 
> informed VFS's lack of useful threading semantics.
> - read operations aren't `const` or documented as threadsafe even though the 
> canonical `RealFilesystem` is. In-tree implementations seem to be threadsafe, 
> some out-of-tree ones are not :-( This definitely bites us here.
> 
>   I think it would be fundamentally possible to use a single VFS here. We 
> need to modify the VFS interface to make concurrent reads safe (and const, 
> while here) and fix any implementations. This isn't trivial. I think it's 
> worthwhile, but experience tells me different projects want different things 
> from the VFS interface...

I totally agree, this was actually my initial motivation. But I wasn't sure if 
it is worth the hassle, as this might receive some push-back from downstream 
users (VFS interface is not just in clang, but in llvm) and coming to a 
consensus is likely to take time.
It would be nice to clear that technical debt, but I don't think it provides 
any other practical benefits.

> The high-level view: IMO FSProvider is basically a hack around VFS's 
> thread-safety issues, rather than a fundamental part of the application - 
> it's a filesystem that doesn't have an attached working directory and whose 
> read operations (e.g. `getFileSystem()->status()`) are threadsafe. We could 
> rename FSProvider -> ThreadsafeFS and getFileSystem() -> newCursor(WD) or 
> something, and maybe we should. 
>  From this perspective, this patch is changing two things that have little to 
> do with each other: it's replacing FS -> ThreadsafeFS and it's moving the FS 
> ownership from one revision of the file to the TUScheduler itself.
> 
> I'm not really sure whether the latter is a good or a bad idea. If we want to 
> do it, the use of ParseInputs as the interface to TUScheduler is a bit 
> confusing - we should at least document that FS isn't used (as we do for 
> CompileCommand) and probably assert that it's null, too.

Right, the latter was a consequence of not storing FSProvider in ParseInputs, 
rather than a deliberate one. We can also push it around as a parameter to 
update, rather than a member if that looks more clear.
I didn't do that as it required more plumbing and change was actually local to 
TUScheduler.

> But maybe simpler would be to "just" change the VFS member in ParseInputs to 
> a `const FSProvider*`. This keeps the design approximately what it is now, 
> but simplifies the ParseInputs concept: it's now truly readonly data, and 
> functions that consume it won't have effects that escape.

Isn't this just what you proposed in A) ? My concern above applies to this one 
too. It is not a practical concern as of today, as most uses of ParseInputs is 
through callbacks in ASTWorker, hence the context should be implicitly set. 
Unless, someone attaches  another async callback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  

[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

2020-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 267970.
nickdesaulniers added a comment.

- add documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80242

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/CommandGuide/clang.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-unused-types.c
  clang/test/CodeGen/debug-info-unused-types.cpp

Index: clang/test/CodeGen/debug-info-unused-types.cpp
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-unused-types.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang++ -fno-eliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang++ -feliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang++ -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang++ -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+using foo = int;
+class bar {};
+enum class baz { BAZ };
+
+// CHECK: !3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz", file: !4, line: 7, baseType: !5, size: 32, flags: DIFlagEnumClass, elements: !6, identifier: "_ZTS3baz")
+// CHECK: !5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// CHECK: !7 = !DIEnumerator(name: "BAZ", value: 0)
+// CHECK: !9 = !DIDerivedType(tag: DW_TAG_typedef, name: "foo", file: !4, line: 5, baseType: !5)
+// CHECK: !10 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "bar", file: !4, line: 6, size: 8, flags: DIFlagTypePassByValue, elements: !11, identifier: "_ZTS3bar")
+
+// NODBG-NOT: !3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz", file: !4, line: 7, baseType: !5, size: 32, flags: DIFlagEnumClass, elements: !6, identifier: "_ZTS3baz")
+// NODBG-NOT: !5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// NODBG-NOT: !7 = !DIEnumerator(name: "BAZ", value: 0)
+// NODBG-NOT: !9 = !DIDerivedType(tag: DW_TAG_typedef, name: "foo", file: !4, line: 5, baseType: !5)
+// NODBG-NOT: !10 = distinct !DICompositeType(tag: DW_TAG_class_type, name: "bar", file: !4, line: 6, size: 8, flags: DIFlagTypePassByValue, elements: !11, identifier: "_ZTS3bar")
Index: clang/test/CodeGen/debug-info-unused-types.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-unused-types.c
@@ -0,0 +1,30 @@
+// RUN: %clang -fno-eliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang -feliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+typedef int my_int;
+struct foo {};
+enum bar { BAR };
+union baz {};
+
+// Check that debug info is emitted for the typedef, struct, enum, and union
+// when -fno-eliminate-unused-debug-types and -g are set.
+
+// CHECK: !3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "bar", file: !4, line: 7, baseType: !5, size: 32, elements: !6)
+// CHECK: !5 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+// CHECK: !7 = !DIEnumerator(name: "BAR", value: 0, isUnsigned: true)
+// CHECK: !9 = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int", file: !4, line: 5, baseType: !10)
+// CHECK: !10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// CHECK: !11 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo", file: !4, line: 6, elements: !12)
+// CHECK: !13 = distinct !DICompositeType(tag: DW_TAG_union_type, name: "baz", file: !4, line: 8, elements: !12)
+
+// Check that debug info is not emitted for the typedef, struct, enum, and
+// union when -fno-eliminate-unused-debug-types and -g are not set.
+
+// NODBG-NOT: !3 = !DICompositeType(tag: DW_TAG_enumeration_type, name: "bar", file: !4, line: 7, baseType: !5, size: 32, elements: !6)
+// NODBG-NOT: !5 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+// NODBG-NOT: !7 = !DIEnumerator(name: "BAR", value: 0, isUnsigned: true)
+// NODBG-NOT: !9 = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int", file: !4, line: 5, baseType: !10)
+// NODBG-NOT: !10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+// NODBG-NOT: !11 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "foo", file: !4, line: 6, elements: !12)
+// NODBG-NOT: !13 = distinct !DICompositeType(tag: DW_TAG_union_type, name: "baz", file: !4, line: 8, elements: !12)
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ 

[PATCH] D79587: [CodeGen][SVE] Legalisation of extends with scalable types

2020-06-02 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:10695
+// Truncate to prevent a DUP with an over wide constant
+SDValue Trunc = DAG.getNode(ISD::TRUNCATE, DL, EltTy, Dup->getOperand(0));
+

It's not legal to generate operations with type EltTy after legalization.  You 
get away with it here because it's guaranteed to constant-fold... but probably 
less confusing to use APInt::trunc instead.


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

https://reviews.llvm.org/D79587



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


[PATCH] D80931: AMDGPU: Fix clang side null pointer value for private

2020-06-02 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment.

Matt, this change to the test was causing a failure in the upstream PS4 linux 
bot here: 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/68593

I put in a fix for the test to unbreak the bot in 
086be9fb20489540e6228a6d9eb4afad533202fa 
 that I 
hope is correct, but please feel free to fix it up if it is not.


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

https://reviews.llvm.org/D80931



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


[clang] 26cb706 - [NFC][ASTMatchers] StringRef-ify and Twine-ify ASTMatchers tests.

2020-06-02 Thread Nathan James via cfe-commits

Author: Nathan James
Date: 2020-06-02T21:20:58+01:00
New Revision: 26cb70683bd4ffa49d94a8dad5ecfda549a673b0

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

LOG: [NFC][ASTMatchers] StringRef-ify and Twine-ify ASTMatchers tests.

Added: 


Modified: 
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
clang/unittests/ASTMatchers/ASTMatchersTest.h
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Removed: 




diff  --git a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp 
b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
index 9f8538edb35b..80eebf227a31 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -19,7 +19,7 @@ namespace clang {
 namespace ast_matchers {
 
 TEST(IsExpandedFromMacro, ShouldMatchInFile) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 void Test() { MY_MACRO(4); }
   )cc";
@@ -27,7 +27,7 @@ TEST(IsExpandedFromMacro, ShouldMatchInFile) {
 }
 
 TEST(IsExpandedFromMacro, ShouldMatchNested) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define WRAPPER(a) MY_MACRO(a)
 void Test() { WRAPPER(4); }
@@ -36,7 +36,7 @@ TEST(IsExpandedFromMacro, ShouldMatchNested) {
 }
 
 TEST(IsExpandedFromMacro, ShouldMatchIntermediate) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define IMPL(a) (4 + (a))
 #define MY_MACRO(a) IMPL(a)
 #define WRAPPER(a) MY_MACRO(a)
@@ -46,7 +46,7 @@ TEST(IsExpandedFromMacro, ShouldMatchIntermediate) {
 }
 
 TEST(IsExpandedFromMacro, ShouldMatchTransitive) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define WRAPPER(a) MY_MACRO(a)
 void Test() { WRAPPER(4); }
@@ -55,7 +55,7 @@ TEST(IsExpandedFromMacro, ShouldMatchTransitive) {
 }
 
 TEST(IsExpandedFromMacro, ShouldMatchArgument) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 void Test() {
   int x = 5;
@@ -68,7 +68,7 @@ TEST(IsExpandedFromMacro, ShouldMatchArgument) {
 // Like IsExpandedFromMacroShouldMatchArgumentMacro, but the argument is itself
 // a macro.
 TEST(IsExpandedFromMacro, ShouldMatchArgumentMacroExpansion) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define IDENTITY(a) (a)
 void Test() {
@@ -79,7 +79,7 @@ TEST(IsExpandedFromMacro, ShouldMatchArgumentMacroExpansion) {
 }
 
 TEST(IsExpandedFromMacro, ShouldMatchWhenInArgument) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 #define IDENTITY(a) (a)
 void Test() {
@@ -90,7 +90,7 @@ TEST(IsExpandedFromMacro, ShouldMatchWhenInArgument) {
 }
 
 TEST(IsExpandedFromMacro, ShouldMatchObjectMacro) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define PLUS (2 + 2)
 void Test() {
   PLUS;
@@ -100,7 +100,7 @@ TEST(IsExpandedFromMacro, ShouldMatchObjectMacro) {
 }
 
 TEST(IsExpandedFromMacro, ShouldMatchFromCommandLine) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 void Test() { FOUR_PLUS_FOUR; }
   )cc";
   EXPECT_TRUE(matchesConditionally(input,
@@ -109,7 +109,7 @@ TEST(IsExpandedFromMacro, ShouldMatchFromCommandLine) {
 }
 
 TEST(IsExpandedFromMacro, ShouldNotMatchBeginOnly) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define ONE_PLUS 1+
   void Test() { ONE_PLUS 4; }
   )cc";
@@ -118,7 +118,7 @@ TEST(IsExpandedFromMacro, ShouldNotMatchBeginOnly) {
 }
 
 TEST(IsExpandedFromMacro, ShouldNotMatchEndOnly) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define PLUS_ONE +1
   void Test() { 4 PLUS_ONE; }
   )cc";
@@ -127,7 +127,7 @@ TEST(IsExpandedFromMacro, ShouldNotMatchEndOnly) {
 }
 
 TEST(IsExpandedFromMacro, ShouldNotMatchDifferentMacro) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define MY_MACRO(a) (4 + (a))
 void Test() { MY_MACRO(4); }
   )cc";
@@ -135,7 +135,7 @@ TEST(IsExpandedFromMacro, ShouldNotMatchDifferentMacro) {
 }
 
 TEST(IsExpandedFromMacro, ShouldNotMatchDifferentInstances) {
-  std::string input = R"cc(
+  StringRef input = R"cc(
 #define FOUR 4
 void Test() { FOUR + FOUR; }
   )cc";
@@ -966,8 +966,8 @@ TEST(Matcher, VarDecl_IsStaticLocal) {
 }
 
 TEST(Matcher, VarDecl_StorageDuration) {
-  std::string T =
-"void f() { int x; static int y; } int a;static int b;extern int c;";
+  StringRef T =
+  "void f() { int x; static int y; } int a;static int b;extern int c;";
 
   EXPECT_TRUE(matches(T, varDecl(hasName("x"), 
hasAutomaticStorageDuration(;
   EXPECT_TRUE(
@@ -1629,7 +1629,7 @@ TEST(Matcher, HasNameSupportsOuterClasses) {
 }
 
 TEST(Matcher, 

[clang] 7694b57 - [Driver] Add multiclass OptInFlag and OptOutFlag to simplify boolean option definition

2020-06-02 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-06-02T13:22:12-07:00
New Revision: 7694b571d9fd6a8a6c96af1e7995068f7066f6f1

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

LOG: [Driver] Add multiclass OptInFlag and OptOutFlag to simplify boolean 
option definition

Reviewed By: dblaikie, echristo

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 3a5f75660421..b807febe1d44 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -66,6 +66,26 @@ class DocFlatten { bit DocFlatten = 1; }
 // GCC compatibility.
 class IgnoredGCCCompat : Flags<[HelpHidden]> {}
 
+// A boolean option which is opt-in in CC1. The positive option exists in CC1 
and
+// Args.hasArg(OPT_ffoo) is used to check that the flag is enabled.
+// This is useful if the option is usually disabled.
+multiclass OptInFFlag flags=[]> {
+  def f#NAME : Flag<["-"], "f"#name>, Flags,
+   HelpText;
+  def fno_#NAME : Flag<["-"], "fno-"#name>, Flags,
+   HelpText;
+}
+
+// A boolean option which is opt-out in CC1. The negative option exists in CC1 
and
+// Args.hasArg(OPT_fno_foo) is used to check that the flag is disabled.
+multiclass OptOutFFlag flags=[]> {
+  def f#NAME : Flag<["-"], "f"#name>, Flags, 
HelpText;
+  def fno_ #NAME : Flag<["-"], "fno-"#name>, Flags,
+   HelpText;
+}
+
 /
 // Groups
 
@@ -824,10 +844,7 @@ def forder_file_instrumentation : Flag<["-"], 
"forder-file-instrumentation">,
 Group, Flags<[CC1Option, CoreOption]>,
 HelpText<"Generate instrumented code to collect order file into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
 
-def faddrsig : Flag<["-"], "faddrsig">, Group, Flags<[CoreOption, 
CC1Option]>,
-  HelpText<"Emit an address-significance table">;
-def fno_addrsig : Flag<["-"], "fno-addrsig">, Group, 
Flags<[CoreOption]>,
-  HelpText<"Don't emit an address-significance table">;
+defm addrsig : OptInFFlag<"addrsig", "Emit", "Don't emit", " an 
address-significance table", [CoreOption]>;
 def fblocks : Flag<["-"], "fblocks">, Group, Flags<[CoreOption, 
CC1Option]>,
   HelpText<"Enable the 'blocks' language feature">;
 def fbootclasspath_EQ : Joined<["-"], "fbootclasspath=">, Group;
@@ -969,15 +986,8 @@ def fno_math_errno : Flag<["-"], "fno-math-errno">, 
Group;
 def fbracket_depth_EQ : Joined<["-"], "fbracket-depth=">, Group, 
Flags<[CoreOption]>;
 def fsignaling_math : Flag<["-"], "fsignaling-math">, Group;
 def fno_signaling_math : Flag<["-"], "fno-signaling-math">, Group;
-def fjump_tables : Flag<["-"], "fjump-tables">, Group;
-def fno_jump_tables : Flag<["-"], "fno-jump-tables">, Group, 
Flags<[CC1Option]>,
-  HelpText<"Do not use jump tables for lowering switches">;
-def fforce_enable_int128 : Flag<["-"], "fforce-enable-int128">,
-  Group, Flags<[CC1Option]>,
-  HelpText<"Enable support for int128_t type">;
-def fno_force_enable_int128 : Flag<["-"], "fno-force-enable-int128">,
-  Group, Flags<[CC1Option]>,
-  HelpText<"Disable support for int128_t type">;
+defm jump_tables : OptOutFFlag<"jump-tables", "Use", "Do not use", " jump 
tables for lowering switches">;
+defm force_enable_int128 : OptInFFlag<"force-enable-int128", "Enable", 
"Disable", " support for int128_t type">;
 def fkeep_static_consts : Flag<["-"], "fkeep-static-consts">, Group, 
Flags<[CC1Option]>,
   HelpText<"Keep static const variables even if unused">;
 def ffixed_point : Flag<["-"], "ffixed-point">, Group,



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


Re: [PATCH] D80293: [clangd] Run PreambleThread in async mode behind a flag

2020-06-02 Thread Kadir Çetinkaya via cfe-commits
thanks for letting me know. it is annoying that the test is flaky, but I
don't see that flakiness on any other buildbot I've access to.

Is there any chance that you are running in a custom build configuration
that I can try to repro the failure? Also what's the falkiness ratio?

On Tue, Jun 2, 2020 at 1:43 AM Wolfgang Pieb via Phabricator <
revi...@reviews.llvm.org> wrote:

> wolfgangp added a comment.
>
> Hi!
>
> Shortly after this patch went in, we started to see intermittent failures
> with the unittest TUSchedulerTests.ManyUpdates. The build server (Windows)
> we're running it on is quite powerful and our own hand-run tests only take
> about 400 ms, so we don't really understand what could be happening. Any
> help would be appreciated.
>
> Here is an excerpt from the log:
>
> Note: Google Test filter = TUSchedulerTests.ManyUpdates
>
> [==] Running 1 test from 1 test case.
>
> [--] Global test environment set-up.
>
> [--] 1 test from TUSchedulerTests
>
> [ RUN  ] TUSchedulerTests.ManyUpdates
>
> C:\j\w\...\clang-tools-extra\clangd\unittests\TUSchedulerTests.cpp(508):
> error: Value of: S.blockUntilIdle(timeoutSeconds(10))
>
>   Actual: false
>
> Expected: true
>
> [  FAILED  ] TUSchedulerTests.ManyUpdates (10576 ms)
>
> [--] 1 test from TUSchedulerTests (10576 ms total)
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D80293/new/
>
> https://reviews.llvm.org/D80293
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 086be9f - Fix test on PS4 linux bot.

2020-06-02 Thread Douglas Yung via cfe-commits

Author: Douglas Yung
Date: 2020-06-02T20:17:02Z
New Revision: 086be9fb20489540e6228a6d9eb4afad533202fa

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

LOG: Fix test on PS4 linux bot.

Commit 301a6da8c24a09052e3bda10e90b450b7b39ffea changed the test and modified a 
CHECK
line that is inconsisent with similar lines elsewhere in the file and was 
causing failures
when run in slightly different configurations. This change makes the line more 
consistent
and should fix the bot failure.

Failure link: 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/68593

Added: 


Modified: 
clang/test/CodeGenOpenCL/amdgpu-nullptr.cl

Removed: 




diff  --git a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl 
b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
index 753f7f6f4406..ddf358c1188e 100644
--- a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
+++ b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -358,8 +358,8 @@ void test_cast_null_pointer_to_sizet_calee(size_t 
arg_private,
size_t arg_generic);
 
 // CHECK-LABEL: test_cast_null_pointer_to_sizet
-// CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 ptrtoint (i8 
addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*) to i64), i64 
ptrtoint (i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*) to 
i64), i64 0, i64 0, i64 0) #7
-// CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 ptrtoint (i8 
addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*) to i64), i64 
ptrtoint (i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*) to 
i64), i64 0, i64 0, i64 0) #7
+// CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 ptrtoint (i8 
addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*) to i64), i64 
ptrtoint (i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*) to 
i64), i64 0, i64 0, i64 0)
+// CHECK: call void @test_cast_null_pointer_to_sizet_calee(i64 ptrtoint (i8 
addrspace(5)* addrspacecast (i8* null to i8 addrspace(5)*) to i64), i64 
ptrtoint (i8 addrspace(3)* addrspacecast (i8* null to i8 addrspace(3)*) to 
i64), i64 0, i64 0, i64 0)
 void test_cast_null_pointer_to_sizet(void) {
   test_cast_null_pointer_to_sizet_calee((size_t)((private char*)0),
 (size_t)((local char*)0),



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


[PATCH] D81017: [Analyzer][WebKit] Check record definition is available in NoUncountedMembers checker

2020-06-02 Thread Jan Korous via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd61ad660503d: [Analyzer][WebKit] Check record definition is 
available in NoUncountedMembers… (authored by jkorous).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81017

Files:
  clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
  clang/test/Analysis/Checkers/WebKit/uncounted-members-regression-46142.cpp


Index: 
clang/test/Analysis/Checkers/WebKit/uncounted-members-regression-46142.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/WebKit/uncounted-members-regression-46142.cpp
@@ -0,0 +1,9 @@
+// regression test for https://bugs.llvm.org/show_bug.cgi?id=46142
+
+// RUN: %clang_analyze_cc1 
-analyzer-checker=webkit.WebKitNoUncountedMemberChecker -verify %s
+// expected-no-diagnostics
+
+class ClassWithoutADefinition;
+class Foo {
+const ClassWithoutADefinition *foo;
+};
Index: clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
@@ -75,7 +75,8 @@
 continue;
 
   if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
-if (isRefCountable(MemberCXXRD))
+// If we don't see the definition we just don't know.
+if (MemberCXXRD->hasDefinition() && isRefCountable(MemberCXXRD))
   reportBug(Member, MemberType, MemberCXXRD, RD);
   }
 }


Index: clang/test/Analysis/Checkers/WebKit/uncounted-members-regression-46142.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/WebKit/uncounted-members-regression-46142.cpp
@@ -0,0 +1,9 @@
+// regression test for https://bugs.llvm.org/show_bug.cgi?id=46142
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.WebKitNoUncountedMemberChecker -verify %s
+// expected-no-diagnostics
+
+class ClassWithoutADefinition;
+class Foo {
+const ClassWithoutADefinition *foo;
+};
Index: clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
@@ -75,7 +75,8 @@
 continue;
 
   if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
-if (isRefCountable(MemberCXXRD))
+// If we don't see the definition we just don't know.
+if (MemberCXXRD->hasDefinition() && isRefCountable(MemberCXXRD))
   reportBug(Member, MemberType, MemberCXXRD, RD);
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D80242: [Clang] implement -fno-eliminate-unused-debug-types

2020-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:768
   Opts.DebugFwdTemplateParams = Args.hasArg(OPT_debug_forward_template_params);
+  Opts.DebugUnusedTypes = Args.hasArg(OPT_eliminate_unused_debug_types_fno);
   Opts.EmbedSource = Args.hasArg(OPT_gembed_source);

dblaikie wrote:
> dblaikie wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > dblaikie wrote:
> > > > > Could this be rolled into the debug-info-kind? (a kind beyond 
> > > > > "standalone")
> > > > That sounds like a good idea.  Looking at the definition of 
> > > > `DebugInfoKind` 
> > > > (llvm/llvm-project/clang/include/clang/Basic/DebugInfoOptions.h), it 
> > > > seems that `DebugInfoKind` is an `enum` that defines a "level" of debug 
> > > > info to emit? Looking at the guard in 
> > > > `CGDebugInfo::EmitExplicitCastType` 
> > > > (llvm/llvm-project/clang/lib/CodeGen/CGDebugInfo.cpp), it calls 
> > > > `CodeGenOptions::hasReducedDebugInfo()` which does a comparison against 
> > > > a certain level.  That seems to indicate the order of the enumerations 
> > > > is important.  Do you have a suggestion what order I should add the new 
> > > > enum?
> > > > 
> > > > I'm guessing after `LimitedDebugInfo` or `DebugInfoConstructor`, but 
> > > > before `FullDebugInfo`? (I find the name of the method 
> > > > `hasReducedDebugInfo` confusing in that regard).
> > > Ok, so I have a diff that implements this approach.  I feel like I should 
> > > maybe publish it as a child commit to this one, to be able to more easily 
> > > discuss it?
> > > 
> > > Two problems I run into:
> > > 1. as alluded to in my previous response in this thread, `DebugInfoKind` 
> > > is an enum that specifies a level.  It tends to "drag" other debug flags 
> > > in based on the ordering.  Looking at extending the `switch` in 
> > > `CGDebugInfo::CreateCompileUnit` (clang/lib/CodeGen/CGDebugInfo.cpp), 
> > > it's not at all clear to me which we existing case should we choose?
> > > 2. we want the flag `-fno-eliminate-unused-debug-types` to match GCC for 
> > > compatibility.  We can additionally add a new debug info kind like 
> > > `"standalone"` (clang/lib/Frontend/CompilerInvocation.cpp), but it's not 
> > > clear how the two flags together should interact.
> > > 
> > > The suggestion for a new debug info kind feels like a recommendation to 
> > > add a new "level" of debug info, but `-fno-eliminate-unused-debug-types` 
> > > feels like it should be mutually exclusive of debug info kind? (I guess 
> > > GCC does *require* `-g` with `-fno-eliminate-unused-debug-types`)
> > > 
> > > @dblaikie maybe you have more recommendations on this?
> > This value would probably go "after" "full" (full isn't full enough, as 
> > you've found - you need a mode that's even "fullerer")
> > 
> > Perhaps renaming "Full" to "Referenced" and then introducing this new kind 
> > as the new "Full" or maybe under a somewhat different name to avoid 
> > ambiguity. 
> > 
> > Any suggestions on a less confusing name for "hasReducedDebugInfo"? (I 
> > think it was basically "Has less than full debug info"... but, yep, it 
> > doesn't do that at all - it's "HasTypeAndVariableDebugInfo" by the looks of 
> > it/by the comment)
> > Ok, so I have a diff that implements this approach. I feel like I should 
> > maybe publish it as a child commit to this one, to be able to more easily 
> > discuss it?
> > 
> > Two problems I run into:
> > 
> > 1. as alluded to in my previous response in this thread, DebugInfoKind is 
> > an enum that specifies a level. It tends to "drag" other debug flags in 
> > based on the ordering. Looking at extending the switch in 
> > CGDebugInfo::CreateCompileUnit (clang/lib/CodeGen/CGDebugInfo.cpp), it's 
> > not at all clear to me which we existing case should we choose?
> > 2. we want the flag -fno-eliminate-unused-debug-types to match GCC for 
> > compatibility. We can additionally add a new debug info kind like 
> > "standalone" (clang/lib/Frontend/CompilerInvocation.cpp), but it's not 
> > clear how the two flags together should interact.
> 
> I'm not suggesting adding a new driver-level flag for this, but implementing 
> the GCC flag name in terms of the debug-info-kind. Probably by having 
> "-fno-eliminate-unused-debug-types" override "-fstandalone-debug" - whichever 
> comes later wins. Because they are part of a progression. Though admittedly 
> that might get a smidge confusing about exactly whit no/yes versions of these 
> two flags override each other - but I think that's inevitable confusion with 
> the nature of these flags.
> 
> What does GCC do for its -f[no-]emit-class-debug-always (which is somewhat 
> similar to -fstandalone-debug) V -f[no-]eliminate-unused-debug-types? I'm not 
> sure we'll want to emulate the behavior exxactly, but it's probably a good 
> place to start to see if there's an existing model that looks OK here.
> 
> > The suggestion for a new debug 

[clang] d61ad66 - [Analyzer][WebKit] Check record definition is available in NoUncountedMembers checker

2020-06-02 Thread Jan Korous via cfe-commits

Author: Jan Korous
Date: 2020-06-02T13:10:36-07:00
New Revision: d61ad660503d2e0c7ba9981ba6526ae0c2f3b7cc

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

LOG: [Analyzer][WebKit] Check record definition is available in 
NoUncountedMembers checker

isRefCountable asserts that the record passed as an argument has a definition 
available.

Fixes: https://bugs.llvm.org/show_bug.cgi?id=46142

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

Added: 
clang/test/Analysis/Checkers/WebKit/uncounted-members-regression-46142.cpp

Modified: 
clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp

Removed: 




diff  --git 
a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
index 89caf602a17e..db53db1587d5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/NoUncountedMembersChecker.cpp
@@ -75,7 +75,8 @@ class NoUncountedMemberChecker
 continue;
 
   if (auto *MemberCXXRD = MemberType->getPointeeCXXRecordDecl()) {
-if (isRefCountable(MemberCXXRD))
+// If we don't see the definition we just don't know.
+if (MemberCXXRD->hasDefinition() && isRefCountable(MemberCXXRD))
   reportBug(Member, MemberType, MemberCXXRD, RD);
   }
 }

diff  --git 
a/clang/test/Analysis/Checkers/WebKit/uncounted-members-regression-46142.cpp 
b/clang/test/Analysis/Checkers/WebKit/uncounted-members-regression-46142.cpp
new file mode 100644
index ..20e58e7926d8
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-members-regression-46142.cpp
@@ -0,0 +1,9 @@
+// regression test for https://bugs.llvm.org/show_bug.cgi?id=46142
+
+// RUN: %clang_analyze_cc1 
-analyzer-checker=webkit.WebKitNoUncountedMemberChecker -verify %s
+// expected-no-diagnostics
+
+class ClassWithoutADefinition;
+class Foo {
+const ClassWithoutADefinition *foo;
+};



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


[PATCH] D80968: [WebAssembly] Improve macro hygiene in wasm_simd128.h

2020-06-02 Thread Thomas Lively via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG237be3404b44: [WebAssembly] Improve macro hygiene in 
wasm_simd128.h (authored by tlively).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80968

Files:
  clang/lib/Headers/wasm_simd128.h


Index: clang/lib/Headers/wasm_simd128.h
===
--- clang/lib/Headers/wasm_simd128.h
+++ clang/lib/Headers/wasm_simd128.h
@@ -1034,24 +1034,24 @@
 #define wasm_v16x8_shuffle(__a, __b, __c0, __c1, __c2, __c3, __c4, __c5, __c6, 
\
__c7)   
\
   ((v128_t)__builtin_wasm_shuffle_v8x16(   
\
-  (__i8x16)(__a), (__i8x16)(__b), __c0 * 2, __c0 * 2 + 1, __c1 * 2,
\
-  __c1 * 2 + 1, __c2 * 2, __c2 * 2 + 1, __c3 * 2, __c3 * 2 + 1, __c4 * 2,  
\
-  __c4 * 2 + 1, __c5 * 2, __c5 * 2 + 1, __c6 * 2, __c6 * 2 + 1, __c7 * 2,  
\
-  __c7 * 2 + 1))
+  (__i8x16)(__a), (__i8x16)(__b), (__c0)*2, (__c0)*2 + 1, (__c1)*2,
\
+  (__c1)*2 + 1, (__c2)*2, (__c2)*2 + 1, (__c3)*2, (__c3)*2 + 1, (__c4)*2,  
\
+  (__c4)*2 + 1, (__c5)*2, (__c5)*2 + 1, (__c6)*2, (__c6)*2 + 1, (__c7)*2,  
\
+  (__c7)*2 + 1))
 
 #define wasm_v32x4_shuffle(__a, __b, __c0, __c1, __c2, __c3)   
\
   ((v128_t)__builtin_wasm_shuffle_v8x16(   
\
-  (__i8x16)(__a), (__i8x16)(__b), __c0 * 4, __c0 * 4 + 1, __c0 * 4 + 2,
\
-  __c0 * 4 + 3, __c1 * 4, __c1 * 4 + 1, __c1 * 4 + 2, __c1 * 4 + 3,
\
-  __c2 * 4, __c2 * 4 + 1, __c2 * 4 + 2, __c2 * 4 + 3, __c3 * 4,
\
-  __c3 * 4 + 1, __c3 * 4 + 2, __c3 * 4 + 3))
+  (__i8x16)(__a), (__i8x16)(__b), (__c0)*4, (__c0)*4 + 1, (__c0)*4 + 2,
\
+  (__c0)*4 + 3, (__c1)*4, (__c1)*4 + 1, (__c1)*4 + 2, (__c1)*4 + 3,
\
+  (__c2)*4, (__c2)*4 + 1, (__c2)*4 + 2, (__c2)*4 + 3, (__c3)*4,
\
+  (__c3)*4 + 1, (__c3)*4 + 2, (__c3)*4 + 3))
 
 #define wasm_v64x2_shuffle(__a, __b, __c0, __c1)   
\
   ((v128_t)__builtin_wasm_shuffle_v8x16(   
\
-  (__i8x16)(__a), (__i8x16)(__b), __c0 * 8, __c0 * 8 + 1, __c0 * 8 + 2,
\
-  __c0 * 8 + 3, __c0 * 8 + 4, __c0 * 8 + 5, __c0 * 8 + 6, __c0 * 8 + 7,
\
-  __c1 * 8, __c1 * 8 + 1, __c1 * 8 + 2, __c1 * 8 + 3, __c1 * 8 + 4,
\
-  __c1 * 8 + 5, __c1 * 8 + 6, __c1 * 8 + 7))
+  (__i8x16)(__a), (__i8x16)(__b), (__c0)*8, (__c0)*8 + 1, (__c0)*8 + 2,
\
+  (__c0)*8 + 3, (__c0)*8 + 4, (__c0)*8 + 5, (__c0)*8 + 6, (__c0)*8 + 7,
\
+  (__c1)*8, (__c1)*8 + 1, (__c1)*8 + 2, (__c1)*8 + 3, (__c1)*8 + 4,
\
+  (__c1)*8 + 5, (__c1)*8 + 6, (__c1)*8 + 7))
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_v8x16_swizzle(v128_t __a,
v128_t __b) {


Index: clang/lib/Headers/wasm_simd128.h
===
--- clang/lib/Headers/wasm_simd128.h
+++ clang/lib/Headers/wasm_simd128.h
@@ -1034,24 +1034,24 @@
 #define wasm_v16x8_shuffle(__a, __b, __c0, __c1, __c2, __c3, __c4, __c5, __c6, \
__c7)   \
   ((v128_t)__builtin_wasm_shuffle_v8x16(   \
-  (__i8x16)(__a), (__i8x16)(__b), __c0 * 2, __c0 * 2 + 1, __c1 * 2,\
-  __c1 * 2 + 1, __c2 * 2, __c2 * 2 + 1, __c3 * 2, __c3 * 2 + 1, __c4 * 2,  \
-  __c4 * 2 + 1, __c5 * 2, __c5 * 2 + 1, __c6 * 2, __c6 * 2 + 1, __c7 * 2,  \
-  __c7 * 2 + 1))
+  (__i8x16)(__a), (__i8x16)(__b), (__c0)*2, (__c0)*2 + 1, (__c1)*2,\
+  (__c1)*2 + 1, (__c2)*2, (__c2)*2 + 1, (__c3)*2, (__c3)*2 + 1, (__c4)*2,  \
+  (__c4)*2 + 1, (__c5)*2, (__c5)*2 + 1, (__c6)*2, (__c6)*2 + 1, (__c7)*2,  \
+  (__c7)*2 + 1))
 
 #define wasm_v32x4_shuffle(__a, __b, __c0, __c1, __c2, __c3)   \
   ((v128_t)__builtin_wasm_shuffle_v8x16(   \
-  (__i8x16)(__a), (__i8x16)(__b), __c0 * 4, __c0 * 4 + 1, __c0 * 4 + 2,\
-  __c0 * 4 + 3, __c1 * 4, __c1 * 4 + 1, __c1 * 4 + 2, __c1 * 4 + 3,\
-  __c2 * 4, __c2 * 4 + 1, __c2 * 4 + 2, __c2 * 4 + 3, __c3 * 4,\
-  __c3 * 4 + 1, __c3 * 4 + 2, __c3 * 4 + 3))
+  (__i8x16)(__a), (__i8x16)(__b), (__c0)*4, (__c0)*4 + 1, (__c0)*4 + 2,\
+  (__c0)*4 + 3, (__c1)*4, (__c1)*4 + 1, (__c1)*4 + 2, (__c1)*4 + 3,\
+  (__c2)*4, (__c2)*4 + 1, (__c2)*4 + 2, (__c2)*4 + 3, (__c3)*4,\
+  (__c3)*4 + 1, (__c3)*4 + 2, (__c3)*4 + 3))
 
 #define wasm_v64x2_shuffle(__a, __b, __c0, __c1)   \
   ((v128_t)__builtin_wasm_shuffle_v8x16(   \
-  (__i8x16)(__a), 

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-06-02 Thread Konrad Wilhelm Kleine via Phabricator via cfe-commits
kwk added a comment.

Done.




Comment at: 
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h:52
+
+  const std::string getMacroName() const { return MacroName; }
+

njames93 wrote:
> Return by reference here.
Yeah, that makes sense. Stupid me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80531



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


[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

2020-06-02 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu marked an inline comment as done.
zequanwu added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:676
+  Value v = vals[vd];
+  if (isUninitialized(v))
+handler.handleConstRefUseOfUninitVariable(vd, getUninitUse(ex, vd, v));

MaskRay wrote:
> This should use isAlwaysUninit. I fixed it in 
> 7096e04a6831d4668c39b388ccd166f84de69191
> 
> Otherwise there are multiple false positives in stage2 build of llvm-project, 
> e.g.
> 
> ```
> if (a < 42)
>   var = 1;
> if (a < 42)
>   const_ref_use(var);
> ```
Thanks for the fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79895



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


[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-06-02 Thread Konrad Wilhelm Kleine via Phabricator via cfe-commits
kwk updated this revision to Diff 267960.
kwk marked 4 inline comments as done.
kwk added a comment.

- Return macro name by reference
- Add colon to paragraph showing code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80531

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp
  
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-replace-disallow-copy-and-assign-macro.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy -format-style=LLVM -check-suffix=DEFAULT %s \
+// RUN:   modernize-replace-disallow-copy-and-assign-macro %t
+
+// RUN: %check_clang_tidy -format-style=LLVM -check-suffix=DIFFERENT-NAME %s \
+// RUN:  modernize-replace-disallow-copy-and-assign-macro %t \
+// RUN:  -config="{CheckOptions: [ \
+// RUN:   {key: modernize-replace-disallow-copy-and-assign-macro.MacroName, \
+// RUN:value: MY_MACRO_NAME}]}"
+
+// RUN: %check_clang_tidy -format-style=LLVM -check-suffix=FINALIZE %s \
+// RUN:  modernize-replace-disallow-copy-and-assign-macro %t \
+// RUN:  -config="{CheckOptions: [ \
+// RUN:   {key: modernize-replace-disallow-copy-and-assign-macro.MacroName, \
+// RUN:value: DISALLOW_COPY_AND_ASSIGN_FINALIZE}]}"
+
+// RUN: clang-tidy %s -checks="-*,modernize-replace-disallow-copy-and-assign-macro" \
+// RUN:   -config="{CheckOptions: [ \
+// RUN:{key: modernize-replace-disallow-copy-and-assign-macro.MacroName, \
+// RUN: value: DISALLOW_COPY_AND_ASSIGN_MORE_AGUMENTS}]}" | count 0
+
+// RUN: clang-tidy %s -checks="-*,modernize-replace-disallow-copy-and-assign-macro" \
+// RUN:   -config="{CheckOptions: [ \
+// RUN:{key: modernize-replace-disallow-copy-and-assign-macro.MacroName, \
+// RUN: value: DISALLOW_COPY_AND_ASSIGN_NEEDS_PREEXPANSION}]}" | count 0
+
+// Note: the last two tests expect no diagnostics, but FileCheck cannot handle
+// that, hence the use of | count 0.
+
+#define DISALLOW_COPY_AND_ASSIGN(TypeName)
+
+class TestClass1 {
+private:
+  DISALLOW_COPY_AND_ASSIGN(TestClass1);
+};
+// CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:3: warning: prefer deleting copy constructor and assignment operator over using macro 'DISALLOW_COPY_AND_ASSIGN' [modernize-replace-disallow-copy-and-assign-macro]
+// CHECK-FIXES-DEFAULT:  {{^}}  TestClass1(const TestClass1 &) = delete;{{$}}
+// CHECK-FIXES-DEFAULT-NEXT: {{^}}  const TestClass1 =(const TestClass1 &) = delete;{{$}}
+
+#define MY_MACRO_NAME(TypeName)
+
+class TestClass2 {
+private:
+  MY_MACRO_NAME(TestClass2);
+};
+// CHECK-MESSAGES-DIFFERENT-NAME: :[[@LINE-2]]:3: warning: prefer deleting copy constructor and assignment operator over using macro 'MY_MACRO_NAME' [modernize-replace-disallow-copy-and-assign-macro]
+// CHECK-FIXES-DIFFERENT-NAME:  {{^}}  TestClass2(const TestClass2 &) = delete;{{$}}
+// CHECK-FIXES-DIFFERENT-NAME-NEXT: {{^}}  const TestClass2 =(const TestClass2 &) = delete;{{$}}
+
+#define DISALLOW_COPY_AND_ASSIGN_FINALIZE(TypeName) \
+  TypeName(const TypeName &) = delete;  \
+  const TypeName =(const TypeName &) = delete;
+
+class TestClass3 {
+private:
+  // Notice, that the macro allows to be used without a semicolon because the
+  // macro definition already contains one above. Therefore our replacement must
+  // contain a semicolon at the end.
+  DISALLOW_COPY_AND_ASSIGN_FINALIZE(TestClass3)
+};
+// CHECK-MESSAGES-FINALIZE: :[[@LINE-2]]:3: warning: prefer deleting copy constructor and assignment operator over using macro 'DISALLOW_COPY_AND_ASSIGN_FINALIZE' [modernize-replace-disallow-copy-and-assign-macro]
+// CHECK-FIXES-FINALIZE:  {{^}}  TestClass3(const TestClass3 &) = delete;{{$}}
+// CHECK-FIXES-FINALIZE-NEXT: {{^}}  const TestClass3 =(const TestClass3 &) = delete;{{$}}
+
+#define DISALLOW_COPY_AND_ASSIGN_MORE_AGUMENTS(A, B)
+
+class TestClass4 {
+private:
+  DISALLOW_COPY_AND_ASSIGN_MORE_AGUMENTS(TestClass4, TestClass4);
+};
+// CHECK-MESSAGES-MORE-ARGUMENTS-NOT: warning: prefer deleting copy constructor and assignment operator over using macro 'DISALLOW_COPY_AND_ASSIGN_MORE_AGUMENTS'
+
+#define DISALLOW_COPY_AND_ASSIGN_NEEDS_PREEXPANSION(A)
+#define TESTCLASS TestClass5
+
+class TestClass5 {
+private:
+  DISALLOW_COPY_AND_ASSIGN_NEEDS_PREEXPANSION(TESTCLASS);
+};
+// 

[PATCH] D80903: [analyzer] Ignore calculated indices of <= 0 in VLASizeChecker

2020-06-02 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision.
Szelethus added a comment.

LGTM! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80903



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


[PATCH] D80876: [clang] Default to windows response files when running on windows

2020-06-02 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment.

The problem I ran into regarding the quoting style differences was trying to 
refer to filenames that contains single quote characters:

Here is our test case that I've currently disabled on windows pending the 
resolution of this issue:
https://github.com/emscripten-core/emscripten/blob/20ecfa2f8101fb21a9c0f9d75e3aa2dee6468e95/tests/test_other.py#L8379


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80876



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


[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-02 Thread Alexandre Ganea via Phabricator via cfe-commits
aganea marked an inline comment as done.
aganea added a comment.

In D80833#2069246 , @amccarth wrote:

> Does the full path to the tool end up only in the object file, or does this 
> make it all the way to the final executable or library?


The `LF_BUILDINFO` records for each .OBJ are emitted as they are in the final 
.PDB.

> Does embedding full paths affect distributed builds or build reproducibility?

It does affect reproducibility for distributed builds, the contents of 
`LF_BUILDINFO` for a remotely-compiled .OBJ will be different in the final .PDB.
In our case, compilation jobs can be executed locally or remotely, depending on 
how they are queued.
For example,

- If a local compilation, `C:\.nuget\llvm.10.0.0.2\bin\clang-cl.exe` would be 
stamped in the `LF_BUILDINFO` record.
- If a remote compilation, 
`C:\Users\SomeUser\AppData\Local\Temp\.fbuild.tmp\worker\toolchain.130589cdf35aed3b\clang-cl.exe`
 would be stamped (the compiler executable is sent to the remote worker).

But you've got a good point. We would need an way to force the remote compiler 
to stamp the local path (which is unique for all users). We've got a manual 
override if the compiler path referred by `LF_BUILDINFO` can't be found 
locally, but this is a bit annoying, see: 
https://liveplusplus.tech/docs/documentation.html#FASTBuild.
It is better if the right information was there in the first place in the .OBJ. 
Could we do a remapping instead through VFS maybe? I'd like to avoid adding 
yet-a-new-flag to handle that.




Comment at: clang/include/clang/Basic/CodeGenOptions.h:320
+  /// Executable and command-line used to create a given CompilerInvocation.
+  const char *BuildTool = nullptr;
+  ArrayRef CommandLineArgs;

hans wrote:
> aganea wrote:
> > hans wrote:
> > > The name BuildTool makes me think of things like Make or MSBuild rather 
> > > than the compiler. And in this case we know the compiler is Clang :-) 
> > > Also since this is for capturing the cc1 arguments, it shouldn't really 
> > > matter if it's clang-cl, clang++ or just clang. So it seems unfortunate 
> > > that it has to be piped through all the way like this..
> > > 
> > > Is there some way we could just grab the path to the current clang binary 
> > > during the pdb writing?
> > There's `sys::fs::getMainExecutable(const char *argv0, void *MainExecAddr)` 
> > but that's not guaranteed to work on all platforms, you still need to 
> > provide argv[0] to fallback, see 
> > https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Unix/Path.inc#L232
> > I'm not well versed in Unix distros, can this only rarely occur?
> > Would you prefer that I used that function instead? (it is ok on Windows 
> > though)
> > 
> I think it can occur for example if the compiler is running in a chroot. And 
> probably other cases as well.
> So maybe piping the path through is better. But perhaps we should just call 
> it argv0?
> Or could we get away with putting just "clang" there? Does the tool consuming 
> this info actually need the real path to the compiler or could we rely on it 
> finding it on PATH?
Ok for argv0.
Users can have several versions of a toolchain installed locally, if they 
switch between branches or if working on different games at the same time.
We need to identify the right compiler through its full path, we can't rely on 
PATH.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833



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


[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

2020-06-02 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D79972#2069366 , @cchen wrote:

> In D79972#2069358 , @ABataev wrote:
>
> > In D79972#2069322 , @cchen wrote:
> >
> > > In D79972#2068976 , @ABataev 
> > > wrote:
> > >
> > > > Still: Did you think about implementing it in the compiler instead of 
> > > > the runtime?
> > >
> > >
> > > I'm not sure I understand your question, which part of code are you 
> > > asking?
> > >  The main work compiler needs to do is to send the {offset, count, 
> > > stride} struct to runtime.
> >
> >
> > I mean did you think about calling `__tgt_target_data_update` function in a 
> > loop in the compiler-generated code instead of putting it into the runtime?
>
>
> Oh, I would prefer to call `tgt_target_data_update` once in the compiler and 
> I'm also doing it now.


I was not quite correct. What I mean, is to generate the array with the array 
section as VLA in the compiler, and fill it in the loop generated by the 
compiler for non-contiguous sections but not in the runtime?
Say, we have the code:

  int arr[3][3]
  ...
   #pragma omp update to(arr[1:2][1:2]

In this case, we're going to transfer the next elements:

  000
  0xx
  0xx

In the compiler-generated code we emit something like this:

  void *bptr[];
  void *ptr[];
  int64 sizes[];
  int64 maptypes[];
  for (int i = 0; i < ; ++i) {
bptr[i] = [1+i][1];
ptr[i] = [1+i][1];
sizes[i] = ...;'
maptypes[i] = ...;
  }
  call void @__tgt_target_data_update(i64 -1, i32 , bptr, ptr, sizes, 
maptypes);

With this solution, you won't need to modify the runtime and add a new mapping 
flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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


[PATCH] D80883: [Driver] Add multiclass OptInFlag and OptOutFlag to simplify boolean option definition

2020-06-02 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.

This looks good to me. Might want to open a project around migrating everything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80883



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


[PATCH] D80876: [clang] Default to windows response files when running on windows

2020-06-02 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment.

If we want want to have the default on windows be dependent on mingw vs 
not-mingw then we should do it across the board so it applies to llvm-ar, lld, 
and other tools.  Right now I don't think any of those other tools have any 
special mingw handling.

Would it make sense to expect mingw users to explicitly pass 
`--rsp-quoting=gnu`.  I'm not very familiar with mingw use cases so I don't 
know if that is an unreadable request?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80876



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


[clang] 237be34 - [WebAssembly] Improve macro hygiene in wasm_simd128.h

2020-06-02 Thread Thomas Lively via cfe-commits

Author: Thomas Lively
Date: 2020-06-02T12:55:06-07:00
New Revision: 237be3404b448637ec3b36f8992434193c5bc64c

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

LOG: [WebAssembly] Improve macro hygiene in wasm_simd128.h

Summary:
The shuffle intrinsic macros did not parenthesize usages of their
constant parameters, which could lead to incorrect results due to
operator precedence issues. This patch fixes the problem by adding the
missing paretheses.

Reviewers: aheejin

Subscribers: dschuff, sbc100, jgravelle-google, sunfish, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Headers/wasm_simd128.h

Removed: 




diff  --git a/clang/lib/Headers/wasm_simd128.h 
b/clang/lib/Headers/wasm_simd128.h
index 580aa1e85c1b..258198a7de34 100644
--- a/clang/lib/Headers/wasm_simd128.h
+++ b/clang/lib/Headers/wasm_simd128.h
@@ -1034,24 +1034,24 @@ wasm_f32x4_convert_u32x4(v128_t __a) {
 #define wasm_v16x8_shuffle(__a, __b, __c0, __c1, __c2, __c3, __c4, __c5, __c6, 
\
__c7)   
\
   ((v128_t)__builtin_wasm_shuffle_v8x16(   
\
-  (__i8x16)(__a), (__i8x16)(__b), __c0 * 2, __c0 * 2 + 1, __c1 * 2,
\
-  __c1 * 2 + 1, __c2 * 2, __c2 * 2 + 1, __c3 * 2, __c3 * 2 + 1, __c4 * 2,  
\
-  __c4 * 2 + 1, __c5 * 2, __c5 * 2 + 1, __c6 * 2, __c6 * 2 + 1, __c7 * 2,  
\
-  __c7 * 2 + 1))
+  (__i8x16)(__a), (__i8x16)(__b), (__c0)*2, (__c0)*2 + 1, (__c1)*2,
\
+  (__c1)*2 + 1, (__c2)*2, (__c2)*2 + 1, (__c3)*2, (__c3)*2 + 1, (__c4)*2,  
\
+  (__c4)*2 + 1, (__c5)*2, (__c5)*2 + 1, (__c6)*2, (__c6)*2 + 1, (__c7)*2,  
\
+  (__c7)*2 + 1))
 
 #define wasm_v32x4_shuffle(__a, __b, __c0, __c1, __c2, __c3)   
\
   ((v128_t)__builtin_wasm_shuffle_v8x16(   
\
-  (__i8x16)(__a), (__i8x16)(__b), __c0 * 4, __c0 * 4 + 1, __c0 * 4 + 2,
\
-  __c0 * 4 + 3, __c1 * 4, __c1 * 4 + 1, __c1 * 4 + 2, __c1 * 4 + 3,
\
-  __c2 * 4, __c2 * 4 + 1, __c2 * 4 + 2, __c2 * 4 + 3, __c3 * 4,
\
-  __c3 * 4 + 1, __c3 * 4 + 2, __c3 * 4 + 3))
+  (__i8x16)(__a), (__i8x16)(__b), (__c0)*4, (__c0)*4 + 1, (__c0)*4 + 2,
\
+  (__c0)*4 + 3, (__c1)*4, (__c1)*4 + 1, (__c1)*4 + 2, (__c1)*4 + 3,
\
+  (__c2)*4, (__c2)*4 + 1, (__c2)*4 + 2, (__c2)*4 + 3, (__c3)*4,
\
+  (__c3)*4 + 1, (__c3)*4 + 2, (__c3)*4 + 3))
 
 #define wasm_v64x2_shuffle(__a, __b, __c0, __c1)   
\
   ((v128_t)__builtin_wasm_shuffle_v8x16(   
\
-  (__i8x16)(__a), (__i8x16)(__b), __c0 * 8, __c0 * 8 + 1, __c0 * 8 + 2,
\
-  __c0 * 8 + 3, __c0 * 8 + 4, __c0 * 8 + 5, __c0 * 8 + 6, __c0 * 8 + 7,
\
-  __c1 * 8, __c1 * 8 + 1, __c1 * 8 + 2, __c1 * 8 + 3, __c1 * 8 + 4,
\
-  __c1 * 8 + 5, __c1 * 8 + 6, __c1 * 8 + 7))
+  (__i8x16)(__a), (__i8x16)(__b), (__c0)*8, (__c0)*8 + 1, (__c0)*8 + 2,
\
+  (__c0)*8 + 3, (__c0)*8 + 4, (__c0)*8 + 5, (__c0)*8 + 6, (__c0)*8 + 7,
\
+  (__c1)*8, (__c1)*8 + 1, (__c1)*8 + 2, (__c1)*8 + 3, (__c1)*8 + 4,
\
+  (__c1)*8 + 5, (__c1)*8 + 6, (__c1)*8 + 7))
 
 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_v8x16_swizzle(v128_t __a,
v128_t __b) {



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


[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-06-02 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision.
njames93 added a comment.
This revision is now accepted and ready to land.

LGTM with 2 small nits, but I'd still give a few days see if anyone else wants 
to weight in.




Comment at: 
clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.h:52
+
+  const std::string getMacroName() const { return MacroName; }
+

Return by reference here.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:17
+
+When running this check on a code like this
+

Put a colon on the end of this line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:26
+
+It will be transformed to this
+

Ditto here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80531



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


[PATCH] D77866: [analyzer][CallAndMessage] Add checker options for each bug category

2020-06-02 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

> While this sounds really good for basic use cases I think it quickly becomes 
> unmanageable for power users.

It's strictly better than the current situation because nothing prevents us 
from keeping the existing checker names as a subset of tags (i.e., 
#core/#core.CallAndMessage or checker:core/checker:core.CallAndMessage). We 
don't even need to break backwards compatibility.

> I am not sure whether using the enable/disable order is user-friendly enough.

I guess it's either the order or allowing users to write boolean formulas :|

Also if they're so smart why don't they introduce their own categories so that 
to avoid writing complicated formulas? (checkmate atheists!) (at least that's 
what i expect IDE developers to do on a regular basis; complicated formulas 
don't need to be autogenerated from high-level ordinary-user-facing checkboxes).

> I'd love to see a more detailed RFC about the topic if someone has the 
> time/up to it.

Yup, this would absolutely require a discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77866



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


[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

2020-06-02 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen added a comment.

In D79972#2069358 , @ABataev wrote:

> In D79972#2069322 , @cchen wrote:
>
> > In D79972#2068976 , @ABataev wrote:
> >
> > > Still: Did you think about implementing it in the compiler instead of the 
> > > runtime?
> >
> >
> > I'm not sure I understand your question, which part of code are you asking?
> >  The main work compiler needs to do is to send the {offset, count, stride} 
> > struct to runtime.
>
>
> I mean did you think about calling `__tgt_target_data_update` function in a 
> loop in the compiler-generated code instead of putting it into the runtime?


Oh, I would prefer to call `tgt_target_data_update` once in the compiler and 
I'm also doing it now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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


[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

2020-06-02 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D79972#2069322 , @cchen wrote:

> In D79972#2068976 , @ABataev wrote:
>
> > Still: Did you think about implementing it in the compiler instead of the 
> > runtime?
>
>
> I'm not sure I understand your question, which part of code are you asking?
>  The main work compiler needs to do is to send the {offset, count, stride} 
> struct to runtime.


I mean did you think about calling `__tgt_target_data_update` function in a 
loop in the compiler-generated code instead of putting it into the runtime?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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


[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

2020-06-02 Thread Benson Chu via Phabricator via cfe-commits
pestctrl updated this revision to Diff 267950.
pestctrl added a comment.

Copy pasted error messages


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return ( < ) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ ( <= ) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ ( > ) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ ( >= ) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int (*)[6]' is complete}}
+ ( == ) &&
+ ( != );
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,24 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,18 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: 

[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

2020-06-02 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

In D79744#2050498 , @rjmccall wrote:

> > For the purpose here, only the callee exists. This is essentially a 
> > freestanding function, the entry point to the program.
>
> I'm definitely not going to let you add a new "generic" argument-passing 
> convention and then only implement exactly the parts you need; that's just 
> adding technical debt.


I'm not sure what you mean here. I don't really want or need a totally new 
generic argument passing convention. Not every IR feature is expected to be 
implemented by every backend. For instance, inalloca/preallocated exist solely 
to implement the x86 windows ABI and no other target tries to handle them. This 
is just another flavor of the same fundamental concept of a parameter attribute 
needed for a target specific calling convention.

> 
> 
>> The load-from-constant nature needs to be exposed earlier, so I think this 
>> necessarily involves changing the convention lowering in some way and it's 
>> just a question of what it looks like. To summarize the 2.5 options I've 
>> come up with are
>> 
>> 1. Use constant byval parameters, as this patch does. This requires the 
>> least implementation effort but doesn't exactly fit in with byval as defined.
> 
> I assume you generate a `byval` caller in some later pass, which will then 
> implicitly copy.  Or do you lower `byval` in a nonstandard way in your 
> backend?

No, the caller is only an external driver of some kind. Since the address 
spaces don't match (and the source address space is read only), anything that 
behaves like a stack byval doesn't really make sense from the beginning which 
is why this patch changes the address space. If we were to leave this as a 
stack address space, we would have to add an alloca and insert a memcpy anyway. 
This would still leave an unusable byval argument around a pass could still 
theoretically try to write into.

D79630  adds the lowering that uses this. 
Because reasons (some of which I referenced in the previous comments), we have 
3 different implementations of the ABI. The byval pointer value is simply 
replaced with a new pointer derived from a constant offset from an intrinsic 
call (this is most obvious in the AMDGPULowerKernelArguments IR version)

>> 1. Replace all IR argument uses with loads from a constant offset from an 
>> intrinsic call. This still needs to leave the IR arguments in place because 
>> we do need to know the original argument sizes and offsets, but they would 
>> never have a use (or I would need to invent some other method of tracking 
>> this information)
> 
> I guess passing aggregate arguments using a normal indirect convention has 
> this same tracking problem.  You'd also have to copy on the caller side to 
> maintain semantics, which is probably hard to eliminate, but I would guess 
> `byval` has the same problem?

Isn't using byval the normal indirect convention? Really the only properties I 
want that byval gives me is a pointee size and alignment. The most abstract 
attribute I could probably come up with is a pointee(type) annotation that I 
could use, without the stack copying implications of byval


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

https://reviews.llvm.org/D79744



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


[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

2020-06-02 Thread Benson Chu via Phabricator via cfe-commits
pestctrl updated this revision to Diff 267947.
pestctrl added a comment.

Copy pasted error message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79945

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/complete-incomplete-pointer-relational-c99.c


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -Wc99-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wc11-extensions %s
+
+int incomplete[]; // expected-warning {{tentative array definition assumed to 
have one element}}
+int complete[6];
+
+int test_comparison_between_incomplete_and_complete_pointer() {
+  return ( < ) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ ( <= ) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ ( > ) &&  // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ ( >= ) && // expected-warning {{pointer 
comparisons before C11 need to be between two complete or two incomplete types; 
'int (*)[]' is incomplete and 'int(*)[6]' is complete}}
+ ( == ) &&
+ ( != );
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11563,11 +11563,24 @@
 // C99 6.5.9p2 and C99 6.5.8p2
 if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
RCanPointeeTy.getUnqualifiedType())) {
-  // Valid unless a relational comparison of function pointers
-  if (IsRelational && LCanPointeeTy->isFunctionType()) {
-Diag(Loc, diag::ext_typecheck_ordered_comparison_of_function_pointers)
-  << LHSType << RHSType << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
+  if (IsRelational) {
+// Pointers both need to point to complete or incomplete types
+if (LCanPointeeTy->isIncompleteType() !=
+RCanPointeeTy->isIncompleteType()) {
+  Diag(Loc,
+   getLangOpts().C11
+   ? diag::ext_typecheck_compare_complete_incomplete_pointers
+   : diag::warn_typecheck_compare_complete_incomplete_pointers)
+  << LHS.get()->getSourceRange() << RHS.get()->getSourceRange()
+  << LHSType << RHSType << LCanPointeeTy->isIncompleteType()
+  << RCanPointeeTy->isIncompleteType();
+}
+if (LCanPointeeTy->isFunctionType()) {
+  // Valid unless a relational comparison of function pointers
+  Diag(Loc, 
diag::ext_typecheck_ordered_comparison_of_function_pointers)
+  << LHSType << RHSType << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
+}
   }
 } else if (!IsRelational &&
(LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6443,6 +6443,18 @@
   "ordered comparison between pointer and zero (%0 and %1)">;
 def err_typecheck_three_way_comparison_of_pointer_and_zero : Error<
   "three-way comparison between pointer and zero">;
+def ext_typecheck_compare_complete_incomplete_pointers : Extension<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup;
+def warn_typecheck_compare_complete_incomplete_pointers : Warning<
+  "pointer comparisons before C11 "
+  "need to be between two complete or two incomplete types; "
+  "%0 is %select{|in}2complete and "
+  "%1 is %select{|in}3complete">,
+  InGroup, DefaultIgnore;
 def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn<
   "ordered comparison of function pointers (%0 and %1)">,
   InGroup>;


Index: clang/test/Sema/complete-incomplete-pointer-relational-c99.c
===
--- /dev/null
+++ clang/test/Sema/complete-incomplete-pointer-relational-c99.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wc99-compat %s
+// RUN: %clang_cc1 

[PATCH] D79972: [OpenMP5.0] map item can be non-contiguous for target update

2020-06-02 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen added a comment.

In D79972#2068976 , @ABataev wrote:

> Still: Did you think about implementing it in the compiler instead of the 
> runtime?


I'm not sure I understand your question, which part of code are you asking?
The main work compiler needs to do is to send the {offset, count, stride} 
struct to runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79972



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


[PATCH] D80925: Fix compiler crash when an expression parsed in the tentative parsing and must be claimed in the another evaluation context.

2020-06-02 Thread Alexey Bataev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f7269b6773d (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80925

Files:
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/AST/alignas_maybe_odr_cleanup.cpp


Index: clang/test/AST/alignas_maybe_odr_cleanup.cpp
===
--- /dev/null
+++ clang/test/AST/alignas_maybe_odr_cleanup.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only %s -ast-dump | FileCheck %s
+
+struct FOO {
+  static const int vec_align_bytes = 32;
+  void foo() {
+double a alignas(vec_align_bytes);
+;
+  }
+};
+
+// CHECK: AlignedAttr {{.*}} alignas
+// CHECK: ConstantExpr {{.+}} 'int' Int: 32
+// CHECK: ImplicitCastExpr {{.*}} 'int' 
+// CHECK: DeclRefExpr {{.*}} 'const int' lvalue Var {{.*}} 'vec_align_bytes' 
'const int' non_odr_use_constant
+// CHECK: NullStmt
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1694,7 +1694,8 @@
   }
 
   case Sema::NC_ContextIndependentExpr:
-Tok.setKind(tok::annot_primary_expr);
+Tok.setKind(Actions.isUnevaluatedContext() ? tok::annot_uneval_primary_expr
+   : tok::annot_primary_expr);
 setExprAnnotation(Tok, Classification.getExpression());
 Tok.setAnnotationEndLoc(NameLoc);
 if (SS.isNotEmpty())
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -1275,6 +1275,15 @@
   // this is ambiguous. Typo-correct to type and expression keywords and
   // to types and identifiers, in order to try to recover from errors.
   TentativeParseCCC CCC(Next);
+  // Tentative parsing may not be done in the right evaluation context
+  // for the ultimate expression.  Enter an unevaluated context to prevent
+  // Sema from immediately e.g. treating this lookup as a potential 
ODR-use.
+  // If we generate an expression annotation token and the parser actually
+  // claims it as an expression, we'll transform the expression to a
+  // potentially-evaluated one then.
+  EnterExpressionEvaluationContext Unevaluated(
+  Actions, Sema::ExpressionEvaluationContext::Unevaluated,
+  Sema::ReuseLambdaContextDecl);
   switch (TryAnnotateName()) {
   case ANK_Error:
 return TPResult::Error;
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -998,8 +998,23 @@
 Diag(Tok, diag::warn_cxx98_compat_nullptr);
 return Actions.ActOnCXXNullPtrLiteral(ConsumeToken());
 
+  case tok::annot_uneval_primary_expr:
   case tok::annot_primary_expr:
 Res = getExprAnnotation(Tok);
+if (SavedKind == tok::annot_uneval_primary_expr) {
+  if (Expr *E = Res.get()) {
+if (!E->isTypeDependent() && !E->containsErrors()) {
+  // TransformToPotentiallyEvaluated expects that it will still be in a
+  // (temporary) unevaluated context and then looks through that 
context
+  // to build it in the surrounding context. So we need to push an
+  // unevaluated context to balance things out.
+  EnterExpressionEvaluationContext Unevaluated(
+  Actions, Sema::ExpressionEvaluationContext::Unevaluated,
+  Sema::ReuseLambdaContextDecl);
+  Res = Actions.TransformToPotentiallyEvaluated(Res.get());
+}
+  }
+}
 ConsumeAnnotationToken();
 if (!Res.isInvalid() && Tok.is(tok::less))
   checkPotentialAngleBracket(Res);
Index: clang/include/clang/Basic/TokenKinds.def
===
--- clang/include/clang/Basic/TokenKinds.def
+++ clang/include/clang/Basic/TokenKinds.def
@@ -742,6 +742,9 @@
 ANNOTATION(non_type_dependent)  // annotation for an assumed non-type member of
 // a dependent base class
 ANNOTATION(primary_expr) // annotation for a primary expression
+ANNOTATION(
+uneval_primary_expr) // annotation for a primary expression which should be
+ // transformed to potentially evaluated
 ANNOTATION(decltype) // annotation for a decltype expression,
  // e.g., "decltype(foo.bar())"
 


Index: clang/test/AST/alignas_maybe_odr_cleanup.cpp
===
--- /dev/null
+++ clang/test/AST/alignas_maybe_odr_cleanup.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only %s -ast-dump | FileCheck 

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

2020-06-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:676
+  Value v = vals[vd];
+  if (isUninitialized(v))
+handler.handleConstRefUseOfUninitVariable(vd, getUninitUse(ex, vd, v));

This should use isAlwaysUninit. I fixed it in 
7096e04a6831d4668c39b388ccd166f84de69191

Otherwise there are multiple false positives in stage2 build of llvm-project, 
e.g.

```
if (a < 42)
  var = 1;
if (a < 42)
  const_ref_use(var);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79895



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


[PATCH] D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: olista01.
MaskRay added a comment.

In D80828#2069287 , @nickdesaulniers 
wrote:

> May I please have a non-Googler to review+(accept|reject) the revision?


I guess @olista01 is an inactive account. Changed to @ostannard


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[PATCH] D80590: [WIP][OPENMP] Fix assertion error for using alignas with OpenMP directive

2020-06-02 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

Must be fixed in 2f7269b6773de2750f9cd1417ef5f21cd6cf7a91 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80590



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


[clang] 2f7269b - Fix compiler crash when an expression parsed in the tentative parsing and must be claimed in the another evaluation context.

2020-06-02 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2020-06-02T14:27:33-04:00
New Revision: 2f7269b6773de2750f9cd1417ef5f21cd6cf7a91

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

LOG: Fix compiler crash when an expression parsed in the tentative parsing and 
must be claimed in the another evaluation context.

Summary:
Clang crashes when trying to finish function body. MaybeODRUseExprs is
not empty because of const static data member parsed in outer evaluation
context, upon call for isTypeIdInParens() function. It builds
annot_primary_expr, later parsed in ParseConstantExpression() in
inner constant expression evaluation context.

Reviewers: rjmccall, rsmith

Subscribers: cfe-commits

Tags: #clang

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

Added: 
clang/test/AST/alignas_maybe_odr_cleanup.cpp

Modified: 
clang/include/clang/Basic/TokenKinds.def
clang/lib/Parse/ParseExpr.cpp
clang/lib/Parse/ParseTentative.cpp
clang/lib/Parse/Parser.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TokenKinds.def 
b/clang/include/clang/Basic/TokenKinds.def
index 919c51f0e41d..876f53c84ee0 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -742,6 +742,9 @@ ANNOTATION(non_type_undeclared) // annotation for an 
undeclared identifier that
 ANNOTATION(non_type_dependent)  // annotation for an assumed non-type member of
 // a dependent base class
 ANNOTATION(primary_expr) // annotation for a primary expression
+ANNOTATION(
+uneval_primary_expr) // annotation for a primary expression which should be
+ // transformed to potentially evaluated
 ANNOTATION(decltype) // annotation for a decltype expression,
  // e.g., "decltype(foo.bar())"
 

diff  --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 544fe6123677..1ef352eb5c27 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -998,8 +998,23 @@ ExprResult Parser::ParseCastExpression(CastParseKind 
ParseKind,
 Diag(Tok, diag::warn_cxx98_compat_nullptr);
 return Actions.ActOnCXXNullPtrLiteral(ConsumeToken());
 
+  case tok::annot_uneval_primary_expr:
   case tok::annot_primary_expr:
 Res = getExprAnnotation(Tok);
+if (SavedKind == tok::annot_uneval_primary_expr) {
+  if (Expr *E = Res.get()) {
+if (!E->isTypeDependent() && !E->containsErrors()) {
+  // TransformToPotentiallyEvaluated expects that it will still be in a
+  // (temporary) unevaluated context and then looks through that 
context
+  // to build it in the surrounding context. So we need to push an
+  // unevaluated context to balance things out.
+  EnterExpressionEvaluationContext Unevaluated(
+  Actions, Sema::ExpressionEvaluationContext::Unevaluated,
+  Sema::ReuseLambdaContextDecl);
+  Res = Actions.TransformToPotentiallyEvaluated(Res.get());
+}
+  }
+}
 ConsumeAnnotationToken();
 if (!Res.isInvalid() && Tok.is(tok::less))
   checkPotentialAngleBracket(Res);

diff  --git a/clang/lib/Parse/ParseTentative.cpp 
b/clang/lib/Parse/ParseTentative.cpp
index ff941cfc441d..9179cc91cfe7 100644
--- a/clang/lib/Parse/ParseTentative.cpp
+++ b/clang/lib/Parse/ParseTentative.cpp
@@ -1275,6 +1275,15 @@ Parser::isCXXDeclarationSpecifier(Parser::TPResult 
BracedCastResult,
   // this is ambiguous. Typo-correct to type and expression keywords and
   // to types and identifiers, in order to try to recover from errors.
   TentativeParseCCC CCC(Next);
+  // Tentative parsing may not be done in the right evaluation context
+  // for the ultimate expression.  Enter an unevaluated context to prevent
+  // Sema from immediately e.g. treating this lookup as a potential 
ODR-use.
+  // If we generate an expression annotation token and the parser actually
+  // claims it as an expression, we'll transform the expression to a
+  // potentially-evaluated one then.
+  EnterExpressionEvaluationContext Unevaluated(
+  Actions, Sema::ExpressionEvaluationContext::Unevaluated,
+  Sema::ReuseLambdaContextDecl);
   switch (TryAnnotateName()) {
   case ANK_Error:
 return TPResult::Error;

diff  --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index de809b9ee849..2cea7307b3ac 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -1694,7 +1694,8 @@ Parser::TryAnnotateName(CorrectionCandidateCallback *CCC) 
{
   }
 
   case Sema::NC_ContextIndependentExpr:
-Tok.setKind(tok::annot_primary_expr);
+Tok.setKind(Actions.isUnevaluatedContext() ? tok::annot_uneval_primary_expr
+

[PATCH] D80828: [Clang][A32/T32][Linux] -O1 implies -fomit-frame-pointer

2020-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.

May I please have a non-Googler to review+(accept|reject) the revision?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80828



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


[PATCH] D79744: clang: Add address space to indirect abi info and use it for kernels

2020-06-02 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

In D79744#2047788 , @jdoerfert wrote:

> In D79744#2047482 , @arsenm wrote:
>
> > For the purpose here, only the callee exists. This is essentially a 
> > freestanding function, the entry point to the program. There is no caller 
> > function, and in the future I would like to make a call to amdgpu_kernel an 
> > IR verifier error (technically OpenCL device enqueue is an exception to 
> > this, but we don't treat this as a call. Instead there's a lot of library 
> > magic to invoke the kernel. From the perspective of the callee nothing 
> > changes, since it's still not allowed to modify the incoming argument 
> > buffer or aware it was called this way).
>
>
> Did you consider callback annotation for the device enqueue call? While that 
> might not change anything *now*, I'm expecting interesting optimization 
> opportunities there at some point "soon".


I'm not sure what you mean by this exactly. I'm assuming this means "move 
device enqueue implementation into the backend". I don't know all the details 
of how device enqueue is implemented, but there's so much code in the library 
to support this, I don't think this would end up looking like a normal calling 
convention lowering. It would guess it would end up looking like an IR pass 
that would need a calling convention with this restriction, and then a pass to 
insert the code the library uses now.

> 
> 
>> The load-from-constant nature needs to be exposed earlier, so I think this 
>> necessarily involves changing the convention lowering in some way and it's 
>> just a question of what it looks like. To summarize the 2.5 options I've 
>> come up with are
>> 
>> 1. Use constant byval parameters, as this patch does. This requires the 
>> least implementation effort but doesn't exactly fit in with byval as defined.
> 
> And, as was noted in the `byval` lang ref patch (D79636 
> ), there is a reasonable argument to be made 
> to phase-out `byval` in favor of some explicit copying. If that happens, this 
> solution should not be "the last `byval` user". Also, `byval` arguments could 
> be used as scratchpad by smart optimizations. I somehow don't believe this is 
> a great choice but I can by now see that the others are neither.

I assume this would need replacement with a slew of other attributes to capture 
the type/size/alignment/dereferencability, or a new attribute entirely?


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

https://reviews.llvm.org/D79744



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


[PATCH] D80301: [yaml][clang-tidy] Fix new line YAML serialization

2020-06-02 Thread Matthias Gehre via Phabricator via cfe-commits
mgehre added inline comments.



Comment at: llvm/lib/Support/YAMLTraits.cpp:904
+   std::string ) {
+  Val.clear();
+  size_t CurrentPos = 0;

DmitryPolukhin wrote:
> mgehre wrote:
> > I wonder whether using StringRef::split() would lead to an easier 
> > implementation 
> > (https://llvm.org/doxygen/classllvm_1_1StringRef.html#af0284e4c41c0e09c0bc4767bc77a899d)
> I'm not sure that it will be easier to read or more efficient 
> (`StringRef::split` will require additional vector).
I don't expect the difference in efficiency to be noticable, but the code would 
look like
```
SmallVector Lines;
StringRef(Val).split(Lines, '\n');
bool First = true;
for (StringRef Line: Lines) {
  if (First)
First = false;
  else
Out << '\n';
  Out << Line;
}
```
which I personally find easier to follow than `CurrentPos`, `LineBreakPos` and 
their arithmetic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80301



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


[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

2020-06-02 Thread Adrian McCarthy via Phabricator via cfe-commits
amccarth added a comment.

Does the full path to the tool end up only in the object file, or does this 
make it all the way to the final executable or library?  Does embedding full 
paths affect distributed builds or build reproducibility?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833



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


  1   2   3   >