[clang-tools-extra] 7c2990b - [clangd] Fix data race in GoToInclude.All test

2020-11-18 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2020-11-19T08:47:43+01:00
New Revision: 7c2990b8af6f906a431b14951e831647d9bbb090

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

LOG: [clangd] Fix data race in GoToInclude.All test

Added: 


Modified: 
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 31d2a37f64a7..c3c87bd628bd 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1259,8 +1259,8 @@ TEST(GoToInclude, All) {
   Annotations HeaderAnnotations(HeaderContents);
   FS.Files[FooH] = std::string(HeaderAnnotations.code());
 
-  Server.addDocument(FooH, HeaderAnnotations.code());
-  Server.addDocument(FooCpp, SourceAnnotations.code());
+  runAddDocument(Server, FooH, HeaderAnnotations.code());
+  runAddDocument(Server, FooCpp, SourceAnnotations.code());
 
   // Test include in preamble.
   auto Locations = runLocateSymbolAt(Server, FooCpp, 
SourceAnnotations.point());
@@ -1307,7 +1307,7 @@ TEST(GoToInclude, All) {
   auto FooM = testPath("foo.m");
   FS.Files[FooM] = std::string(ObjC.code());
 
-  Server.addDocument(FooM, ObjC.code());
+  runAddDocument(Server, FooM, ObjC.code());
   Locations = runLocateSymbolAt(Server, FooM, ObjC.point());
   ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error";
   EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range(),



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


[PATCH] D91596: [PowerPC] [Clang] Fix alignment of 128-bit floating types

2020-11-18 Thread Qiu Chaofan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6b1341eb5bb7: [PowerPC] [Clang] Fix alignment of 128-bit 
float types (authored by qiucf).

Changed prior to commit:
  https://reviews.llvm.org/D91596?vs=305692=306315#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91596

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/ppc64le-varargs-f128.c


Index: clang/test/CodeGen/ppc64le-varargs-f128.c
===
--- /dev/null
+++ clang/test/CodeGen/ppc64le-varargs-f128.c
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu -emit-llvm \
+// RUN:   -target-cpu pwr9 -target-feature +float128 -mabi=ieeelongdouble \
+// RUN:   -o - %s | FileCheck %s -check-prefix=IEEE
+// RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu -emit-llvm \
+// RUN:   -target-cpu pwr9 -target-feature +float128 \
+// RUN:   -o - %s | FileCheck %s -check-prefix=IBM
+
+#include 
+
+// IEEE-LABEL: define fp128 @f128(i32 signext %n, ...)
+// IEEE: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
+// IEEE: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %{{[0-9a-zA-Z_.]+}}, 15
+// IEEE: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
+// IEEE: %[[P3:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
+// IEEE: %[[P4:[0-9a-zA-Z_.]+]] = bitcast i8* %[[P3]] to fp128*
+// IEEE: %{{[0-9a-zA-Z_.]+}} = load fp128, fp128* %[[P4]], align 16
+// IEEE: call void @llvm.va_end(i8* %{{[0-9a-zA-Z_.]+}})
+__float128 f128(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  __float128 x = va_arg(ap, __float128);
+  va_end(ap);
+  return x;
+}
+
+// IEEE-LABEL: define fp128 @long_double(i32 signext %n, ...)
+// IEEE: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
+// IEEE: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %{{[0-9a-zA-Z_.]+}}, 15
+// IEEE: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
+// IEEE: %[[P3:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
+// IEEE: %[[P4:[0-9a-zA-Z_.]+]] = bitcast i8* %[[P3]] to fp128*
+// IEEE: %{{[0-9a-zA-Z_.]+}} = load fp128, fp128* %[[P4]], align 16
+// IEEE: call void @llvm.va_end(i8* %{{[0-9a-zA-Z_.]+}})
+
+// IBM-LABEL: define ppc_fp128 @long_double(i32 signext %n, ...)
+// IBM: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
+// IBM: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %{{[0-9a-zA-Z_.]+}}, 15
+// IBM: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
+// IBM: %[[P3:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
+// IBM: %[[P4:[0-9a-zA-Z_.]+]] = bitcast i8* %[[P3]] to ppc_fp128*
+// IBM: %{{[0-9a-zA-Z_.]+}} = load ppc_fp128, ppc_fp128* %[[P4]], align 16
+// IBM: call void @llvm.va_end(i8* %{{[0-9a-zA-Z_.]+}})
+long double long_double(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  long double x = va_arg(ap, long double);
+  va_end(ap);
+  return x;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -5051,6 +5051,11 @@
 return CharUnits::fromQuantity(16);
   } else if (Ty->isVectorType()) {
 return CharUnits::fromQuantity(getContext().getTypeSize(Ty) == 128 ? 16 : 
8);
+  } else if (Ty->isRealFloatingType() && getContext().getTypeSize(Ty) == 128) {
+// IEEE 128-bit floating numbers are also stored in vector registers.
+// And both IEEE quad-precision and IBM extended double (ppc_fp128) should
+// be quad-word aligned.
+return CharUnits::fromQuantity(16);
   }
 
   // For single-element float/vector structs, we consider the whole type


Index: clang/test/CodeGen/ppc64le-varargs-f128.c
===
--- /dev/null
+++ clang/test/CodeGen/ppc64le-varargs-f128.c
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu -emit-llvm \
+// RUN:   -target-cpu pwr9 -target-feature +float128 -mabi=ieeelongdouble \
+// RUN:   -o - %s | FileCheck %s -check-prefix=IEEE
+// RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu -emit-llvm \
+// RUN:   -target-cpu pwr9 -target-feature +float128 \
+// RUN:   -o - %s | FileCheck %s -check-prefix=IBM
+
+#include 
+
+// IEEE-LABEL: define fp128 @f128(i32 signext %n, ...)
+// IEEE: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
+// IEEE: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %{{[0-9a-zA-Z_.]+}}, 15
+// IEEE: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
+// IEEE: %[[P3:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
+// IEEE: %[[P4:[0-9a-zA-Z_.]+]] = bitcast i8* %[[P3]] to fp128*
+// IEEE: %{{[0-9a-zA-Z_.]+}} = load fp128, fp128* %[[P4]], align 16
+// IEEE: call void @llvm.va_end(i8* %{{[0-9a-zA-Z_.]+}})
+__float128 f128(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  __float128 x = va_arg(ap, __float128);
+  va_end(ap);
+  return x;
+}
+
+// IEEE-LABEL: define fp128 @long_double(i32 signext %n, ...)
+// IEEE: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
+// IEEE: 

[clang] 6b1341e - [PowerPC] [Clang] Fix alignment of 128-bit float types

2020-11-18 Thread Qiu Chaofan via cfe-commits

Author: Qiu Chaofan
Date: 2020-11-19T14:22:14+08:00
New Revision: 6b1341eb5bb71a1ce4547165a247b23bb30ef44e

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

LOG: [PowerPC] [Clang] Fix alignment of 128-bit float types

According to ELF v2 ABI, both IEEE 128-bit and IBM extended floating
point variables should be quad-word (16 bytes) aligned. Previously, only
vector types are considered aligned as quad-word on PowerPC.

This patch will fix incorrectness of IEEE 128-bit float argument in
va_arg cases.

Reviewed By: rjmccall

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

Added: 
clang/test/CodeGen/ppc64le-varargs-f128.c

Modified: 
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index a98e4095b074..64240b1bde20 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -5051,6 +5051,11 @@ CharUnits 
PPC64_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
 return CharUnits::fromQuantity(16);
   } else if (Ty->isVectorType()) {
 return CharUnits::fromQuantity(getContext().getTypeSize(Ty) == 128 ? 16 : 
8);
+  } else if (Ty->isRealFloatingType() && getContext().getTypeSize(Ty) == 128) {
+// IEEE 128-bit floating numbers are also stored in vector registers.
+// And both IEEE quad-precision and IBM extended double (ppc_fp128) should
+// be quad-word aligned.
+return CharUnits::fromQuantity(16);
   }
 
   // For single-element float/vector structs, we consider the whole type

diff  --git a/clang/test/CodeGen/ppc64le-varargs-f128.c 
b/clang/test/CodeGen/ppc64le-varargs-f128.c
new file mode 100644
index ..6562fe6f8fe4
--- /dev/null
+++ b/clang/test/CodeGen/ppc64le-varargs-f128.c
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu -emit-llvm \
+// RUN:   -target-cpu pwr9 -target-feature +float128 -mabi=ieeelongdouble \
+// RUN:   -o - %s | FileCheck %s -check-prefix=IEEE
+// RUN: %clang_cc1 -triple powerpc64le-unknown-linux-gnu -emit-llvm \
+// RUN:   -target-cpu pwr9 -target-feature +float128 \
+// RUN:   -o - %s | FileCheck %s -check-prefix=IBM
+
+#include 
+
+// IEEE-LABEL: define fp128 @f128(i32 signext %n, ...)
+// IEEE: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
+// IEEE: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %{{[0-9a-zA-Z_.]+}}, 15
+// IEEE: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
+// IEEE: %[[P3:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
+// IEEE: %[[P4:[0-9a-zA-Z_.]+]] = bitcast i8* %[[P3]] to fp128*
+// IEEE: %{{[0-9a-zA-Z_.]+}} = load fp128, fp128* %[[P4]], align 16
+// IEEE: call void @llvm.va_end(i8* %{{[0-9a-zA-Z_.]+}})
+__float128 f128(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  __float128 x = va_arg(ap, __float128);
+  va_end(ap);
+  return x;
+}
+
+// IEEE-LABEL: define fp128 @long_double(i32 signext %n, ...)
+// IEEE: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
+// IEEE: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %{{[0-9a-zA-Z_.]+}}, 15
+// IEEE: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
+// IEEE: %[[P3:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
+// IEEE: %[[P4:[0-9a-zA-Z_.]+]] = bitcast i8* %[[P3]] to fp128*
+// IEEE: %{{[0-9a-zA-Z_.]+}} = load fp128, fp128* %[[P4]], align 16
+// IEEE: call void @llvm.va_end(i8* %{{[0-9a-zA-Z_.]+}})
+
+// IBM-LABEL: define ppc_fp128 @long_double(i32 signext %n, ...)
+// IBM: call void @llvm.va_start(i8* %{{[0-9a-zA-Z_.]+}})
+// IBM: %[[P1:[0-9a-zA-Z_.]+]] = add i64 %{{[0-9a-zA-Z_.]+}}, 15
+// IBM: %[[P2:[0-9a-zA-Z_.]+]] = and i64 %[[P1]], -16
+// IBM: %[[P3:[0-9a-zA-Z_.]+]] = inttoptr i64 %[[P2]] to i8*
+// IBM: %[[P4:[0-9a-zA-Z_.]+]] = bitcast i8* %[[P3]] to ppc_fp128*
+// IBM: %{{[0-9a-zA-Z_.]+}} = load ppc_fp128, ppc_fp128* %[[P4]], align 16
+// IBM: call void @llvm.va_end(i8* %{{[0-9a-zA-Z_.]+}})
+long double long_double(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  long double x = va_arg(ap, long double);
+  va_end(ap);
+  return x;
+}



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


[PATCH] D91596: [PowerPC] [Clang] Fix alignment of 128-bit floating types

2020-11-18 Thread Qiu Chaofan via Phabricator via cfe-commits
qiucf added a comment.

In D91596#2404514 , @rjmccall wrote:

> I assume this has always been taken care of properly in the backend, so this 
> is just a fix for va_arg treatment?  If so, LGTM.

Yes. That's just fix for `va_arg`. Thanks!


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

https://reviews.llvm.org/D91596

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


[PATCH] D91760: [Driver] Default Generic_GCC aarch64 to use -fasynchronous-unwind-tables

2020-11-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 306312.
MaskRay added a comment.

Drop an unneeded change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91760

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/aarch64-features.c


Index: clang/test/Driver/aarch64-features.c
===
--- clang/test/Driver/aarch64-features.c
+++ clang/test/Driver/aarch64-features.c
@@ -1,6 +1,8 @@
 // RUN: %clang -target aarch64-none-linux-gnu -### %s -fsyntax-only 2>&1 | 
FileCheck %s
 // RUN: %clang -target arm64-none-linux-gnu -### %s -fsyntax-only 2>&1 | 
FileCheck %s
 
+// CHECK: "-munwind-tables"
+
 // The AArch64 PCS states that chars should be unsigned.
 // CHECK: fno-signed-char
 
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2672,7 +2672,13 @@
 }
 
 bool Generic_GCC::IsUnwindTablesDefault(const ArgList ) const {
-  return getArch() == llvm::Triple::x86_64;
+  switch (getArch()) {
+  case llvm::Triple::aarch64:
+  case llvm::Triple::x86_64:
+return true;
+  default:
+return false;
+  }
 }
 
 bool Generic_GCC::isPICDefault() const {


Index: clang/test/Driver/aarch64-features.c
===
--- clang/test/Driver/aarch64-features.c
+++ clang/test/Driver/aarch64-features.c
@@ -1,6 +1,8 @@
 // RUN: %clang -target aarch64-none-linux-gnu -### %s -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target arm64-none-linux-gnu -### %s -fsyntax-only 2>&1 | FileCheck %s
 
+// CHECK: "-munwind-tables"
+
 // The AArch64 PCS states that chars should be unsigned.
 // CHECK: fno-signed-char
 
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2672,7 +2672,13 @@
 }
 
 bool Generic_GCC::IsUnwindTablesDefault(const ArgList ) const {
-  return getArch() == llvm::Triple::x86_64;
+  switch (getArch()) {
+  case llvm::Triple::aarch64:
+  case llvm::Triple::x86_64:
+return true;
+  default:
+return false;
+  }
 }
 
 bool Generic_GCC::isPICDefault() const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64651: [Driver] Support file paths with -ccc-gcc-name

2020-11-18 Thread Alex James via Phabricator via cfe-commits
al3xtjames updated this revision to Diff 306309.

Repository:
  rC Clang

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

https://reviews.llvm.org/D64651

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp


Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -170,7 +170,12 @@
   } else
 GCCName = "gcc";
 
-  const char *Exec = 
Args.MakeArgString(getToolChain().GetProgramPath(GCCName));
+  const char *Exec;
+  if (llvm::sys::fs::exists(GCCName))
+Exec = GCCName;
+  else
+Exec = Args.MakeArgString(getToolChain().GetProgramPath(GCCName));
+
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
  Exec, CmdArgs, Inputs, Output));


Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -170,7 +170,12 @@
   } else
 GCCName = "gcc";
 
-  const char *Exec = Args.MakeArgString(getToolChain().GetProgramPath(GCCName));
+  const char *Exec;
+  if (llvm::sys::fs::exists(GCCName))
+Exec = GCCName;
+  else
+Exec = Args.MakeArgString(getToolChain().GetProgramPath(GCCName));
+
   C.addCommand(std::make_unique(JA, *this,
  ResponseFileSupport::AtFileCurCP(),
  Exec, CmdArgs, Inputs, Output));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D87216: [NewPM] Support --print-before/after in NPM

2020-11-18 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D87216#2356770 , @jamieschmeiser 
wrote:

> The changes are specific to -print-before and -print-after (which is the 
> intended target and this work originated before -print-changed) but could the 
> change be made universal?  I would rather see the existing StringRef passed 
> to all the callbacks changed rather than adding the new StringRef to specific 
> callbacks.  This way all passes benefit and are consistent.  In particular, I 
> would like to see this change applied to -printChanged; this came up in the 
> reviews for that PR.  If you would rather not make it universal, then at 
> least change all of the callbacks to also receive the new StringRef.

I think this change might be a better way, PTAL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

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


[PATCH] D87216: [NewPM] Support --print-before/after in NPM

2020-11-18 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 306307.
aeubanks added a comment.

rebase
now we don't add a new parameter to callbacks, instead let individual 
instrumentations ask PassInstrumentationCallbacks for pass names


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87216

Files:
  llvm/include/llvm/IR/PassInstrumentation.h
  llvm/include/llvm/IR/PrintPasses.h
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/IR/LegacyPassManager.cpp
  llvm/lib/IR/PassInstrumentation.cpp
  llvm/lib/IR/PrintPasses.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/CodeGen/Generic/print-after.ll
  llvm/test/Other/loop-pass-printer.ll
  llvm/test/Other/print-before-after.ll
  llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/IR/BUILD.gn
@@ -59,6 +59,7 @@
 "PassManager.cpp",
 "PassRegistry.cpp",
 "PassTimingInfo.cpp",
+"PrintPasses.cpp",
 "ProfileSummary.cpp",
 "SafepointIRVerifier.cpp",
 "Statepoint.cpp",
Index: llvm/test/Other/print-before-after.ll
===
--- /dev/null
+++ llvm/test/Other/print-before-after.ll
@@ -0,0 +1,35 @@
+; RUN: not --crash opt < %s -disable-output -passes='no-op-module' -print-before=bleh 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: not --crash opt < %s -disable-output -passes='no-op-module' -print-after=bleh 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: opt < %s -disable-output -passes='no-op-module' -print-before=no-op-function 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: opt < %s -disable-output -passes='no-op-module' -print-after=no-op-function 2>&1 | FileCheck %s --check-prefix=NONE --allow-empty
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-before=no-op-module 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-after=no-op-module 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-function' -print-before=no-op-function 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-function' -print-after=no-op-function 2>&1 | FileCheck %s --check-prefix=ONCE
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-before=no-op-function --print-module-scope 2>&1 | FileCheck %s --check-prefix=TWICE
+; RUN: opt < %s -disable-output -passes='no-op-module,no-op-function' -print-after=no-op-function --print-module-scope 2>&1 | FileCheck %s --check-prefix=TWICE
+
+; REQUIRES: asserts
+
+; NONE-NOT: @foo
+; NONE-NOT: @bar
+
+; ONCE: @foo
+; ONCE: @bar
+; ONCE-NOT: @foo
+; ONCE-NOT: @bar
+
+; TWICE: @foo
+; TWICE: @bar
+; TWICE: @foo
+; TWICE: @bar
+; TWICE-NOT: @foo
+; TWICE-NOT: @bar
+
+define void @foo() {
+  ret void
+}
+
+define void @bar() {
+  ret void
+}
Index: llvm/test/Other/loop-pass-printer.ll
===
--- llvm/test/Other/loop-pass-printer.ll
+++ llvm/test/Other/loop-pass-printer.ll
@@ -1,23 +1,23 @@
 ; This test checks -print-after/before on loop passes
 ; Besides of the loop itself it should be dumping loop pre-header and exits.
 ;
-; RUN: opt -enable-new-pm=0 < %s 2>&1 -disable-output \
+; RUN: opt < %s 2>&1 -disable-output \
 ; RUN: 	   -loop-deletion -print-before=loop-deletion \
 ; RUN:	   | FileCheck %s -check-prefix=DEL
 ; RUN: opt < %s 2>&1 -disable-output \
-; RUN: 	   -passes='loop(loop-deletion)' -print-before-all \
+; RUN: 	   -passes='loop(loop-deletion)' -print-before=loop-deletion \
 ; RUN:	   | FileCheck %s -check-prefix=DEL
 ; RUN: opt -enable-new-pm=0 < %s 2>&1 -disable-output \
 ; RUN: 	   -loop-unroll -print-after=loop-unroll -filter-print-funcs=bar \
 ; RUN:	   | FileCheck %s -check-prefix=BAR -check-prefix=BAR-OLD
 ; RUN: opt < %s 2>&1 -disable-output \
-; RUN: 	   -passes='require,loop(loop-unroll-full)' -print-after-all -filter-print-funcs=bar \
+; RUN: 	   -passes='require,loop(loop-unroll-full)' -print-after=loop-unroll-full -filter-print-funcs=bar \
 ; RUN:	   | FileCheck %s -check-prefix=BAR
 ; RUN: opt -enable-new-pm=0 < %s 2>&1 -disable-output \
 ; RUN: 	   -loop-unroll -print-after=loop-unroll -filter-print-funcs=foo -print-module-scope \
 ; RUN:	   | FileCheck %s -check-prefix=FOO-MODULE -check-prefix=FOO-MODULE-OLD
 ; RUN: opt < %s 2>&1 -disable-output \
-; RUN: 	   -passes='require,loop(loop-unroll-full)' -print-after-all -filter-print-funcs=foo -print-module-scope \
+; RUN: 	   -passes='require,loop(loop-unroll-full)' -print-after=loop-unroll-full -filter-print-funcs=foo -print-module-scope \
 ; RUN:	   | FileCheck %s 

[PATCH] D90507: Adding DWARF64 clang flag: -gdwarf64

2020-11-18 Thread Igor Kudrin via Phabricator via cfe-commits
ikudrin added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2145-2146
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Generate DWARF64 debug information.">;
 

ayermolo wrote:
> dblaikie wrote:
> > ayermolo wrote:
> > > dblaikie wrote:
> > > > Does this actually enable debug info emission (the -gdwarf-N versions 
> > > > do cause debug info to be emitted, and set the dwarf version - but, say 
> > > > -ggnu-pubnames does not cause debug info to be emitted if it isn't 
> > > > otherwise requested). Experience has shown it's probably better to have 
> > > > flags like this not enable debug info emission - so they can be used 
> > > > orthogonally (eg: if a project is large, it can add -gdwarf64 to its 
> > > > flags permanently, even though maybe only some build modes, etc, enable 
> > > > debug info (with -g))
> > > > 
> > > > If this doesn't enable debug info emission, the phrasing of the help 
> > > > text might be worth changing somewhat (I wonder how we document 
> > > > -ggnu-pubnames, for instance/comparison/inspiration)
> > > This enables DWARF64 if emission is enabled.
> > > 
> > > From DebugInfo it seems like it's only Elf Format:
> > > 
> > > bool Dwarf64 = Asm->TM.Options.MCOptions.Dwarf64 &&
> > >  DwarfVersion >= 3 &&   // DWARF64 was introduced in 
> > > DWARFv3.
> > >  TT.isArch64Bit() &&// DWARF64 requires 64-bit 
> > > relocations.
> > >  TT.isOSBinFormatELF(); // Support only ELF for now.
> > > This enables DWARF64 if emission is enabled.
> > 
> > Then perhaps the Help Text phrasing should distinguish this behavior from 
> > the behavior of -gdwarf-N which not only sets the version, but enables 
> > emission. (perhaps the phrasing of flags that don't enable emission, like 
> > "-ggnu-pubnames" could provide inspiration for phrasing the help text for 
> > this new flag)
> > 
> > > From DebugInfo it seems like it's only Elf Format:
> > 
> > I'm curious as to why it's ELF only, though - could you explain what the 
> > motivation is there?
> > 
> > (random aside: I still worry about this precedent for -g* flags that 
> > sometimes enable debug info and set a feature, and sometimes they only set 
> > the feature and don't enable debug info - it's a bit of a confusing mess 
> > and I personally rather the -fdebug* flags set a feature but don't enable 
> > emission and -g* flags enable emission, but there's a lack of clear 
> > agreement and there's examples on both sides, so not sure there's much to 
> > be done about that here)
> > This enables DWARF64 if emission is enabled.
> > 
> 
> 
> 
> > From DebugInfo it seems like it's only Elf Format:
> 
> I'm curious as to why it's ELF only, though - could you explain what the 
> motivation is there?
> 

Our customers need DWARF64 for ELF and I do not have much experience with other 
platforms, so I cannot provide adequate support there. Some platform-specific 
tools, like `dsymutil`, have to be fixed to work with DWARF64, but I am 
personally a bit reluctant to do that because I cannot check the result in 
practice. If there will be developers who are willing to support DWARF64 on 
these platforms, they will be able to unlock it and use all the experience we 
collect with ELF, but for now, I prefer to focus on one particular platform 
with practical applications.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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


[PATCH] D90392: [clang-tidy] Omit std::make_unique/make_shared for default initialization.

2020-11-18 Thread Chris Kennelly via Phabricator via cfe-commits
ckennelly updated this revision to Diff 306302.
ckennelly added a comment.

Added option to allow rewrites for default to value initialization


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90392

Files:
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
  clang-tools-extra/docs/clang-tidy/checks/modernize-make-shared.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-make-unique.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-make-shared.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique-default-init.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
@@ -83,49 +83,60 @@
   // CHECK-FIXES: return std::make_unique();
 }
 
+std::unique_ptr getPointerValue() {
+  return std::unique_ptr(new Base());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
+  // CHECK-FIXES: return std::make_unique();
+}
+
 void basic() {
   std::unique_ptr P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr P1 = std::make_unique();
+  std::unique_ptr P2 = std::unique_ptr(new int);
 
   P1.reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique();
+  P2.reset(new int);
 
   P1 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P1 = std::make_unique();
+  P1 = std::unique_ptr(new int);
 
-  // Without parenthesis.
-  std::unique_ptr P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P2 = std::make_unique();
+  // Without parenthesis, default initialization.
+  std::unique_ptr P3 = std::unique_ptr(new int);
 
   P2.reset(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   P2 = std::unique_ptr(new int);
-  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: P2 = std::make_unique();
 
   // With auto.
-  auto P3 = std::unique_ptr(new int());
+  auto P4 = std::unique_ptr(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
-  // CHECK-FIXES: auto P3 = std::make_unique();
+  // CHECK-FIXES: auto P4 = std::make_unique();
+  auto P5 = std::unique_ptr(new int);
 
-  std::unique_ptr P4 = std::unique_ptr((new int));
+  std::unique_ptr P6 = std::unique_ptr((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P4 = std::make_unique();
-  P4.reset((new int));
+  // CHECK-FIXES: std::unique_ptr P6 = std::make_unique();
+  std::unique_ptr P7 = std::unique_ptr((new int));
+
+  P4.reset((new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P4 = std::make_unique();
-  std::unique_ptr P5 = std::unique_ptrnew int;
+  P5.reset((new int));
+
+  std::unique_ptr P8 = std::unique_ptrnew int();
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
-  // CHECK-FIXES: std::unique_ptr P5 = std::make_unique();
-  P5.reset(new int);
+  // CHECK-FIXES: std::unique_ptr P8 = std::make_unique();
+  std::unique_ptr P9 = std::unique_ptrnew int;
+
+  P5.reset(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: P5 = std::make_unique();
+  P6.reset(new int);
 
   {
 // No std.
@@ -133,40 +144,55 @@
 unique_ptr Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use std::make_unique instead
 // CHECK-FIXES: unique_ptr Q = std::make_unique();
+unique_ptr P = unique_ptr(new int);
 
 Q = unique_ptr(new int());
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use std::make_unique instead
 // CHECK-FIXES: Q = std::make_unique();
+
+P = unique_ptr(new int);
   }
 
   std::unique_ptr R(new int());
+  std::unique_ptr S(new int);
 
   // Create the unique_ptr as a parameter to a function.
   int T = g(std::unique_ptr(new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
   // CHECK-FIXES: int T = 

[PATCH] D91760: [Driver] Default Generic_GCC aarch64 to use -fasynchronous-unwind-tables

2020-11-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision.
MaskRay added reviewers: joerg, psmith, resistor.
Herald added subscribers: cfe-commits, danielkiss, mstorsjo, kristof.beyls, 
krytarowski, arichardson, emaste.
Herald added a project: clang.
MaskRay requested review of this revision.

In GCC, `aarch64-*-linux` and `aarch64-*-freebsd` made the switch in 2018
(https://gcc.gnu.org/pipermail/gcc-patches/2018-March/495549.html).
In Clang, FreeBSD/Fuchsia/NetBSD/MinGW aarch64 default to 
-fasynchronous-unwind-tables.

This patch defaults Generic_GCC aarch64 (which affects Linux) to use 
-fasynchronous-unwind-tables.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91760

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/aarch64-features.c


Index: clang/test/Driver/aarch64-features.c
===
--- clang/test/Driver/aarch64-features.c
+++ clang/test/Driver/aarch64-features.c
@@ -1,6 +1,8 @@
 // RUN: %clang -target aarch64-none-linux-gnu -### %s -fsyntax-only 2>&1 | 
FileCheck %s
 // RUN: %clang -target arm64-none-linux-gnu -### %s -fsyntax-only 2>&1 | 
FileCheck %s
 
+// CHECK: "-munwind-tables"
+
 // The AArch64 PCS states that chars should be unsigned.
 // CHECK: fno-signed-char
 
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2672,7 +2672,14 @@
 }
 
 bool Generic_GCC::IsUnwindTablesDefault(const ArgList ) const {
-  return getArch() == llvm::Triple::x86_64;
+  switch (getArch()) {
+  case llvm::Triple::aarch64:
+return !getTriple().isOSNetBSD();
+  case llvm::Triple::x86_64:
+return true;
+  default:
+return false;
+  }
 }
 
 bool Generic_GCC::isPICDefault() const {


Index: clang/test/Driver/aarch64-features.c
===
--- clang/test/Driver/aarch64-features.c
+++ clang/test/Driver/aarch64-features.c
@@ -1,6 +1,8 @@
 // RUN: %clang -target aarch64-none-linux-gnu -### %s -fsyntax-only 2>&1 | FileCheck %s
 // RUN: %clang -target arm64-none-linux-gnu -### %s -fsyntax-only 2>&1 | FileCheck %s
 
+// CHECK: "-munwind-tables"
+
 // The AArch64 PCS states that chars should be unsigned.
 // CHECK: fno-signed-char
 
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2672,7 +2672,14 @@
 }
 
 bool Generic_GCC::IsUnwindTablesDefault(const ArgList ) const {
-  return getArch() == llvm::Triple::x86_64;
+  switch (getArch()) {
+  case llvm::Triple::aarch64:
+return !getTriple().isOSNetBSD();
+  case llvm::Triple::x86_64:
+return true;
+  default:
+return false;
+  }
 }
 
 bool Generic_GCC::isPICDefault() const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D87981: [X86] AMX programming model.

2020-11-18 Thread LuoYuanke via Phabricator via cfe-commits
LuoYuanke added inline comments.



Comment at: llvm/lib/Target/X86/X86TileConfig.cpp:223
+  LLVM_DEBUG(dbgs() << "** TILE REGISTER CONFIGURE**\n"
+<< "** Function: " << mf.getName() << '\n');
+  MF = 

mehdi_amini wrote:
> Is this usual for MachinePasses to do this? In general the pass manager can 
> display the scheduling when debugging
Make sense. I'll remove the debug log.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87981

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


[PATCH] D72184: [BPF] support atomic instructions

2020-11-18 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added inline comments.



Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:783
+  let Inst{47-32} = addr{15-0}; // offset
+  let Inst{11-8} = new;
+  let Inst{7-4} = BPF_CMPXCHG.Value;

ast wrote:
> yonghong-song wrote:
> > jackmanb wrote:
> > > If we go down the route of matching operands with x86 as you have done 
> > > for `XFALU64` and `XCHG`, I think  we should also do it for cmpxchg.
> > > 
> > > IIUC this is `dst = atomic_cmpxchg(*(src + off), r0, new);` 
> > > 
> > > But to do it in a single x86 instruction we need to have only 2 operands 
> > > + the hard-coded r0. `r0 = atomic_xmpxchg(*(dst + off), r0, src);` would 
> > > seem most natural to me.
> > We can do this:
> >   r0 = atomic_xmpxchg(*(dst + off), r0, src);
> I'm confused. Isn't that what that is already?
> r0 = atomic_cmpxchg(*(dst + off), r0, src);
> 
> In "class CMPXCHG":
> let Inst{51-48} = addr{19-16}; // this is 'dst_reg' in bpf_insn.
> let Inst{55-52} = new; // this is 'src_reg' in bpf_insn.
> 
> Same as old xadd and new fetch_add insns.
> What am I missing?
I just fixed to return R0/W0 in the early afternoon. So you looked at old 
comments with new code and that is why it is confusion :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

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


[PATCH] D87981: [X86] AMX programming model.

2020-11-18 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment.

(Drive by comments)




Comment at: llvm/include/llvm/CodeGen/TileShapeInfo.h:27
+
+class ShapeT {
+public:

Can you document the class?



Comment at: llvm/lib/Target/X86/X86PreTileConfig.cpp:30
+
+class X86PreTileConfig : public MachineFunctionPass {
+  // context

Can you document this as well?



Comment at: llvm/lib/Target/X86/X86TileConfig.cpp:34
+
+class X86TileConfig : public MachineFunctionPass {
+  // context

Can you document this pass?



Comment at: llvm/lib/Target/X86/X86TileConfig.cpp:223
+  LLVM_DEBUG(dbgs() << "** TILE REGISTER CONFIGURE**\n"
+<< "** Function: " << mf.getName() << '\n');
+  MF = 

Is this usual for MachinePasses to do this? In general the pass manager can 
display the scheduling when debugging


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87981

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


[PATCH] D91596: [PowerPC] [Clang] Fix alignment of 128-bit floating types

2020-11-18 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

I assume this has always been taken care of properly in the backend, so this is 
just a fix for va_arg treatment?  If so, LGTM.


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

https://reviews.llvm.org/D91596

___
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-11-18 Thread Ten Tzen via Phabricator via cfe-commits
tentzen updated this revision to Diff 306287.
tentzen added a comment.

per review feedback from John McCall and others, this update includes:

- Rename intrinsic eha.scope.begin() with seh.scope.begin().
- Update LangRef.rst with detailed explanation
- Add conditional cleanup test case and move the emission of seh.scope.begin() 
into EHScopeStack::pushCleanup()
- Add sideEntry label support in AST-dumper and de/serialization.

also add tests for Windows ABI temps' ctor and dtor process


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

https://reviews.llvm.org/D80344

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/EHScopeStack.h
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGen/windows-seh-EHa-CppCatchDotDotDot.cpp
  clang/test/CodeGen/windows-seh-EHa-CppCondiTemps.cpp
  clang/test/CodeGen/windows-seh-EHa-CppDtors01.cpp
  clang/test/CodeGen/windows-seh-EHa-TryInFinally.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Verifier.cpp

Index: llvm/lib/IR/Verifier.cpp
===
--- llvm/lib/IR/Verifier.cpp
+++ llvm/lib/IR/Verifier.cpp
@@ -4223,6 +4223,10 @@
   Assert(
   !F->isIntrinsic() || isa(I) ||
   F->getIntrinsicID() == Intrinsic::donothing ||
+  F->getIntrinsicID() == Intrinsic::seh_try_begin ||
+  F->getIntrinsicID() == Intrinsic::seh_try_end ||
+  F->getIntrinsicID() == Intrinsic::seh_scope_begin ||
+  F->getIntrinsicID() == Intrinsic::seh_scope_end ||
   F->getIntrinsicID() == Intrinsic::coro_resume ||
   F->getIntrinsicID() == Intrinsic::coro_destroy ||
   F->getIntrinsicID() == Intrinsic::experimental_patchpoint_void ||
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -2789,6 +2789,10 @@
   llvm_unreachable("Cannot invoke this intrinsic");
 case Intrinsic::donothing:
   // Ignore invokes to @llvm.donothing: jump directly to the next BB.
+case Intrinsic::seh_try_begin:
+case Intrinsic::seh_scope_begin:
+case Intrinsic::seh_try_end:
+case Intrinsic::seh_scope_end:
   break;
 case Intrinsic::experimental_patchpoint_void:
 case Intrinsic::experimental_patchpoint_i64:
@@ -6583,6 +6587,10 @@
   lowerCallToExternalSymbol(I, FunctionName);
 return;
   case Intrinsic::donothing:
+  case Intrinsic::seh_try_begin:
+  case Intrinsic::seh_scope_begin:
+  case Intrinsic::seh_try_end:
+  case Intrinsic::seh_scope_end:
 // ignore
 return;
   case Intrinsic::experimental_stackmap:
Index: llvm/include/llvm/IR/Intrinsics.td
===
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -456,6 +456,16 @@
  [llvm_ptr_ty, llvm_ptr_ty],
  [IntrNoMem]>;
 
+// To mark the beginning/end of a try-scope for Windows SEH -EHa
+//  calls/invokes to these intrinsics are placed to model control flows
+//caused by HW exceptions under option -EHa.
+//  calls/invokes to these intrinsics will be discarded during a codegen pass
+//   after EH tables are generated
+def int_seh_try_begin : Intrinsic<[], [], [IntrReadMem, IntrWriteMem, IntrWillReturn]>;
+def int_seh_try_end : Intrinsic<[], [], [IntrReadMem, IntrWriteMem, IntrWillReturn]>;
+def int_seh_scope_begin : Intrinsic<[], [], [IntrNoMem]>;
+def int_seh_scope_end : Intrinsic<[], [], [IntrNoMem]>;
+
 // Note: we treat stacksave/stackrestore as writemem because we don't otherwise
 // model their dependencies on allocas.
 def int_stacksave : Intrinsic<[llvm_ptr_ty]>,
Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -11530,6 +11530,66 @@
 the escaped allocas are allocated, which would break attempts to use
 '``llvm.localrecover``'.
 
+'``llvm.seh.try.begin``' and '``llvm.seh.try.end``' Intrinsics
+
+
+Syntax:
+"""
+
+::
+
+  declare void 

[PATCH] D91047: Add a call super attribute plugin example

2020-11-18 Thread Yafei Liu via Phabricator via cfe-commits
psionic12 updated this revision to Diff 306283.
psionic12 added a comment.

Simplify the test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91047

Files:
  clang/examples/CMakeLists.txt
  clang/examples/CallSuperAttribute/CMakeLists.txt
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/test/CMakeLists.txt
  clang/test/Frontend/plugin-call-super.cpp

Index: clang/test/Frontend/plugin-call-super.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-call-super.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fsyntax-only -Xclang -verify=callsuper %s
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -DBAD_CALLSUPER -fsyntax-only -Xclang -verify=badcallsuper %s
+// REQUIRES: plugins, examples
+
+// callsuper-no-diagnostics
+struct Base1 {
+  [[clang::call_super]] virtual void Test() {}
+};
+struct Base2 {
+  [[clang::call_super]] virtual void Test() {}
+};
+struct Derive : public Base1, public Base2 {
+#ifndef BAD_CALLSUPER
+  void Test() override;
+#else
+  [[clang::call_super]] virtual void Test() override final;
+  // badcallsuper-warning@16 {{'call_super' attribute marked on a final method}}
+#endif
+};
+void Derive::Test() {
+  Base1::Test();
+#ifndef BAD_CALLSUPER
+  Base2::Test();
+#else
+  // badcallsuper-warning@20 {{virtual function 'Base2::Test' is marked as 'call_super' but this overriding method does not call the base version}}
+  // badcallsuper-note@10 {{function marked 'call_super' here}}
+#endif
+}
+struct Derive2 : public Base1, public Base2 {
+  void Test() override {
+Base1::Test();
+Base2::Test();
+  }
+};
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -91,6 +91,7 @@
   list(APPEND CLANG_TEST_DEPS
 Attribute
 AnnotateFunctions
+CallSuperAttr
 clang-interpreter
 PrintFunctionNames
 )
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- /dev/null
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -0,0 +1,190 @@
+//===- AnnotateFunctions.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Attribute plugin to mark a virtual method as ``call_super``, subclasses must
+// call it in the overridden method.
+//
+// This example shows that attribute plugins combined with ``PluginASTAction``
+// in Clang can do some of the same things which Java Annotations do.
+//
+// Unlike the other attribute plugin examples, this one does not attach an
+// attribute AST node to the declaration AST node. Instead, it keeps a separate
+// list of attributed declarations, which may be faster than using
+// ``Decl::getAttr()`` in some cases. The disadvantage of this approach is
+// that the attribute is not part of the AST, which means that dumping the AST
+// will lose the attribute information, pretty printing the AST won't write the
+// attribute back out to source, and AST matchers will not be able to match
+// against the attribute on the declaration.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Sema/ParsedAttr.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Sema/SemaDiagnostic.h"
+#include "llvm/ADT/SmallPtrSet.h"
+using namespace clang;
+
+namespace {
+// Cached methods which are marked as 'call_super'.
+llvm::SmallPtrSet MarkedMethods;
+bool isMarkedAsCallSuper(const CXXMethodDecl *D) {
+  // Uses this way to avoid add an annotation attr to the AST.
+  return MarkedMethods.contains(D);
+}
+
+class MethodUsageVisitor : public RecursiveASTVisitor {
+public:
+  bool IsOverriddenUsed = false;
+  explicit MethodUsageVisitor(
+  llvm::SmallPtrSet )
+  : MustCalledMethods(MustCalledMethods) {}
+  bool VisitCallExpr(CallExpr *CallExpr) {
+const CXXMethodDecl *Callee = nullptr;
+for (const auto  : MustCalledMethods) {
+  if (CallExpr->getCalleeDecl() == MustCalled) {
+// Super is called.
+// Notice that we cannot do delete or insert in the iteration
+// when using SmallPtrSet.
+Callee = MustCalled;
+  }
+}
+if (Callee)
+  MustCalledMethods.erase(Callee);
+
+return true;
+  }
+
+private:
+  llvm::SmallPtrSet 
+};
+
+class CallSuperVisitor : public RecursiveASTVisitor {
+public:

[PATCH] D91756: [CSSPGO] Pseudo probes for function calls.

2020-11-18 Thread Hongtao Yu via Phabricator via cfe-commits
hoy updated this revision to Diff 306280.
hoy edited the summary of this revision.
hoy added a comment.

Updating D91756 : [CSSPGO] Pseudo probes for 
function calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91756

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/include/llvm/IR/PseudoProbe.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/IPO/SampleProfileProbe.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/PseudoProbeInserter.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
  llvm/test/Transforms/SampleProfile/emit-pseudo-probe.ll

Index: llvm/test/Transforms/SampleProfile/emit-pseudo-probe.ll
===
--- llvm/test/Transforms/SampleProfile/emit-pseudo-probe.ll
+++ llvm/test/Transforms/SampleProfile/emit-pseudo-probe.ll
@@ -1,6 +1,6 @@
 ; RUN: opt < %s -passes=pseudo-probe -function-sections -S -o %t
 ; RUN: FileCheck %s < %t --check-prefix=CHECK-IL
-; RUN: llc %t -stop-after=instruction-select -o - | FileCheck %s --check-prefix=CHECK-MIR
+; RUN: llc %t -pseudo-probe-for-profiling -stop-after=pseudo-probe-inserter -o - | FileCheck %s --check-prefix=CHECK-MIR
 ;
 ;; Check the generation of pseudoprobe intrinsic call.
 
@@ -28,9 +28,35 @@
   ret void, !dbg !12
 }
 
+declare void @bar(i32 %x) 
+
+define internal void @foo2(void (i32)* %f) !dbg !4 {
+entry:
+; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID2:]], i64 1)
+; CHECK-MIR: PSEUDO_PROBE [[#GUID2:]], 1, 0
+; Check pseudo_probe metadata attached to the indirect call instruction.
+; CHECK-IL: call void %f(i32 1), !dbg ![[#PROBE0:]]
+; CHECK-MIR: PSEUDO_PROBE [[#GUID2]], 2, 1
+  call void %f(i32 1), !dbg !13
+; Check pseudo_probe metadata attached to the direct call instruction.
+; CHECK-IL: call void @bar(i32 1), !dbg ![[#PROBE1:]]
+; CHECK-MIR: PSEUDO_PROBE	[[#GUID2]], 3, 2
+  call void @bar(i32 1)
+  ret void
+}
+
 ; CHECK-IL: ![[#FOO:]] = distinct !DISubprogram(name: "foo"
 ; CHECK-IL: ![[#FAKELINE]] = !DILocation(line: 0, scope: ![[#FOO]])
 ; CHECK-IL: ![[#REALLINE]] = !DILocation(line: 2, scope: ![[#FOO]])
+; CHECK-IL: ![[#PROBE0]] = !DILocation(line: 2, column: 20, scope: ![[#SCOPE0:]])
+;; A discriminator of 67108887 which is 0x417 in hexdecimal, stands for a direct call probe
+;; with an index of 2.
+; CHECK-IL: ![[#SCOPE0]] = !DILexicalBlockFile(scope: ![[#]], file: ![[#]], discriminator: 67108887)
+; CHECK-IL: ![[#PROBE1]] = !DILocation(line: 0, scope: ![[#SCOPE1:]])
+;; A discriminator of 134217759 which is 0x81f in hexdecimal, stands for a direct call probe
+;; with an index of 3.
+; CHECK-IL: ![[#SCOPE1]] = !DILexicalBlockFile(scope: ![[#]], file: ![[#]], discriminator: 134217759)
+
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!9, !10}
@@ -39,10 +65,12 @@
 !1 = !DIFile(filename: "test.c", directory: "")
 !2 = !{}
 !3 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !5, unit: !0, retainedNodes: !2)
+!4 = distinct !DISubprogram(name: "foo2", scope: !1, file: !1, line: 2, type: !5, unit: !0, retainedNodes: !2)
 !5 = !DISubroutineType(types: !6)
 !6 = !{!7}
 !7 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
 !9 = !{i32 2, !"Dwarf Version", i32 4}
 !10 = !{i32 2, !"Debug Info Version", i32 3}
 !11 = !{!"clang version 3.9.0"}
-!12 = !DILocation(line: 2, scope: !3)
\ No newline at end of file
+!12 = !DILocation(line: 2, scope: !3)
+!13 = !DILocation(line: 2, column: 20, scope: !4)
Index: llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
===
--- llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
+++ llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
@@ -37,8 +37,10 @@
 
 SampleProfileProber::SampleProfileProber(Function ) : F() {
   BlockProbeIds.clear();
+  CallProbeIds.clear();
   LastProbeId = (uint32_t)PseudoProbeReservedId::Last;
   computeProbeIdForBlocks();
+  computeProbeIdForCallsites();
 }
 
 void SampleProfileProber::computeProbeIdForBlocks() {
@@ -47,11 +49,28 @@
   }
 }
 
+void SampleProfileProber::computeProbeIdForCallsites() {
+  for (auto  : *F) {
+for (auto  : BB) {
+  if (!isa(I))
+continue;
+  if (isa())
+continue;
+  CallProbeIds[] = ++LastProbeId;
+}
+  }
+}
+
 uint32_t SampleProfileProber::getBlockId(const BasicBlock *BB) const {
   auto I = BlockProbeIds.find(const_cast(BB));
   return I == BlockProbeIds.end() ? 0 : I->second;
 }
 
+uint32_t SampleProfileProber::getCallsiteId(const Instruction *call) const {
+  auto iter = 

[PATCH] D91015: [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.

2020-11-18 Thread Chris Kennelly via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG25f5406f0875: [clang-tidy] Extend 
bugprone-string-constructor-check to std::string_view. (authored by ckennelly).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91015

Files:
  clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
  clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp
@@ -14,6 +14,15 @@
 };
 typedef basic_string string;
 typedef basic_string wstring;
+
+template >
+struct basic_string_view {
+  basic_string_view();
+  basic_string_view(const C *, unsigned int size);
+  basic_string_view(const C *);
+};
+typedef basic_string_view string_view;
+typedef basic_string_view wstring_view;
 }
 
 const char* kText = "";
@@ -52,11 +61,35 @@
   // CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructing string from nullptr is undefined behaviour
 }
 
+void TestView() {
+  std::string_view q0("test", 0);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructor creating an empty string
+  std::string_view q1(kText, -4);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: negative value used as length parameter
+  std::string_view q2("test", 200);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: length is bigger than string literal size
+  std::string_view q3(kText, 200);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: length is bigger than string literal size
+  std::string_view q4(kText2, 200);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: length is bigger than string literal size
+  std::string_view q5(kText3, 0x100);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: suspicious large length parameter
+  std::string_view q6(nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: constructing string from nullptr is undefined behaviour
+  std::string_view q7 = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: constructing string from nullptr is undefined behaviour
+}
+
 std::string StringFromZero() {
   return 0;
   // CHECK-MESSAGES: [[@LINE-1]]:10: warning: constructing string from nullptr is undefined behaviour
 }
 
+std::string_view StringViewFromZero() {
+  return 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: constructing string from nullptr is undefined behaviour
+}
+
 void Valid() {
   std::string empty();
   std::string str(4, 'x');
@@ -64,6 +97,11 @@
   std::string s1("test", 4);
   std::string s2("test", 3);
   std::string s3("test");
+
+  std::string_view emptyv();
+  std::string_view sv1("test", 4);
+  std::string_view sv2("test", 3);
+  std::string_view sv3("test");
 }
 
 namespace instantiation_dependent_exprs {
@@ -71,5 +109,6 @@
 struct S {
   bool x;
   std::string f() { return x ? "a" : "b"; }
+  std::string_view g() { return x ? "a" : "b"; }
 };
 }
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
@@ -21,6 +21,7 @@
 .. code-block:: c++
 
   std::string("test", 200);   // Will include random characters after "test".
+  std::string_view("test", 200);
 
 Creating an empty string from constructors with parameters is considered
 suspicious. The programmer should use the empty constructor instead.
@@ -30,6 +31,7 @@
 .. code-block:: c++
 
   std::string("test", 0);   // Creation of an empty string.
+  std::string_view("test", 0);
 
 Options
 ---
@@ -42,3 +44,12 @@
 .. option::  LargeLengthThreshold
 
An integer specifying the large length threshold. Default is `0x80`.
+
+.. option:: StringNames
+
+Default is `::std::basic_string;::std::basic_string_view`.
+
+Semicolon-delimited list of class names to apply this check to.
+By default `::std::basic_string` applies to ``std::string`` and
+``std::wstring``. Set to e.g. `::std::basic_string;llvm::StringRef;QString`
+to perform this check on custom classes.
Index: clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
@@ -32,6 +32,7 @@
 private:
   const bool WarnOnLargeLength;
   const unsigned int LargeLengthThreshold;
+  std::vector StringNames;
 };
 
 } // 

[clang-tools-extra] 25f5406 - [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.

2020-11-18 Thread Chris Kennelly via cfe-commits

Author: Chris Kennelly
Date: 2020-11-18T21:16:03-05:00
New Revision: 25f5406f087579d43ca9a82dee7f3e76f0691bad

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

LOG: [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.

This allows for matching the constructors std::string has in common with
std::string_view.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-string-constructor.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
index 96d93a1d0413..6b23f7cd4a9c 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "StringConstructorCheck.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Tooling/FixIt.h"
@@ -21,17 +22,36 @@ namespace {
 AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
   return Node.getValue().getZExtValue() > N;
 }
+
+const char DefaultStringNames[] =
+"::std::basic_string;::std::basic_string_view";
+
+static std::vector
+removeNamespaces(const std::vector ) {
+  std::vector Result;
+  Result.reserve(Names.size());
+  for (StringRef Name : Names) {
+std::string::size_type ColonPos = Name.rfind(':');
+Result.push_back(
+Name.substr(ColonPos == std::string::npos ? 0 : ColonPos + 1));
+  }
+  return Result;
+}
+
 } // namespace
 
 StringConstructorCheck::StringConstructorCheck(StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   WarnOnLargeLength(Options.get("WarnOnLargeLength", true)),
-  LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x80)) {}
+  LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x80)),
+  StringNames(utils::options::parseStringList(
+  Options.get("StringNames", DefaultStringNames))) {}
 
 void StringConstructorCheck::storeOptions(ClangTidyOptions::OptionMap ) {
   Options.store(Opts, "WarnOnLargeLength", WarnOnLargeLength);
   Options.store(Opts, "LargeLengthThreshold", LargeLengthThreshold);
+  Options.store(Opts, "StringNames", DefaultStringNames);
 }
 
 void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
@@ -80,7 +100,8 @@ void StringConstructorCheck::registerMatchers(MatchFinder 
*Finder) {
   // parameters. [i.e. string (const char* s, size_t n);]
   Finder->addMatcher(
   cxxConstructExpr(
-  hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
+  hasDeclaration(cxxConstructorDecl(ofClass(
+  cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)),
   hasArgument(0, hasType(CharPtrType)),
   hasArgument(1, hasType(isInteger())),
   anyOf(
@@ -100,11 +121,17 @@ void StringConstructorCheck::registerMatchers(MatchFinder 
*Finder) {
   // Check the literal string constructor with char pointer.
   // [i.e. string (const char* s);]
   Finder->addMatcher(
-traverse(TK_AsIs,
-  cxxConstructExpr(hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
-   hasArgument(0, expr().bind("from-ptr")),
-   hasArgument(1, unless(hasType(isInteger()
-  .bind("constructor")),
+  traverse(TK_AsIs,
+   cxxConstructExpr(
+   hasDeclaration(cxxConstructorDecl(ofClass(cxxRecordDecl(
+   hasAnyName(removeNamespaces(StringNames)),
+   hasArgument(0, expr().bind("from-ptr")),
+   // do not match std::string(ptr, int)
+   // match std::string(ptr, alloc)
+   // match std::string(ptr)
+   anyOf(hasArgument(1, unless(hasType(isInteger(,
+ argumentCountIs(1)))
+   .bind("constructor")),
   this);
 }
 

diff  --git a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h 
b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
index 687f3b106fb4..50338f8abead 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.h
@@ -32,6 +32,7 @@ class 

[PATCH] D90275: [clang][IR] Add support for leaf attribute

2020-11-18 Thread Gulfem Savrun Yeniceri via Phabricator via cfe-commits
gulfem added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1435
+  let Spellings = [GCC<"leaf">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];

gulfem wrote:
> aaron.ballman wrote:
> > gulfem wrote:
> > > aaron.ballman wrote:
> > > > gulfem wrote:
> > > > > aaron.ballman wrote:
> > > > > > gulfem wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > gulfem wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > gulfem wrote:
> > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > Should this attribute also be supported on things like 
> > > > > > > > > > > > ObjC method decls or other function-like interfaces?
> > > > > > > > > > > Do I need to do anything else to support this attribute 
> > > > > > > > > > > in Objective-C as well?
> > > > > > > > > > > I think we should support it in all the C languages 
> > > > > > > > > > > family.
> > > > > > > > > > >I think we should support it in all the C languages family.
> > > > > > > > > > 
> > > > > > > > > > That's already happening automatically -- there's a C and 
> > > > > > > > > > C++ spelling available for it and the attribute doesn't 
> > > > > > > > > > specify that it requires a particular language mode or 
> > > > > > > > > > target.
> > > > > > > > > > 
> > > > > > > > > > > Do I need to do anything else to support this attribute 
> > > > > > > > > > > in Objective-C as well?
> > > > > > > > > > You can add multiple subjects to the list here, so you can 
> > > > > > > > > > have this apply to `Function, ObjCMethod` for both of 
> > > > > > > > > > those. Another one to consider is whether this attribute 
> > > > > > > > > > can be written on a block declaration (like a lambda, but 
> > > > > > > > > > with different syntax). Beyond that, it's mostly just 
> > > > > > > > > > documentation, devising the test cases to ensure the ObjC 
> > > > > > > > > > functionality behaves as expected, possibly some codegen 
> > > > > > > > > > changes, etc.
> > > > > > > > > AFAIK, users can specify function attributes in lambda 
> > > > > > > > > expressions.
> > > > > > > > > Lambda functions can only be accessed/called by the functions 
> > > > > > > > > in the same translation unit, right?
> > > > > > > > > Leaf attribute does not have any effect on the functions that 
> > > > > > > > > are defined in the same translation unit.
> > > > > > > > > For this reason, I'm thinking that leaf attribute would not 
> > > > > > > > > have any effect if they are used in lambda expressions.
> > > > > > > > > Do you agree with me?
> > > > > > > > > AFAIK, users can specify function attributes in lambda 
> > > > > > > > > expressions.
> > > > > > > > 
> > > > > > > > I always forget that you can do that for declaration attributes 
> > > > > > > > using GNU-style syntax...
> > > > > > > > 
> > > > > > > > > Lambda functions can only be accessed/called by the functions 
> > > > > > > > > in the same translation unit, right?
> > > > > > > > 
> > > > > > > > Not necessarily, you could pass one across TU boundaries like a 
> > > > > > > > function pointer, for instance. e.g.,
> > > > > > > > ```
> > > > > > > > // TU1.cpp
> > > > > > > > void foo() {
> > > > > > > >   auto l = []() { ... };
> > > > > > > >   bar(l);
> > > > > > > > }
> > > > > > > > 
> > > > > > > > // TU2.cpp
> > > > > > > > void bar(auto func) {
> > > > > > > >   func();
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > Not necessarily, you could pass one across TU boundaries like a 
> > > > > > > > function pointer, for instance. e.g.,
> > > > > > > As I mentioned before, leaf attribute is specifically intended 
> > > > > > > for library functions and I think all the existing usage of leaf 
> > > > > > > attribute is in the library function declarations. For this 
> > > > > > > reason, I think we do not need to support them for lambdas. Is 
> > > > > > > that reasonable?
> > > > > > > 
> > > > > > > For this reason, I think we do not need to support them for 
> > > > > > > lambdas. Is that reasonable?
> > > > > > 
> > > > > > Is this considered a library function?
> > > > > > 
> > > > > > ```
> > > > > > struct S {
> > > > > >   void f(); // Is this a library function?
> > > > > >   void operator()(); // How about this?
> > > > > > };
> > > > > > ```
> > > > > > If the answer is "no", then I think we only need to support 
> > > > > > `FunctionDecl` and nothing else (not even `ObjCMethodDecl`, which 
> > > > > > is like a member function for ObjC). If the answer is "yes", then 
> > > > > > it's not clear to me whether lambdas should or should not be 
> > > > > > supported given that the attribute on the lambda expression is 
> > > > > > attached to the function call operator for the lambda declaration.
> > > > > > If the answer is "no", then I think we only need to support 
> > > > > > `FunctionDecl` and nothing else (not even `ObjCMethodDecl`, which 
> > > > > > is like a member function for 

[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

2020-11-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:150
+  const SanitizerArgs  = getToolChain().getSanitizerArgs();
+  if (LINKER_SUPPORTS_Z_RELAX_TRANSTLS &&
+  getToolChain().getTriple().getArch() == llvm::Triple::x86_64 &&

MaskRay wrote:
> Can the configure-time variable `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` be 
> replaced by a version check on the triple suffix?
> 
> You can see some tests where `x86_64-unknown-freebsd11` or 
> `x86_64-unknown-freebsd12` is used. The idea is that the driver continuously 
> bumps the default version.
.. and the code base should not be littered with -D macros from configure-time 
variables here and there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

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


[PATCH] D91605: [sanitizers] Implement GetTls on Solaris

2020-11-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:150
+  const SanitizerArgs  = getToolChain().getSanitizerArgs();
+  if (LINKER_SUPPORTS_Z_RELAX_TRANSTLS &&
+  getToolChain().getTriple().getArch() == llvm::Triple::x86_64 &&

Can the configure-time variable `LINKER_SUPPORTS_Z_RELAX_TRANSTLS` be replaced 
by a version check on the triple suffix?

You can see some tests where `x86_64-unknown-freebsd11` or 
`x86_64-unknown-freebsd12` is used. The idea is that the driver continuously 
bumps the default version.



Comment at: clang/tools/driver/CMakeLists.txt:123
+
+check_linker_flag("-Wl,-z,relax=transtls" LINKER_SUPPORTS_Z_RELAX_TRANSTLS)

GNU ld reports a warning instead of an error when an unknown `-z` is seen. The 
warning remains a warning even with `--fatal-warnings`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91605

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


[PATCH] D91756: [CSSPGO] Pseudo probes for function calls.

2020-11-18 Thread Hongtao Yu via Phabricator via cfe-commits
hoy created this revision.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, wenlei, 
pengfei, hiraditya, mgorny.
Herald added projects: clang, LLVM.
hoy requested review of this revision.

An indirect call site needs to be probed for its potential call targets. With 
CSSPGO a direct call also needs a probe so that a calling context can be 
represented by a stack of callsite probes. Unlike pseudo probes for basic 
blocks that are in form of standalone intrinsic call instructions, pseudo 
probes for callsites have to be attached to the call instruction, thus a 
separate instruction would not work.

One possible way of attaching a probe to a call instruction is to use a special 
metadata that carries information about the probe. The special metadata will 
have to make its way through the optimization pipeline down to object emission. 
This requires additional efforts to maintain the metadata in various places. 
Given that the !dbg metadata is a first-class metadata and has all essential 
support in place , leveraging the !dbg metadata as a channel to encode pseudo 
probe information is handy.

With the requirement of not inflating !dbg metadata that is allocated for 
almost every instruction, we found that the 32-bit DWARF discriminator field 
which mainly serves AutoFDO can be reused for pseudo probes. DWARF 
discriminators distinguish identical source locations between instructions and 
with pseudo probes such support is not required. In this change we are using 
the discriminator field to encode the ID and type of a callsite probe and the 
encoded value will be unpacked and consumed right before object emission.

To avoid collision with the baseline AutoFDO in various places that handles 
dwarf discriminators where a check against  the `-pseudo-probe-for-profiling` 
switch is not available, a special encoding scheme is used to tell apart a 
pseudo probe discriminator from a regular discriminator. For the regular 
discriminator, if all lowest 3 bits are non-zero, it means the discriminator is 
basically empty and all higher 29 bits can be reversed for pseudo probe use.

Callsite pseudo probes are inserted in `SampleProfileProbePass` and a 
target-independent MIR pass `PseudoProbeInserter` is added to unpack the probe 
ID/type from `!dbg`.

Note that with this work the switch -debug-info-for-profiling will not work 
with -pseudo-probe-for-profiling anymore. They cannot be used at the same time.

Test Plan:


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91756

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/include/llvm/IR/PseudoProbe.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/include/llvm/Transforms/IPO/SampleProfileProbe.h
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/PseudoProbeInserter.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
  llvm/test/Transforms/SampleProfile/emit-pseudo-probe.ll

Index: llvm/test/Transforms/SampleProfile/emit-pseudo-probe.ll
===
--- llvm/test/Transforms/SampleProfile/emit-pseudo-probe.ll
+++ llvm/test/Transforms/SampleProfile/emit-pseudo-probe.ll
@@ -1,6 +1,6 @@
 ; RUN: opt < %s -passes=pseudo-probe -function-sections -S -o %t
 ; RUN: FileCheck %s < %t --check-prefix=CHECK-IL
-; RUN: llc %t -stop-after=instruction-select -o - | FileCheck %s --check-prefix=CHECK-MIR
+; RUN: llc %t -pseudo-probe-for-profiling -stop-after=pseudo-probe-inserter -o - | FileCheck %s --check-prefix=CHECK-MIR
 ;
 ;; Check the generation of pseudoprobe intrinsic call.
 
@@ -28,9 +28,35 @@
   ret void, !dbg !12
 }
 
+declare void @bar(i32 %x) 
+
+define internal void @foo2(void (i32)* %f) !dbg !4 {
+entry:
+; CHECK-IL: call void @llvm.pseudoprobe(i64 [[#GUID2:]], i64 1)
+; CHECK-MIR: PSEUDO_PROBE [[#GUID2:]], 1, 0
+; Check pseudo_probe metadata attached to the indirect call instruction.
+; CHECK-IL: call void %f(i32 1), !dbg ![[#PROBE0:]]
+; CHECK-MIR: PSEUDO_PROBE [[#GUID2]], 2, 1
+  call void %f(i32 1), !dbg !13
+; Check pseudo_probe metadata attached to the direct call instruction.
+; CHECK-IL: call void @bar(i32 1), !dbg ![[#PROBE1:]]
+; CHECK-MIR: PSEUDO_PROBE	[[#GUID2]], 3, 2
+  call void @bar(i32 1)
+  ret void
+}
+
 ; CHECK-IL: ![[#FOO:]] = distinct !DISubprogram(name: "foo"
 ; CHECK-IL: ![[#FAKELINE]] = !DILocation(line: 0, scope: ![[#FOO]])
 ; CHECK-IL: ![[#REALLINE]] = !DILocation(line: 2, scope: ![[#FOO]])
+; CHECK-IL: ![[#PROBE0]] = !DILocation(line: 2, column: 20, scope: ![[#SCOPE0:]])
+;; A discriminator of 67108887 which is 0x417 in hexdecimal, stands for a direct call probe
+;; with an index of 2.
+; CHECK-IL: ![[#SCOPE0]] = 

[PATCH] D91279: [PowerPC] DForm instructions should be preferred when using zero register

2020-11-18 Thread ChenZheng via Phabricator via cfe-commits
shchenz added a comment.

> After a discussion with the group I would like to correct what I said in the 
> previous post.
> There already is a plan to do this in ISel in a different patch. The reason 
> we also want to do this optimization here is to try to catch situations where 
> this pattern is not known in ISel and only appears after other optimizations 
> later on. Ideally we do not want to have any situations where the XForm 
> exists in the final binary and having this final check in the PreEmitPeephole 
> should ensure that. Basically, we also want to do this check here to find 
> anything that ISel may have missed.

Good to know we have a plan to fix such kind of issue in ISEL. For the patterns 
generated after ISEL, I also think adding them in `convertToImmediateForm` is 
better. That function is called pre and post RA. It handles several patterns 
there, maybe we just need to add a new function like 
`transformZeroInputXformToImmForm` in that function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91279

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


[PATCH] D91279: [PowerPC] DForm instructions should be preferred when using zero register

2020-11-18 Thread Qing Shan Zhang via Phabricator via cfe-commits
steven.zhang added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:418
+  // should prefer D-form if LXVX / STXVX uses a ZERO or ZERO8
+  if (MI.getOpcode() == PPC::LXVX || MI.getOpcode() == PPC::STXVX) {
+LLVM_DEBUG(dbgs() << "Replacing LXVX/STXVX REG, 0 with LXV/STX\n");

Some comments for current implementation.
1. Can we do this inside TII->convertToImmediateForm()? Notice that we have 
statistic data for such convert. Though it now handles x-load + addi -> d-load, 
it could be extended to handle x-load, 0 -> d-load.
2. I am not sure why just handle the lxvx. We have quite a lot x-form. 
ImmInstrInfo contains such information and can we query it to do the 
transformation instead of enum the opcode?



Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:421
+// if it's Frame index we should not apply the transformation
+if (!MI.getOperand(1).isFI() &&
+(MI.getOperand(1).getReg() == PPC::ZERO ||

check isReg()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91279

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


[PATCH] D90507: Adding DWARF64 clang flag: -gdwarf64

2020-11-18 Thread Alexander Yermolovich via Phabricator via cfe-commits
ayermolo updated this revision to Diff 306263.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -375,3 +375,16 @@
 // RUN:| FileCheck -check-prefix=NO_DEBUG_UNUSED_TYPES %s
 // NO_DEBUG_UNUSED_TYPES: 
"-debug-info-kind={{limited|line-tables-only|standalone}}"
 // NO_DEBUG_UNUSED_TYPES-NOT: "-debug-info-kind=unused-types"
+//
+// RUN: %clang -### -gdwarf-5 -gdwarf64 -target x86_64-linux-gnu %s 2>&1 | 
FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 -target x86_64-linux-gnu %s 2>&1 | 
FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-3 -gdwarf64 -target x86_64-linux-gnu %s 2>&1 | 
FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-2 -gdwarf64 -target x86_64-linux-gnu %s 2>&1 | 
FileCheck -check-prefix=GDWARF64_OFF %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 -target x86_64-linux-gnu -target 
x86_64-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 -target i386-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_32ARCH %s
+//
+// GDWARF64_ON:  "-gdwarf64"
+// GDWARF64_OFF:  error: invalid argument '-gdwarf64' only allowed with 
'-gdwarf-5, -gdwarf-4, -gdwarf-3'
+// GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 
bit architecture'
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1005,6 +1005,7 @@
   Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = !Args.hasArg(OPT_fno_verbose_asm);
+  Opts.Dwarf64 = Args.hasArg(OPT_gdwarf64);
   Opts.PreserveAsmComments = !Args.hasArg(OPT_fno_preserve_as_comments);
   Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);
   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4015,6 +4015,22 @@
   if (DebuggerTuning == llvm::DebuggerKind::SCE)
 CmdArgs.push_back("-dwarf-explicit-import");
 
+  if (Args.hasArg(options::OPT_gdwarf64)) {
+const Arg *A = Args.getLastArg(options::OPT_gdwarf64);
+const llvm::Triple  = TC.getTriple();
+if (DWARFVersion < 3)
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "DWARVv3 or greater.";
+else if (!RawTriple.isArch64Bit())
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "64 bit architecutre";
+else if (!RawTriple.isOSBinFormatELF())
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "elf output format.";
+else
+  CmdArgs.push_back("-gdwarf64");
+  }
+
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);
 }
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -563,6 +563,7 @@
   Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
   Options.MCOptions.MCNoWarn = CodeGenOpts.NoWarn;
   Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose;
+  Options.MCOptions.Dwarf64 = CodeGenOpts.Dwarf64;
   Options.MCOptions.PreserveAsmComments = CodeGenOpts.PreserveAsmComments;
   Options.MCOptions.ABIName = TargetOpts.ABI;
   for (const auto  : HSOpts.UserEntries)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2142,6 +2142,8 @@
   HelpText<"Generate source-level debug information with dwarf version 4">;
 def gdwarf_5 : Flag<["-"], "gdwarf-5">, Group,
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Enables DWARF64 format for ELF binaries, if debug information 
emission is enabled.">;
 
 def gcodeview : Flag<["-"], "gcodeview">,
   HelpText<"Generate CodeView debug information">,
Index: clang/include/clang/Basic/CodeGenOptions.def

[PATCH] D90507: Adding DWARF64 clang flag: -gdwarf64

2020-11-18 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2145-2146
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Generate DWARF64 debug information.">;
 

ayermolo wrote:
> dblaikie wrote:
> > Does this actually enable debug info emission (the -gdwarf-N versions do 
> > cause debug info to be emitted, and set the dwarf version - but, say 
> > -ggnu-pubnames does not cause debug info to be emitted if it isn't 
> > otherwise requested). Experience has shown it's probably better to have 
> > flags like this not enable debug info emission - so they can be used 
> > orthogonally (eg: if a project is large, it can add -gdwarf64 to its flags 
> > permanently, even though maybe only some build modes, etc, enable debug 
> > info (with -g))
> > 
> > If this doesn't enable debug info emission, the phrasing of the help text 
> > might be worth changing somewhat (I wonder how we document -ggnu-pubnames, 
> > for instance/comparison/inspiration)
> This enables DWARF64 if emission is enabled.
> 
> From DebugInfo it seems like it's only Elf Format:
> 
> bool Dwarf64 = Asm->TM.Options.MCOptions.Dwarf64 &&
>  DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.
>  TT.isArch64Bit() &&// DWARF64 requires 64-bit 
> relocations.
>  TT.isOSBinFormatELF(); // Support only ELF for now.
> This enables DWARF64 if emission is enabled.

Then perhaps the Help Text phrasing should distinguish this behavior from the 
behavior of -gdwarf-N which not only sets the version, but enables emission. 
(perhaps the phrasing of flags that don't enable emission, like 
"-ggnu-pubnames" could provide inspiration for phrasing the help text for this 
new flag)

> From DebugInfo it seems like it's only Elf Format:

I'm curious as to why it's ELF only, though - could you explain what the 
motivation is there?

(random aside: I still worry about this precedent for -g* flags that sometimes 
enable debug info and set a feature, and sometimes they only set the feature 
and don't enable debug info - it's a bit of a confusing mess and I personally 
rather the -fdebug* flags set a feature but don't enable emission and -g* flags 
enable emission, but there's a lack of clear agreement and there's examples on 
both sides, so not sure there's much to be done about that here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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


[PATCH] D90507: Adding DWARF64 clang flag: -gdwarf64

2020-11-18 Thread Alexander Yermolovich via Phabricator via cfe-commits
ayermolo added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2145-2146
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Generate DWARF64 debug information.">;
 

ayermolo wrote:
> dblaikie wrote:
> > Does this actually enable debug info emission (the -gdwarf-N versions do 
> > cause debug info to be emitted, and set the dwarf version - but, say 
> > -ggnu-pubnames does not cause debug info to be emitted if it isn't 
> > otherwise requested). Experience has shown it's probably better to have 
> > flags like this not enable debug info emission - so they can be used 
> > orthogonally (eg: if a project is large, it can add -gdwarf64 to its flags 
> > permanently, even though maybe only some build modes, etc, enable debug 
> > info (with -g))
> > 
> > If this doesn't enable debug info emission, the phrasing of the help text 
> > might be worth changing somewhat (I wonder how we document -ggnu-pubnames, 
> > for instance/comparison/inspiration)
> This enables DWARF64 if emission is enabled.
> 
> From DebugInfo it seems like it's only Elf Format:
> 
> bool Dwarf64 = Asm->TM.Options.MCOptions.Dwarf64 &&
>  DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.
>  TT.isArch64Bit() &&// DWARF64 requires 64-bit 
> relocations.
>  TT.isOSBinFormatELF(); // Support only ELF for now.
> This enables DWARF64 if emission is enabled.
> 






Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4025
+  << A->getAsString(Args) << "64 bit architecutre";
+else
+  CmdArgs.push_back("-gdwarf64");

dblaikie wrote:
> ikudrin wrote:
> > We also should check that the output format is ELF.
> Is DWARF64 not supported on MachO, for instance? (I'd have DWARF64 would be 
> pretty usable on any platform that supported DWARF in general - certainly 
> some system tools that are DWARF aware (including actual debuggers) might not 
> be up to the task of using it - but if we can correctly generate it in the 
> object files, it seems OK to accept the request (useful for folks testing 
> out/implementing DWARF64 consumers on those platforms))
 From DebugInfo it seems like it's only Elf Format:
 

```
 bool Dwarf64 = Asm->TM.Options.MCOptions.Dwarf64 &&
  DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.
  TT.isArch64Bit() &&// DWARF64 requires 64-bit relocations.
 TT.isOSBinFormatELF(); // Support only ELF for now.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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


[PATCH] D90507: Adding DWARF64 clang flag: -gdwarf64

2020-11-18 Thread Alexander Yermolovich via Phabricator via cfe-commits
ayermolo added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2145-2146
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Generate DWARF64 debug information.">;
 

dblaikie wrote:
> Does this actually enable debug info emission (the -gdwarf-N versions do 
> cause debug info to be emitted, and set the dwarf version - but, say 
> -ggnu-pubnames does not cause debug info to be emitted if it isn't otherwise 
> requested). Experience has shown it's probably better to have flags like this 
> not enable debug info emission - so they can be used orthogonally (eg: if a 
> project is large, it can add -gdwarf64 to its flags permanently, even though 
> maybe only some build modes, etc, enable debug info (with -g))
> 
> If this doesn't enable debug info emission, the phrasing of the help text 
> might be worth changing somewhat (I wonder how we document -ggnu-pubnames, 
> for instance/comparison/inspiration)
This enables DWARF64 if emission is enabled.

From DebugInfo it seems like it's only Elf Format:

bool Dwarf64 = Asm->TM.Options.MCOptions.Dwarf64 &&
 DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.
 TT.isArch64Bit() &&// DWARF64 requires 64-bit relocations.
 TT.isOSBinFormatELF(); // Support only ELF for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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


[PATCH] D90889: Remove RemappedFiles param from ASTUnit::LoadFromASTFile, NFC

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D90889

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


[PATCH] D90888: Frontend: Remove redundant call to CompilerInstance::setFileManager, NFC

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D90888

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


[PATCH] D91317: Support: Add RedirectingFileSystem::create from simple list of redirections

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D91317

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


[PATCH] D90887: ARCMigrate: Stop abusing PreprocessorOptions for passing back file remappings, NFC

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a reviewer: jansvoboda11.
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D90887

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


[PATCH] D90733: Frontend: Sink named pipe logic from CompilerInstance down to FileManager

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a reviewer: jansvoboda11.
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D90733

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


[PATCH] D90497: Module: Use FileEntryRef and DirectoryEntryRef in Umbrella, Header, and DirectoryName, NFC

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a reviewer: jansvoboda11.
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D90497

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


[PATCH] D90484: FileManager: Add FileEntryRef::getDir, returning DirectoryEntryRef

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a reviewer: jansvoboda11.
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D90484

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


[PATCH] D90053: Serialization: Change InputFile to use FileEntryRef and add getVirtualFileRef, NFC

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a reviewer: jansvoboda11.
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D90053

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


[PATCH] D90485: Lex: Update Module::findHeader to return FileEntryRef, NFC

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D90485

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


[PATCH] D91298: Frontend: Always create a new FileManager in ASTUnit::CodeComplete

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D91298

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


[PATCH] D91297: Frontend: Take VFS and MainFileBuffer by reference in PrecompiledPreamble::CanReuse, NFC

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D91297

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


[PATCH] D91296: Frontend: Clarify logic for using the preamble in ASTUnit::CodeComplete, almost NFC

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment.

ping


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

https://reviews.llvm.org/D91296

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


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

2020-11-18 Thread Wei Wang via Phabricator via cfe-commits
weiwang added a comment.

In D85808#2403587 , @tejohnson wrote:

> Thanks for adding the Driver test. I was thinking of something to test the 
> CompilerInvocation changes, similar to your test using opt, that ensures the 
> option has the desired behavior when invoked via clang. Looks like there is 
> an existing test clang/test/Frontend/optimization-remark-with-hotness.c that 
> perhaps could be extended or leveraged?

Thanks for the suggestion. I have added a new test case for clang. The source 
file used is the same one in the opt test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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


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

2020-11-18 Thread Wei Wang via Phabricator via cfe-commits
weiwang updated this revision to Diff 306256.
weiwang added a comment.

1. Add clang test with remarks output.
2. Fix a missing dependency on PSI in legacy pass manager.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/opt-record.c
  clang/test/Frontend/Inputs/remarks-hotness.prof
  clang/test/Frontend/remarks-hotness.cpp
  llvm/include/llvm/Analysis/ProfileSummaryInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/Module.h
  llvm/lib/Analysis/OptimizationRemarkEmitter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/Module.cpp
  llvm/test/Other/optimization-remarks-auto.ll
  llvm/test/Transforms/SampleProfile/Inputs/remarks-hotness.prof
  llvm/test/Transforms/SampleProfile/remarks-hotness.ll

Index: llvm/test/Transforms/SampleProfile/remarks-hotness.ll
===
--- /dev/null
+++ llvm/test/Transforms/SampleProfile/remarks-hotness.ll
@@ -0,0 +1,96 @@
+;; This test verifies 'auto' hotness threshold when profile file is provided.
+;;
+;; new PM
+; RUN: rm -f %t.yaml %t.hot.yaml
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.yaml \
+; RUN: -pass-remarks-with-hotness --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.yaml
+; RUN: FileCheck %s -check-prefix=YAML-MISS < %t.yaml
+
+;; test 'auto' threshold
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks-filter=inline --pass-remarks-output=%t.hot.yaml \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output
+; RUN: FileCheck %s -check-prefix=YAML-PASS < %t.hot.yaml
+; RUN: not FileCheck %s -check-prefix=YAML-MISS < %t.hot.yaml
+
+; RUN: opt %s --enable-new-pm --passes='sample-profile,cgscc(inline)' \
+; RUN: --sample-profile-file=%S/Inputs/remarks-hotness.prof \
+; RUN: -S --pass-remarks=inline --pass-remarks-missed=inline --pass-remarks-analysis=inline \
+; RUN: --pass-remarks-with-hotness --pass-remarks-hotness-threshold=auto --disable-output 2>&1 | FileCheck %s -check-prefix=CHECK-RPASS
+
+; YAML-PASS:  --- !Passed
+; YAML-PASS-NEXT: Pass:inline
+; YAML-PASS-NEXT: Name:Inlined
+; YAML-PASS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 10, Column: 10 }
+; YAML-PASS-NEXT: Function:_Z7caller1v
+; YAML-PASS-NEXT: Hotness: 401
+
+; YAML-MISS:  --- !Missed
+; YAML-MISS-NEXT: Pass:inline
+; YAML-MISS-NEXT: Name:NeverInline
+; YAML-MISS-NEXT: DebugLoc:{ File: remarks-hotness.cpp, Line: 14, Column: 10 }
+; YAML-MISS-NEXT: Function:_Z7caller2v
+; YAML-MISS-NEXT: Hotness: 2
+
+; CHECK-RPASS: _Z7callee1v inlined into _Z7caller1v with (cost=-30, threshold=4500) at callsite _Z7caller1v:1 (hotness: 401)
+; CHECK-RPASS-NOT: _Z7callee2v not inlined into _Z7caller2v because it should never be inlined (cost=never): noinline function attribute (hotness: 2)
+
+; ModuleID = 'remarks-hotness.cpp'
+source_filename = "remarks-hotness.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7callee1v() #0 !dbg !7 {
+  ret i32 1, !dbg !11
+}
+
+; Function Attrs: noinline nounwind uwtable use-sample-profile
+define dso_local i32 @_Z7callee2v() #1 !dbg !12 {
+  ret i32 2, !dbg !13
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller1v() #0 !dbg !14 {
+  %1 = call i32 @_Z7callee1v(), !dbg !15
+  ret i32 %1, !dbg !16
+}
+
+; Function Attrs: use-sample-profile
+define dso_local i32 @_Z7caller2v() #0 !dbg !17 {
+  %1 = call i32 @_Z7callee2v(), !dbg !18
+  ret i32 %1, !dbg !19
+}
+
+attributes #0 = { "use-sample-profile" }
+attributes #1 = { noinline nounwind uwtable "use-sample-profile" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, debugInfoForProfiling: true, nameTableKind: None)
+!1 = !DIFile(filename: "remarks-hotness.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 11.0.0"}
+!7 = distinct !DISubprogram(name: 

[PATCH] D91311: Add new 'preferred_name' attribute.

2020-11-18 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D91311#2403805 , @ldionne wrote:

> We can stick with this design, but I'd like to understand why `#if 
> _LIBCPP_HAS_NO_PREFERRED_NAME` is necessary in ``, and also the CI is 
> failing on MacOS.

You mean the HWAddressSanitizer test failure? That appears to be a flake. 
Looking through recent failures I found more that look the same: 
https://reviews.llvm.org/B79364 https://reviews.llvm.org/B79363 
https://reviews.llvm.org/B79358




Comment at: libcxx/include/iosfwd:188
 
+#ifdef _LIBCPP_PREFERRED_NAME
+template 

ldionne wrote:
> rsmith wrote:
> > aaron.ballman wrote:
> > > We always define `_LIBCPP_PREFERRED_NAME` so is this actually needed?
> > Thanks, I was trying to avoid the redundant redeclarations when the 
> > attribute is unavailable, but clearly this doesn't do that! Fixed.
> Is that really needed? What's the issue with having redundant declarations?
It's not necessary. I'm happy to remove it and redeclare the templates 
unconditionally if you prefer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91311

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


[clang] 67f16e9 - [NPM] Remove -enable-npm-optnone flag

2020-11-18 Thread Arthur Eubanks via cfe-commits

Author: Arthur Eubanks
Date: 2020-11-18T15:49:16-08:00
New Revision: 67f16e9e91f2f07cdb9813b60c195f5a1cd1f57d

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

LOG: [NPM] Remove -enable-npm-optnone flag

It has been on by default for a couple months without complaint.

Reviewed By: asbirlea

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

Added: 


Modified: 
clang/test/CodeGen/aggregate-assign-call.c
clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
llvm/lib/Passes/StandardInstrumentations.cpp
llvm/test/Feature/optnone-opt.ll

Removed: 




diff  --git a/clang/test/CodeGen/aggregate-assign-call.c 
b/clang/test/CodeGen/aggregate-assign-call.c
index a8382075ed3d..d3736fc94499 100644
--- a/clang/test/CodeGen/aggregate-assign-call.c
+++ b/clang/test/CodeGen/aggregate-assign-call.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -fexperimental-new-pass-manager -mllvm -enable-npm-optnone 
-triple x86_64-unknown-linux-gnu -O1 -S -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=O1
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -triple 
x86_64-unknown-linux-gnu -O1 -S -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=O1
 // RUN: %clang_cc1 -fno-experimental-new-pass-manager -triple 
x86_64-unknown-linux-gnu -O1 -S -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=O1
-// RUN: %clang_cc1 -fexperimental-new-pass-manager -mllvm -enable-npm-optnone 
-triple x86_64-unknown-linux-gnu -O0 -S -emit-llvm -o - %s | FileCheck %s 
--check-prefix=O0
+// RUN: %clang_cc1 -fexperimental-new-pass-manager -triple 
x86_64-unknown-linux-gnu -O0 -S -emit-llvm -o - %s | FileCheck %s 
--check-prefix=O0
 // RUN: %clang_cc1 -fno-experimental-new-pass-manager -triple 
x86_64-unknown-linux-gnu -O0 -S -emit-llvm -o - %s | FileCheck %s 
--check-prefix=O0
 //
 // Ensure that we place appropriate lifetime markers around indirectly returned

diff  --git a/clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c 
b/clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
index 1d85f8819579..9f877bed3990 100644
--- a/clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
+++ b/clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
@@ -1,15 +1,15 @@
 // Test that no passes are skipped under NPM with -O0/opt-bisect
 //
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fcoroutines-ts 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fsanitize=address 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fsanitize=hwaddress 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fsanitize=memory 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fsanitize=thread 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fsanitize=local-bounds 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fsanitize=dataflow 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fsanitize-coverage-trace-pc-guard 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fmemory-profile 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fcoroutines-ts 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fsanitize=address 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -O0 
-fexperimental-new-pass-manager %s 

[PATCH] D91743: [NPM] Remove -enable-npm-optnone flag

2020-11-18 Thread Arthur Eubanks via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG67f16e9e91f2: [NPM] Remove -enable-npm-optnone flag 
(authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91743

Files:
  clang/test/CodeGen/aggregate-assign-call.c
  clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Feature/optnone-opt.ll


Index: llvm/test/Feature/optnone-opt.ll
===
--- llvm/test/Feature/optnone-opt.ll
+++ llvm/test/Feature/optnone-opt.ll
@@ -4,13 +4,13 @@
 ; RUN: opt -O3 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s 
--check-prefix=O1 --check-prefix=O2O3
 ; RUN: opt -dce -gvn-hoist -loweratomic -S -debug -enable-new-pm=0 %s 2>&1 | 
FileCheck %s --check-prefix=MORE
 ; RUN: opt -indvars -licm -loop-deletion -loop-extract -loop-idiom 
-loop-instsimplify -loop-reduce -loop-reroll -loop-rotate -loop-unroll 
-loop-unswitch -enable-new-pm=0 -S -debug %s 2>&1 | FileCheck %s 
--check-prefix=LOOP
-; RUN: opt -enable-npm-optnone -passes='default' -S -debug-pass-manager %s 
2>&1 | FileCheck %s --check-prefix=%llvmcheckext-NPM-O0
-; RUN: opt -enable-npm-optnone -passes='default' -S -debug-pass-manager %s 
2>&1 | FileCheck %s --check-prefix=NPM-O1
-; RUN: opt -enable-npm-optnone -passes='default' -S -debug-pass-manager %s 
2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
-; RUN: opt -enable-npm-optnone -passes='default' -S -debug-pass-manager %s 
2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
-; RUN: opt -enable-npm-optnone -passes='dce,gvn-hoist,loweratomic' -S 
-debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-MORE
-; RUN: opt -enable-npm-optnone 
-passes='loop(indvars,licm,loop-deletion,loop-idiom,loop-instsimplify,loop-reduce,simple-loop-unswitch),loop-unroll'
 -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-LOOP
-; RUN: opt -enable-npm-optnone -passes='instsimplify,verify' -S 
-debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-REQUIRED
+; RUN: opt -passes='default' -S -debug-pass-manager %s 2>&1 | FileCheck %s 
--check-prefix=%llvmcheckext-NPM-O0
+; RUN: opt -passes='default' -S -debug-pass-manager %s 2>&1 | FileCheck %s 
--check-prefix=NPM-O1
+; RUN: opt -passes='default' -S -debug-pass-manager %s 2>&1 | FileCheck %s 
--check-prefix=NPM-O1 --check-prefix=NPM-O2O3
+; RUN: opt -passes='default' -S -debug-pass-manager %s 2>&1 | FileCheck %s 
--check-prefix=NPM-O1 --check-prefix=NPM-O2O3
+; RUN: opt -passes='dce,gvn-hoist,loweratomic' -S -debug-pass-manager %s 2>&1 
| FileCheck %s --check-prefix=NPM-MORE
+; RUN: opt 
-passes='loop(indvars,licm,loop-deletion,loop-idiom,loop-instsimplify,loop-reduce,simple-loop-unswitch),loop-unroll'
 -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-LOOP
+; RUN: opt -passes='instsimplify,verify' -S -debug-pass-manager %s 2>&1 | 
FileCheck %s --check-prefix=NPM-REQUIRED
 
 ; REQUIRES: asserts
 
Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -32,12 +32,6 @@
 
 using namespace llvm;
 
-// TODO: remove once all required passes are marked as such.
-static cl::opt
-EnableOptnone("enable-npm-optnone", cl::init(true),
-  cl::desc("Enable skipping optional passes optnone functions "
-   "under new pass manager"));
-
 cl::opt PreservedCFGCheckerInstrumentation::VerifyPreservedCFG(
 "verify-cfg-preserved", cl::Hidden,
 #ifdef NDEBUG
@@ -545,8 +539,6 @@
 }
 
 bool OptNoneInstrumentation::shouldRun(StringRef PassID, Any IR) {
-  if (!EnableOptnone)
-return true;
   const Function *F = nullptr;
   if (any_isa(IR)) {
 F = any_cast(IR);
Index: clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
===
--- clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
+++ clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
@@ -1,15 +1,15 @@
 // Test that no passes are skipped under NPM with -O0/opt-bisect
 //
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fcoroutines-ts 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fsanitize=address 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone 

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

2020-11-18 Thread Zequan Wu via Phabricator via cfe-commits
zequanwu created this revision.
zequanwu added reviewers: EricWF, rnk, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
zequanwu requested review of this revision.

According to https://eel.is/c++draft/cpp.predefined#2.6, `__STDCPP_THREADS__` 
is a predefined macro.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91747

Files:
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Preprocessor/init-aarch64.c


Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -233,6 +233,7 @@
 // AARCH64-NEXT: #define __SIZE_TYPE__ long unsigned int
 // AARCH64-NEXT: #define __SIZE_WIDTH__ 64
 // AARCH64_CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
+// AARCH64_CXX: #define __STDCPP_THREADS__ 1
 // AARCH64-NEXT: #define __STDC_HOSTED__ 1
 // AARCH64-NEXT: #define __STDC_UTF_16__ 1
 // AARCH64-NEXT: #define __STDC_UTF_32__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -403,6 +403,12 @@
 Builder.defineMacro("__STDCPP_DEFAULT_NEW_ALIGNMENT__",
 Twine(TI.getNewAlign() / TI.getCharWidth()) +
 TI.getTypeConstantSuffix(TI.getSizeType()));
+
+//   -- __STDCPP_­THREADS__
+//  Defined, and has the value integer literal 1, if and only if a
+//  program can have more than one thread of execution.
+if (!FEOpts.IsSingleThreadModel)
+  Builder.defineMacro("__STDCPP_THREADS__", "1");
   }
 
   // In C11 these are environment macros. In C++11 they are only defined
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2027,6 +2027,8 @@
   Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
   Opts.UseTemporary = !Args.hasArg(OPT_fno_temp_file);
   Opts.IsSystemModule = Args.hasArg(OPT_fsystem_module);
+  Opts.IsSingleThreadModel =
+  Args.getLastArgValue(OPT_mthread_model, "") == "single";
 
   if (Opts.ProgramAction != frontend::GenerateModule && Opts.IsSystemModule)
 Diags.Report(diag::err_drv_argument_only_allowed_with) << "-fsystem-module"
Index: clang/include/clang/Frontend/FrontendOptions.h
===
--- clang/include/clang/Frontend/FrontendOptions.h
+++ clang/include/clang/Frontend/FrontendOptions.h
@@ -303,6 +303,9 @@
   /// When using -emit-module, treat the modulemap as a system module.
   unsigned IsSystemModule : 1;
 
+  /// Wheather using -mthread-model single.
+  unsigned IsSingleThreadModel : 1;
+
   CodeCompleteOptions CodeCompleteOpts;
 
   /// Specifies the output format of the AST.


Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -233,6 +233,7 @@
 // AARCH64-NEXT: #define __SIZE_TYPE__ long unsigned int
 // AARCH64-NEXT: #define __SIZE_WIDTH__ 64
 // AARCH64_CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
+// AARCH64_CXX: #define __STDCPP_THREADS__ 1
 // AARCH64-NEXT: #define __STDC_HOSTED__ 1
 // AARCH64-NEXT: #define __STDC_UTF_16__ 1
 // AARCH64-NEXT: #define __STDC_UTF_32__ 1
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -403,6 +403,12 @@
 Builder.defineMacro("__STDCPP_DEFAULT_NEW_ALIGNMENT__",
 Twine(TI.getNewAlign() / TI.getCharWidth()) +
 TI.getTypeConstantSuffix(TI.getSizeType()));
+
+//   -- __STDCPP_­THREADS__
+//  Defined, and has the value integer literal 1, if and only if a
+//  program can have more than one thread of execution.
+if (!FEOpts.IsSingleThreadModel)
+  Builder.defineMacro("__STDCPP_THREADS__", "1");
   }
 
   // In C11 these are environment macros. In C++11 they are only defined
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2027,6 +2027,8 @@
   Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
   Opts.UseTemporary = !Args.hasArg(OPT_fno_temp_file);
   Opts.IsSystemModule = Args.hasArg(OPT_fsystem_module);
+  Opts.IsSingleThreadModel =
+  Args.getLastArgValue(OPT_mthread_model, "") == "single";
 
   if (Opts.ProgramAction != frontend::GenerateModule && 

[PATCH] D91509: [clangd] Add AST config to prebuild ASTs

2020-11-18 Thread Daan De Meyer via Phabricator via cfe-commits
DaanDeMeyer abandoned this revision.
DaanDeMeyer added a comment.

Abandoning as the end result is too slow even when including a limited number 
of files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91509

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


[PATCH] D72184: [BPF] support atomic instructions

2020-11-18 Thread Alexei Starovoitov via Phabricator via cfe-commits
ast added inline comments.



Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:783
+  let Inst{47-32} = addr{15-0}; // offset
+  let Inst{11-8} = new;
+  let Inst{7-4} = BPF_CMPXCHG.Value;

yonghong-song wrote:
> jackmanb wrote:
> > If we go down the route of matching operands with x86 as you have done for 
> > `XFALU64` and `XCHG`, I think  we should also do it for cmpxchg.
> > 
> > IIUC this is `dst = atomic_cmpxchg(*(src + off), r0, new);` 
> > 
> > But to do it in a single x86 instruction we need to have only 2 operands + 
> > the hard-coded r0. `r0 = atomic_xmpxchg(*(dst + off), r0, src);` would seem 
> > most natural to me.
> We can do this:
>   r0 = atomic_xmpxchg(*(dst + off), r0, src);
I'm confused. Isn't that what that is already?
r0 = atomic_cmpxchg(*(dst + off), r0, src);

In "class CMPXCHG":
let Inst{51-48} = addr{19-16}; // this is 'dst_reg' in bpf_insn.
let Inst{55-52} = new; // this is 'src_reg' in bpf_insn.

Same as old xadd and new fetch_add insns.
What am I missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

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


[PATCH] D89684: [AIX] Add mabi=vec-extabi options to enable the AIX extended and default vector ABIs.

2020-11-18 Thread Xiangling Liao via Phabricator via cfe-commits
Xiangling_L added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4616
+  if (Args.hasArg(options::OPT_maltivec) &&
+  (Triple.isOSAIX() || Triple.isOSBinFormatXCOFF())) {
+for (const Arg *A : Args) {

I am wondering what cases are not covered by `Triple.isOSAIX()`? Why do we also 
query `Triple.isOSBinFormatXCOFF()`?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4617
+  (Triple.isOSAIX() || Triple.isOSBinFormatXCOFF())) {
+for (const Arg *A : Args) {
+  auto optID = A->getOption().getID();

I see your intention here is to find if there are any `-mabi=vec-extabi` or 
`-mabi=vec-default` option specifying. If I am correct, why we need to loop 
through whole Args? Can we just use `hasArg` query?



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1417
+  if (Arg *A = Args.getLastArg(OPT_mabi_EQ_vec_default, 
OPT_mabi_EQ_vec_extabi)) {
+if (!T.isOSAIX() || !T.isOSBinFormatXCOFF())
+  Diags.Report(diag::err_drv_unsupported_opt_for_target)

ditto.



Comment at: clang/test/CodeGen/altivec.c:2
 // RUN: %clang_cc1 -target-feature +altivec -triple powerpc-unknown-unknown 
-emit-llvm %s -o - | FileCheck %s
-
+// RUN: %clang_cc1 -target-feature +altivec -mabi=vec-extabi -triple 
powerpc-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -target-feature +altivec -mabi=vec-extabi -triple 
powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s

Based on the code added in `BackendUtil:551`, should we also add a case for 
compiling a source to assembly?



Comment at: llvm/include/llvm/Target/TargetMachine.h:246
 
+  bool getAIXExtendedAltivecABI() const {
+return Options.AIXExtendedAltivecABI;

I don't see this function gets invoked anywhere,can we remove it?



Comment at: llvm/include/llvm/Target/TargetOptions.h:179
 
+/// AIXExtendedAltivecABI - This flag returns true when -mabi=vec-extabi is
+/// specified. The code generator is then able to use both volatile and

-mabi=vec-extabi is the FE option, should we s/-mabi=vec-extabi/-vec-extabi?



Comment at: llvm/lib/CodeGen/CommandFlags.cpp:286
 
+  static cl::opt AIXExtendedAltivecABI(
+  "vec-extabi", cl::desc("Enable the AIX Extended Altivec ABI."),

Based on the naming convention in this file context, this seems should be 
`EnableAIXExtendedAltivecABI`.



Comment at: llvm/lib/CodeGen/CommandFlags.cpp:287
+  static cl::opt AIXExtendedAltivecABI(
+  "vec-extabi", cl::desc("Enable the AIX Extended Altivec ABI."),
+  cl::init(false));

Can we add a testcase for this backend option?



Comment at: llvm/test/CodeGen/PowerPC/aix-default-vec-abi.ll:1
+; RUN: not --crash llc < %s -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr8 2>&1 | 
FileCheck %s
+; RUN: not --crash llc < %s -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr8 2>&1 | 
FileCheck %s

Can we simplify the testcase to only contain one vector type parameter? I think 
that would be sufficient to trigger the error.



Comment at: llvm/test/CodeGen/PowerPC/aix-default-vec-abi.ll:4
+
+define dso_local <4 x i32> @vec_callee(<4 x i32> %vec1, <4 x i32> %vec2, <4 x 
i32> %vec3, <4 x i32> %vec4, <4 x i32> %vec5, <4 x i32> %vec6, <4 x i32> %vec7, 
<4 x i32> %vec8, <4 x i32> %vec9, <4 x i32> %vec10, <4 x i32> %vec11, <4 x i32> 
%vec12, <4 x i32> %vec13, <4 x i32> %vec14) {
+  entry:

ditto: can we simplify the testcase?



Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-str.ll:7
-; RUN: llc -verify-machineinstrs -mcpu=pwr4 \
-; RUN: -mtriple powerpc-ibm-aix-xcoff  -data-sections=false < %s | 
FileCheck %s
-; RUN: llc -verify-machineinstrs -mcpu=pwr4 \

I am wondering why do we remove `-data-sections=false` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89684

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


[PATCH] D91743: [NPM] Remove -enable-npm-optnone flag

2020-11-18 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
aeubanks added reviewers: asbirlea, ychen.
Herald added subscribers: llvm-commits, cfe-commits, jfb, hiraditya.
Herald added projects: clang, LLVM.
aeubanks requested review of this revision.

It has been on by default for a couple months without complaint.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91743

Files:
  clang/test/CodeGen/aggregate-assign-call.c
  clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Feature/optnone-opt.ll


Index: llvm/test/Feature/optnone-opt.ll
===
--- llvm/test/Feature/optnone-opt.ll
+++ llvm/test/Feature/optnone-opt.ll
@@ -4,13 +4,13 @@
 ; RUN: opt -O3 -S -debug -enable-new-pm=0 %s 2>&1 | FileCheck %s 
--check-prefix=O1 --check-prefix=O2O3
 ; RUN: opt -dce -gvn-hoist -loweratomic -S -debug -enable-new-pm=0 %s 2>&1 | 
FileCheck %s --check-prefix=MORE
 ; RUN: opt -indvars -licm -loop-deletion -loop-extract -loop-idiom 
-loop-instsimplify -loop-reduce -loop-reroll -loop-rotate -loop-unroll 
-loop-unswitch -enable-new-pm=0 -S -debug %s 2>&1 | FileCheck %s 
--check-prefix=LOOP
-; RUN: opt -enable-npm-optnone -passes='default' -S -debug-pass-manager %s 
2>&1 | FileCheck %s --check-prefix=%llvmcheckext-NPM-O0
-; RUN: opt -enable-npm-optnone -passes='default' -S -debug-pass-manager %s 
2>&1 | FileCheck %s --check-prefix=NPM-O1
-; RUN: opt -enable-npm-optnone -passes='default' -S -debug-pass-manager %s 
2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
-; RUN: opt -enable-npm-optnone -passes='default' -S -debug-pass-manager %s 
2>&1 | FileCheck %s --check-prefix=NPM-O1 --check-prefix=NPM-O2O3
-; RUN: opt -enable-npm-optnone -passes='dce,gvn-hoist,loweratomic' -S 
-debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-MORE
-; RUN: opt -enable-npm-optnone 
-passes='loop(indvars,licm,loop-deletion,loop-idiom,loop-instsimplify,loop-reduce,simple-loop-unswitch),loop-unroll'
 -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-LOOP
-; RUN: opt -enable-npm-optnone -passes='instsimplify,verify' -S 
-debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-REQUIRED
+; RUN: opt -passes='default' -S -debug-pass-manager %s 2>&1 | FileCheck %s 
--check-prefix=%llvmcheckext-NPM-O0
+; RUN: opt -passes='default' -S -debug-pass-manager %s 2>&1 | FileCheck %s 
--check-prefix=NPM-O1
+; RUN: opt -passes='default' -S -debug-pass-manager %s 2>&1 | FileCheck %s 
--check-prefix=NPM-O1 --check-prefix=NPM-O2O3
+; RUN: opt -passes='default' -S -debug-pass-manager %s 2>&1 | FileCheck %s 
--check-prefix=NPM-O1 --check-prefix=NPM-O2O3
+; RUN: opt -passes='dce,gvn-hoist,loweratomic' -S -debug-pass-manager %s 2>&1 
| FileCheck %s --check-prefix=NPM-MORE
+; RUN: opt 
-passes='loop(indvars,licm,loop-deletion,loop-idiom,loop-instsimplify,loop-reduce,simple-loop-unswitch),loop-unroll'
 -S -debug-pass-manager %s 2>&1 | FileCheck %s --check-prefix=NPM-LOOP
+; RUN: opt -passes='instsimplify,verify' -S -debug-pass-manager %s 2>&1 | 
FileCheck %s --check-prefix=NPM-REQUIRED
 
 ; REQUIRES: asserts
 
Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -32,12 +32,6 @@
 
 using namespace llvm;
 
-// TODO: remove once all required passes are marked as such.
-static cl::opt
-EnableOptnone("enable-npm-optnone", cl::init(true),
-  cl::desc("Enable skipping optional passes optnone functions "
-   "under new pass manager"));
-
 cl::opt PreservedCFGCheckerInstrumentation::VerifyPreservedCFG(
 "verify-cfg-preserved", cl::Hidden,
 #ifdef NDEBUG
@@ -545,8 +539,6 @@
 }
 
 bool OptNoneInstrumentation::shouldRun(StringRef PassID, Any IR) {
-  if (!EnableOptnone)
-return true;
   const Function *F = nullptr;
   if (any_isa(IR)) {
 F = any_cast(IR);
Index: clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
===
--- clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
+++ clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
@@ -1,15 +1,15 @@
 // Test that no passes are skipped under NPM with -O0/opt-bisect
 //
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fcoroutines-ts 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 
-fexperimental-new-pass-manager %s -fdebug-pass-manager -emit-llvm -o /dev/null 
-fsanitize=address 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -mllvm -enable-npm-optnone -O0 

[PATCH] D90507: Adding DWARF64 clang flag: -gdwarf64

2020-11-18 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2145-2146
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Generate DWARF64 debug information.">;
 

Does this actually enable debug info emission (the -gdwarf-N versions do cause 
debug info to be emitted, and set the dwarf version - but, say -ggnu-pubnames 
does not cause debug info to be emitted if it isn't otherwise requested). 
Experience has shown it's probably better to have flags like this not enable 
debug info emission - so they can be used orthogonally (eg: if a project is 
large, it can add -gdwarf64 to its flags permanently, even though maybe only 
some build modes, etc, enable debug info (with -g))

If this doesn't enable debug info emission, the phrasing of the help text might 
be worth changing somewhat (I wonder how we document -ggnu-pubnames, for 
instance/comparison/inspiration)



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4022-4023
+if (DWARFVersion < 3)
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "-gdwarf-5, -gdwarf-4, -gdwarf-3";
+else if (!RawTriple.isArch64Bit())

perhaps we could phrase this more generally (so it's true even when DWARFv6 
comes along - and doesn't get unwieldy listing all the DWARF versions).

"DWARFv3 or later" ?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4025
+  << A->getAsString(Args) << "64 bit architecutre";
+else
+  CmdArgs.push_back("-gdwarf64");

ikudrin wrote:
> We also should check that the output format is ELF.
Is DWARF64 not supported on MachO, for instance? (I'd have DWARF64 would be 
pretty usable on any platform that supported DWARF in general - certainly some 
system tools that are DWARF aware (including actual debuggers) might not be up 
to the task of using it - but if we can correctly generate it in the object 
files, it seems OK to accept the request (useful for folks testing 
out/implementing DWARF64 consumers on those platforms))



Comment at: clang/test/Driver/debug-options.c:390
+// GDWARF64_OFF:  error: invalid argument '-gdwarf64' only allowed with 
'-gdwarf-5, -gdwarf-4, -gdwarf-3'
+// GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 
bit architecutre'

Typo in "architecture"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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


[PATCH] D91669: Don’t break before nested block param when prior param is not a block

2020-11-18 Thread Dave Lee via Phabricator via cfe-commits
kastiglione accepted this revision.
kastiglione added a comment.

The purpose and the tests LGTM! Hopefully someone will weigh in on the 
implementation in ContinuationIndenter.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91669

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


[PATCH] D86853: [modules] Fix crash in call to `FunctionDecl::setPure()`

2020-11-18 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment.

I forgot to follow up, but LGTM too.

> @rsmith, @v.g.vassilev hey I stamped this patch assuming it looks ok. But 
> definitely shout at me if more feedback needs to be addressed. Happy to 
> follow up.

FWIW, this has been working on our code base for sometime now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86853

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


[PATCH] D90507: Adding DWARF64 clang flag: -gdwarf64

2020-11-18 Thread Alexander Yermolovich via Phabricator via cfe-commits
ayermolo added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4020
+const Arg *A = Args.getLastArg(options::OPT_gdwarf64);
+const llvm::Triple  = TC.getTriple();
+if (DWARFVersion < 3)

MaskRay wrote:
> The variable is only used once. Inline it
Both A, and RawTriple are used more than once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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


[PATCH] D90507: Adding DWARF64 clang flag: -gdwarf64

2020-11-18 Thread Alexander Yermolovich via Phabricator via cfe-commits
ayermolo updated this revision to Diff 306223.
ayermolo retitled this revision from "Adding DWARF64 clang flag" to "Adding 
DWARF64 clang flag: -gdwarf64".

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -375,3 +375,16 @@
 // RUN:| FileCheck -check-prefix=NO_DEBUG_UNUSED_TYPES %s
 // NO_DEBUG_UNUSED_TYPES: 
"-debug-info-kind={{limited|line-tables-only|standalone}}"
 // NO_DEBUG_UNUSED_TYPES-NOT: "-debug-info-kind=unused-types"
+//
+// RUN: %clang -### -gdwarf-5 -gdwarf64 -target x86_64-linux-gnu %s 2>&1 | 
FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 -target x86_64-linux-gnu %s 2>&1 | 
FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-3 -gdwarf64 -target x86_64-linux-gnu %s 2>&1 | 
FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-2 -gdwarf64 -target x86_64-linux-gnu %s 2>&1 | 
FileCheck -check-prefix=GDWARF64_OFF %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 -target x86_64-linux-gnu -target 
x86_64-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 -target i386-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_32ARCH %s
+//
+// GDWARF64_ON:  "-gdwarf64"
+// GDWARF64_OFF:  error: invalid argument '-gdwarf64' only allowed with 
'-gdwarf-5, -gdwarf-4, -gdwarf-3'
+// GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 
bit architecutre'
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1005,6 +1005,7 @@
   Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = !Args.hasArg(OPT_fno_verbose_asm);
+  Opts.Dwarf64 = Args.hasArg(OPT_gdwarf64);
   Opts.PreserveAsmComments = !Args.hasArg(OPT_fno_preserve_as_comments);
   Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);
   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4015,6 +4015,22 @@
   if (DebuggerTuning == llvm::DebuggerKind::SCE)
 CmdArgs.push_back("-dwarf-explicit-import");
 
+  if (Args.hasArg(options::OPT_gdwarf64)) {
+const Arg *A = Args.getLastArg(options::OPT_gdwarf64);
+const llvm::Triple  = TC.getTriple();
+if (DWARFVersion < 3)
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "-gdwarf-5, -gdwarf-4, -gdwarf-3";
+else if (!RawTriple.isArch64Bit())
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "64 bit architecutre";
+else if (!RawTriple.isOSBinFormatELF())
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "elf output format.";
+else
+  CmdArgs.push_back("-gdwarf64");
+  }
+
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);
 }
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -563,6 +563,7 @@
   Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
   Options.MCOptions.MCNoWarn = CodeGenOpts.NoWarn;
   Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose;
+  Options.MCOptions.Dwarf64 = CodeGenOpts.Dwarf64;
   Options.MCOptions.PreserveAsmComments = CodeGenOpts.PreserveAsmComments;
   Options.MCOptions.ABIName = TargetOpts.ABI;
   for (const auto  : HSOpts.UserEntries)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2142,6 +2142,8 @@
   HelpText<"Generate source-level debug information with dwarf version 4">;
 def gdwarf_5 : Flag<["-"], "gdwarf-5">, Group,
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Generate DWARF64 debug information.">;
 
 def gcodeview : Flag<["-"], "gcodeview">,
   HelpText<"Generate CodeView debug information">,
Index: 

[PATCH] D86853: [modules] Fix crash in call to `FunctionDecl::setPure()`

2020-11-18 Thread Xun Li via Phabricator via cfe-commits
lxfind added a comment.

@rsmith, @v.g.vassilev hey I stamped this patch assuming it looks ok. But 
definitely shout at me if more feedback needs to be addressed. Happy to follow 
up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86853

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


[clang] 5f2c554 - Fix assert on valid due to incorrect assumption that a field name must

2020-11-18 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2020-11-18T14:04:02-08:00
New Revision: 5f2c5541f78750c21004e0172f13db4632966fd3

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

LOG: Fix assert on valid due to incorrect assumption that a field name must
be unique in its scope.

Added: 
clang/test/SemaTemplate/default-member-init.cpp

Modified: 
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index d31d2e32547a..9d2090dfd8eb 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -15082,24 +15082,14 @@ ExprResult 
Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
 DeclContext::lookup_result Lookup =
 ClassPattern->lookup(Field->getDeclName());
 
-// Lookup can return at most two results: the pattern for the field, or the
-// injected class name of the parent record. No other member can have the
-// same name as the field.
-// In modules mode, lookup can return multiple results (coming from
-// 
diff erent modules).
-assert((getLangOpts().Modules || (!Lookup.empty() && Lookup.size() <= 2)) 
&&
-   "more than two lookup results for field name");
-FieldDecl *Pattern = dyn_cast(Lookup[0]);
-if (!Pattern) {
-  assert(isa(Lookup[0]) &&
- "cannot have other non-field member with same name");
-  for (auto L : Lookup)
-if (isa(L)) {
-  Pattern = cast(L);
-  break;
-}
-  assert(Pattern && "We must have set the Pattern!");
+FieldDecl *Pattern = nullptr;
+for (auto L : Lookup) {
+  if (isa(L)) {
+Pattern = cast(L);
+break;
+  }
 }
+assert(Pattern && "We must have set the Pattern!");
 
 if (!Pattern->hasInClassInitializer() ||
 InstantiateInClassInitializer(Loc, Field, Pattern,

diff  --git a/clang/test/SemaTemplate/default-member-init.cpp 
b/clang/test/SemaTemplate/default-member-init.cpp
new file mode 100644
index ..5615d11e33d6
--- /dev/null
+++ b/clang/test/SemaTemplate/default-member-init.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// expected-no-diagnostics
+
+struct Q { enum F { f }; };
+
+template struct A : Q {
+  enum E { e } E = e;
+
+  using Q::F;
+  Q::F F = f;
+};
+A a = {};



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


[PATCH] D91279: [PowerPC] DForm instructions should be preferred when using zero register

2020-11-18 Thread Stefan Pintilie via Phabricator via cfe-commits
stefanp added a comment.

In D91279#2400854 , @stefanp wrote:

> In D91279#2390160 , @shchenz wrote:
>
>> Using dform with offset 0 can save one register r0/X0, this is benefit for 
>> register allocation? But adding it in `PPCPreEmitPeephole` pass which is 
>> after register allocation will make the benefit gone.
>> Maybe we need to do it before register allocation? For example at the place 
>> where the x-form with zero register is generated.
>>
>> I checked one example `loadConstant` in  
>> `test/CodeGen/PowerPC/f128-passByValue.ll`.
>> We generate `LXVX $zero8, ` in ISEL because we meet the worst case and we 
>> don't have d-form choice for the instruction selection. so we have to use 
>> x-form and in x-form selection, we have to use zero/zero8 as the base and 
>> use load address as the index. See 
>> `PPCTargetLowering::SelectAddressRegRegOnly`.
>>
>> I guess most cases are with same reason for generating x-form + zero 
>> register, we meet the worst case in ISEL, so we have to use x-form + zero 
>> register form, with this form, we can always select a powerpc load/store 
>> instruction.
>>
>> For me, a better solution should be change the worst case handling in ISEL, 
>> it is before RA and it is also transparent for types like STXVX/LXVX/ and 
>> also LDX/STDX, LFDX/STFDX...
>
> I'm just going to jump in to give a little more background. The initial 
> reason we wanted to do this was to enable an optimization that actually 
> happens in the linker after the code is emitted.
> To get the idea you can look at this test:
>
>   /llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll
>
> Which contains this section:
>
>   ; FIXME: we should always convert X-Form instructions that use
>   ; PPC::ZERO[8] to the corresponding D-Form so we can perform this opt.
>   define dso_local void @ReadWrite128() local_unnamed_addr #0 {
>   ; CHECK-LABEL: ReadWrite128:
>   ; CHECK:   # %bb.0: # %entry
>   ; CHECK-NEXT:pld r3, input128@got@pcrel(0), 1
>   ; CHECK-NEXT:lxvx vs0, 0, r3
>   ; CHECK-NEXT:pld r3, output128@got@pcrel(0), 1
>   ; CHECK-NEXT:stxvx vs0, 0, r3
>   ; CHECK-NEXT:blr
>   entry:
> %0 = load i128, i128* @input128, align 16
> store i128 %0, i128* @output128, align 16
> ret void
>   }
>
> When we have a GOT access like this it is possible for the compiler to mark 
> the instruction with `R_PPC64_PCREL_OPT` and then the linker merges the two 
> instructions into one and replaces the second instruction with a `nop`. The 
> problem is that this opt can only be done if the second instruction is DForm. 
> We noticed that when we implemented this optimization we could not catch all 
> of the cases because in some situations (like the one above) we use the XForm 
> instead of the DForm.
>
> Having said that, we should try to do this before the PreEmitPeephole. The 
> optimization that adds the `R_PPC64_PCREL_OPT` relocation is also in the 
> PreEmitPeephole and I'm not sure if it will be detected if we do both things 
> at the same time (both as in convert the XForm to a DForm and then have the 
> same opt use that DForm to add the relocation).
>
> I agree that ISel is a better place for this. If we cannot do this in ISel 
> then we should still try to do this before we get to the PreEmitPeephole or 
> at least make sure that both the DForm is present and that the 
> `R_PPC64_PCREL_OPT` relocation is added as we expected in the same pass.

After a discussion with the group I would like to correct what I said in the 
previous post.
There already is a plan to do this in ISel in a different patch. The reason we 
also want to do this optimization here is to try to catch situations where this 
pattern is not known in ISel and only appears after other optimizations later 
on. Ideally we do not want to have any situations where the XForm exists in the 
final binary and having this final check in the PreEmitPeephole should ensure 
that. Basically, we also want to do this check here to find anything that ISel 
may have missed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91279

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


[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

2020-11-18 Thread Louis Dionne via Phabricator via cfe-commits
ldionne added a comment.

I think this looks good. I must admit I'm a bit worried about any unintended 
consequences this might have or breakage this might cause in cases where we'd 
switch from linking against the SDK libc++.dylib to the toolchain one. This 
would only impact the toolchain released by LLVM, not Apple's, though, since 
Apple's toolchain doesn't contain the dylib in `/../lib`.

I think this would merit a Clang release note.


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

https://reviews.llvm.org/D45639

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


[PATCH] D87946: [OpenMP] Add Location Fields to Libomptarget Runtime for Debugging

2020-11-18 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 306210.
jhuber6 added a comment.

Rebasing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87946

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/capturing_in_templates.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  clang/test/OpenMP/declare_target_link_codegen.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_lambda_pointer_capturing.cpp
  clang/test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
  clang/test/OpenMP/target_codegen.cpp
  clang/test/OpenMP/target_data_codegen.cpp
  clang/test/OpenMP/target_data_map_pointer_array_subscript_codegen.cpp
  clang/test/OpenMP/target_data_member_codegen.cpp
  clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
  clang/test/OpenMP/target_defaultmap_codegen.cpp
  clang/test/OpenMP/target_depend_codegen.cpp
  clang/test/OpenMP/target_device_codegen.cpp
  clang/test/OpenMP/target_enter_data_codegen.cpp
  clang/test/OpenMP/target_enter_data_depend_codegen.cpp
  clang/test/OpenMP/target_exit_data_codegen.cpp
  clang/test/OpenMP/target_exit_data_depend_codegen.cpp
  clang/test/OpenMP/target_firstprivate_codegen.cpp
  clang/test/OpenMP/target_is_device_ptr_codegen.cpp
  clang/test/OpenMP/target_map_codegen_00.cpp
  clang/test/OpenMP/target_map_codegen_01.cpp
  clang/test/OpenMP/target_map_codegen_02.cpp
  clang/test/OpenMP/target_map_codegen_03.cpp
  clang/test/OpenMP/target_map_codegen_04.cpp
  clang/test/OpenMP/target_map_codegen_05.cpp
  clang/test/OpenMP/target_map_codegen_06.cpp
  clang/test/OpenMP/target_map_codegen_07.cpp
  clang/test/OpenMP/target_map_codegen_08.cpp
  clang/test/OpenMP/target_map_codegen_09.cpp
  clang/test/OpenMP/target_map_codegen_10.cpp
  clang/test/OpenMP/target_map_codegen_11.cpp
  clang/test/OpenMP/target_map_codegen_12.cpp
  clang/test/OpenMP/target_map_codegen_13.cpp
  clang/test/OpenMP/target_map_codegen_14.cpp
  clang/test/OpenMP/target_map_codegen_15.cpp
  clang/test/OpenMP/target_map_codegen_16.cpp
  clang/test/OpenMP/target_map_codegen_17.cpp
  clang/test/OpenMP/target_map_codegen_18.inc
  clang/test/OpenMP/target_map_codegen_19.cpp
  clang/test/OpenMP/target_map_codegen_20.cpp
  clang/test/OpenMP/target_map_codegen_21.cpp
  clang/test/OpenMP/target_map_codegen_22.cpp
  clang/test/OpenMP/target_map_codegen_23.cpp
  clang/test/OpenMP/target_map_codegen_24.cpp
  clang/test/OpenMP/target_map_codegen_25.cpp
  clang/test/OpenMP/target_map_codegen_26.cpp
  clang/test/OpenMP/target_map_codegen_27.cpp
  clang/test/OpenMP/target_map_codegen_28.cpp
  clang/test/OpenMP/target_map_codegen_29.cpp
  clang/test/OpenMP/target_map_codegen_30.cpp
  clang/test/OpenMP/target_map_codegen_31.cpp
  clang/test/OpenMP/target_map_codegen_32.cpp
  clang/test/OpenMP/target_map_codegen_33.cpp
  clang/test/OpenMP/target_map_member_expr_array_section_codegen.cpp
  clang/test/OpenMP/target_map_names.cpp
  clang/test/OpenMP/target_parallel_codegen.cpp
  clang/test/OpenMP/target_parallel_depend_codegen.cpp
  clang/test/OpenMP/target_parallel_for_codegen.cpp
  clang/test/OpenMP/target_parallel_for_depend_codegen.cpp
  clang/test/OpenMP/target_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  clang/test/OpenMP/target_parallel_for_simd_uses_allocators_codegen.cpp
  clang/test/OpenMP/target_parallel_for_uses_allocators_codegen.cpp
  clang/test/OpenMP/target_parallel_if_codegen.cpp

[PATCH] D90507: Adding DWARF64 clang flag

2020-11-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

> Adding DWARF64 clang flag

It is important to mention the exact option name: Add `-gdwarf-64` to enable 
64-bit DWARF format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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


[PATCH] D90507: Adding DWARF64 clang flag

2020-11-18 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4018
 
+  if (Args.hasArg(options::OPT_gdwarf64)) {
+const Arg *A = Args.getLastArg(options::OPT_gdwarf64);

`if (const Arg *A = Args.getLastArg(options::OPT_gdwarf64)`)



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4020
+const Arg *A = Args.getLastArg(options::OPT_gdwarf64);
+const llvm::Triple  = TC.getTriple();
+if (DWARFVersion < 3)

The variable is only used once. Inline it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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


[PATCH] D91311: Add new 'preferred_name' attribute.

2020-11-18 Thread Louis Dionne via Phabricator via cfe-commits
ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Actually, apologies -- I might have accepted this too quickly.

We can stick with this design, but I'd like to understand why `#if 
_LIBCPP_HAS_NO_PREFERRED_NAME` is necessary in ``, and also the CI is 
failing on MacOS.




Comment at: libcxx/include/iosfwd:188
 
+#ifdef _LIBCPP_PREFERRED_NAME
+template 

rsmith wrote:
> aaron.ballman wrote:
> > We always define `_LIBCPP_PREFERRED_NAME` so is this actually needed?
> Thanks, I was trying to avoid the redundant redeclarations when the attribute 
> is unavailable, but clearly this doesn't do that! Fixed.
Is that really needed? What's the issue with having redundant declarations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91311

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


[PATCH] D91311: Add new 'preferred_name' attribute.

2020-11-18 Thread Louis Dionne via Phabricator via cfe-commits
ldionne accepted this revision as: libc++.
ldionne added a comment.
This revision is now accepted and ready to land.

In D91311#2400998 , @dblaikie wrote:

> A concrete/real-world example might be helpful, if you happen to have one on 
> hand.

See what Richard included in his comment.

@rsmith

Ok, thanks for thinking this through. I do agree we'd most likely want a more 
powerful mechanism for creating these strings.

I'm still only "meh" on the current design, especially because I see the 
ergonomics as a high barrier, but this isn't blocking. So, LGTM from libc++'s 
perspective -- thanks for having a discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91311

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


[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

2020-11-18 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev planned changes to this revision.
kbobyrev added a comment.

Need to address the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71880

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


[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

2020-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:274
+return canonicalRenameDecl(Function);
+  return D;
+}

I think we should call `getCanonicalDecl` before returning the final result 
(calling it at the beginning doesn't guarantee the final result is canonical).



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:601
 return makeError(ReasonToReject::AmbiguousSymbol);
-  const auto  =
-  llvm::cast(*(*DeclsUnderCursor.begin())->getCanonicalDecl());
+  const auto  = llvm::cast(*(*DeclsUnderCursor.begin()));
   if (RenameDecl.getName() == RInputs.NewName)

nit: llvm::cast is not needed, just `const NamedDecl  = 
*(*DeclsUnderCursor.begin());`



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:239
+  if (const auto *Template = D->getPrimaryTemplate())
+return canonicalRenameDecl(Template);
+  return D;

kbobyrev wrote:
> hokein wrote:
> > the `auto` + varies `canonicalRenameDecl` overrides make the code hard to 
> > follow.
> > 
> > since these functions are small, I think we can inline them into the main 
> > `canonicalRenameDecl(const NamedDecl *D)`
> I removed `auto`s but I believe merging them into a single function would not 
> be great for two reasons:
> 
> * Some classes are already handled by the same function outside of the main 
> one: ctor/dtor's parent `CXXRecordDecl`, etc: in the main function we'd have 
> to have a weird logic behind extracting those and this is likely to result in 
> more code
> * Some functions call other ones (e.g. here we have 
> `canonicalRenameDecl(Template)` - sure, right now it's just 
> `D->getTemplatedDecl()` but if we decide to change something and when we 
> introduce more code it'd be easy to forget and code duplication is not really 
> a good idea).
> 
> WDYT?
this is more about encapsulation -- my understanding is that 
`canonicalRenameDecl(const NamedDecl)` should be the only main entry, others 
are just implementation details, so we expect client just calls the main 
function, but not other overrides.  The current state leaks these 
implementation details outside of the world.


> Some classes are already handled by the same function outside of the main 
> one: ctor/dtor's parent CXXRecordDecl, etc: in the main function we'd have to 
> have a weird logic behind extracting those and this is likely to result in 
> more code

don't follow this, for ctor/dtor's parent CXXRecordDecl, I think just calling 
the main canonicalRenameDecl recursively on the parent should be enough?


> Some functions call other ones (e.g. here we have 
> canonicalRenameDecl(Template) - sure, right now it's just 
> D->getTemplatedDecl() but if we decide to change something and when we 
> introduce more code it'd be easy to forget and code duplication is not really 
> a good idea).

I agree your points. Given the current code, I think inlining them seems quite 
straight-forward and clearer to me (the only case is `FunctionDecl`, which 
calls the `canonicalRenameDecl(const TemplateDecl *D)`, but we can just call 
`getTemplatedDecl` as well). 

Regarding your concerns, I think we can always find ways to extend the code 
when we need to enhance the TemplateDecl case (what kind of enhancement we'll 
do for templateDecl?)




Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:255
+const NamedDecl *canonicalRenameDecl(const ClassTemplateSpecializationDecl *D) 
{
+  return D->getSpecializedTemplate()->getTemplatedDecl();
+}

kbobyrev wrote:
> hokein wrote:
> > want to confirm my understanding:
> > 
> > given the example below:
> > 
> > ```
> > template
> > class Foo {};
> > 
> > template<>
> > class Foo {};
> > ```
> > 
> > the AST is like:
> > 
> > ```
> > ClassTemplateDecl
> >   |-CXXRecordDecl (Foo definition) -> (1)
> >   |-ClassTemplateSpecialization. 
> > 
> > ClassTemplateSpecializationDecl -> call canonicalRenameDecl on it.
> >   |-Template Argument int
> >   |-CXXRecordDecl -> (2)
> > ```
> > 
> > if we pass `ClassTemplateSpecializationDecl` to this function, this 
> > function will return (2)?
> No, this will return (1): `getSpecializedTemplate()` returns 
> `ClassTemplateDecl` and then `getTemplatedDecl()` gets to `CXXRecordDecl` in 
> it.
I see. thanks for the explanation. I misunderstood the `getSpecializedTemplate`.



Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:261
+//   CXXRecordDecl
+// - Specializations should point to the specialized declaration.
+// - Instantiations should point to instantiated declaration.

kbobyrev wrote:
> hokein wrote:
> > I think the motivation is for merely renaming the explicit template 
> > specialization, but not the primary template?
> How come? The specialized template is the primary template, am I missing 
> something?
ah, you're right, sorry -- I was confused by the term "specialized template", I 

[PATCH] D90507: Adding DWARF64 clang flag

2020-11-18 Thread Alexander Yermolovich via Phabricator via cfe-commits
ayermolo marked 2 inline comments as done.
ayermolo added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:35
 CODEGENOPT(AsmVerbose, 1, 0) ///< -dA, -fverbose-asm.
+CODEGENOPT(Dwarf64   , 1, 0) ///< -gdwarf64.
 CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comments.

ikudrin wrote:
> dblaikie wrote:
> > Is there any precedent to draw from for this flag name? (Does GCC support 
> > DWARF64? Does it support it under this flag name or some other? (similarly 
> > with other gcc-like compilers (Intel's? Whoever else... )))
> It looks like we are pioneering in that area. To me, the proposed name looks 
> consonant with other debug-related switches.
I didn't see any dwarf64 flags in gcc:
https://gcc.gnu.org/onlinedocs/gcc/Option-Summary.html

I tried to follow clang convention for other dwarf flags.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

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


[PATCH] D91565: Guard init_priority attribute within libc++

2020-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:384
 
+def ExcludeTarget : TargetSpec {
+  let CustomCode = [{ !Target.getTriple().isOSzOS() }];

This is not a very descriptive name -- if this is only supposed to be used for 
`init_priority` how about `TargetSupportsInitPriority` instead?

Also, this change is missing test coverage and documentation changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91565

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


[PATCH] D88676: [PPC][AIX] Add vector callee saved registers for AIX extended vector ABI

2020-11-18 Thread Zarko Todorovski via Phabricator via cfe-commits
ZarkoCA added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:235
+return TM.isPPC64()
+   ? (Subtarget.hasAltivec() ? CSR_64_AllRegs_Altivec_RegMask
+ : CSR_PPC64_RegMask)

sfertile wrote:
> `CSR_64_AllRegs_Altivec_RegMask` should be `CSR_PPC64_Altivec_RegMask`.  FWIW 
> I don't think this is testable without D86476. If that's the case, then it 
> should go in that patch, not this patch. 
Are you suggesting that I also leave the error in if I were to move this change 
to D84676? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88676

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


[PATCH] D90507: Adding DWARF64 clang flag

2020-11-18 Thread Alexander Yermolovich via Phabricator via cfe-commits
ayermolo updated this revision to Diff 306187.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -375,3 +375,16 @@
 // RUN:| FileCheck -check-prefix=NO_DEBUG_UNUSED_TYPES %s
 // NO_DEBUG_UNUSED_TYPES: 
"-debug-info-kind={{limited|line-tables-only|standalone}}"
 // NO_DEBUG_UNUSED_TYPES-NOT: "-debug-info-kind=unused-types"
+//
+// RUN: %clang -### -gdwarf-5 -gdwarf64 %s 2>&1 | FileCheck 
-check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 %s 2>&1 | FileCheck 
-check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-3 -gdwarf64 %s 2>&1 | FileCheck 
-check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-2 -gdwarf64 %s 2>&1 | FileCheck 
-check-prefix=GDWARF64_OFF %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 -target x86_64-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 -target i386-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_32ARCH %s
+//
+// GDWARF64_ON:  "-gdwarf64"
+// GDWARF64_OFF:  error: invalid argument '-gdwarf64' only allowed with 
'-gdwarf-5, -gdwarf-4, -gdwarf-3'
+// GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 
bit architecutre'
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1005,6 +1005,7 @@
   Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = !Args.hasArg(OPT_fno_verbose_asm);
+  Opts.Dwarf64 = Args.hasArg(OPT_gdwarf64);
   Opts.PreserveAsmComments = !Args.hasArg(OPT_fno_preserve_as_comments);
   Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);
   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4015,6 +4015,22 @@
   if (DebuggerTuning == llvm::DebuggerKind::SCE)
 CmdArgs.push_back("-dwarf-explicit-import");
 
+  if (Args.hasArg(options::OPT_gdwarf64)) {
+const Arg *A = Args.getLastArg(options::OPT_gdwarf64);
+const llvm::Triple  = TC.getTriple();
+if (DWARFVersion < 3)
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "-gdwarf-5, -gdwarf-4, -gdwarf-3";
+else if (!RawTriple.isArch64Bit())
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "64 bit architecutre";
+else if (!RawTriple.isOSBinFormatELF())
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "elf output format.";
+else
+  CmdArgs.push_back("-gdwarf64");
+  }
+
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);
 }
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -563,6 +563,7 @@
   Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
   Options.MCOptions.MCNoWarn = CodeGenOpts.NoWarn;
   Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose;
+  Options.MCOptions.Dwarf64 = CodeGenOpts.Dwarf64;
   Options.MCOptions.PreserveAsmComments = CodeGenOpts.PreserveAsmComments;
   Options.MCOptions.ABIName = TargetOpts.ABI;
   for (const auto  : HSOpts.UserEntries)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2142,6 +2142,8 @@
   HelpText<"Generate source-level debug information with dwarf version 4">;
 def gdwarf_5 : Flag<["-"], "gdwarf-5">, Group,
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Generate DWARF64 debug information.">;
 
 def gcodeview : Flag<["-"], "gcodeview">,
   HelpText<"Generate CodeView debug information">,
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -32,6 +32,7 @@
 

[PATCH] D90507: Adding DWARF64 clang flag

2020-11-18 Thread Alexander Yermolovich via Phabricator via cfe-commits
ayermolo updated this revision to Diff 306186.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90507

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -374,4 +374,17 @@
 // RUN: %clang -### -fno-eliminate-unused-debug-types -g1 -c %s 2>&1 \
 // RUN:| FileCheck -check-prefix=NO_DEBUG_UNUSED_TYPES %s
 // NO_DEBUG_UNUSED_TYPES: 
"-debug-info-kind={{limited|line-tables-only|standalone}}"
-// NO_DEBUG_UNUSED_TYPES-NOT: "-debug-info-kind=unused-types"
+// NO_DEBUG_U∏NUSED_TYPES-NOT: "-debug-info-kind=unused-types"
+//
+// RUN: %clang -### -gdwarf-5 -gdwarf64 %s 2>&1 | FileCheck 
-check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 %s 2>&1 | FileCheck 
-check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-3 -gdwarf64 %s 2>&1 | FileCheck 
-check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-2 -gdwarf64 %s 2>&1 | FileCheck 
-check-prefix=GDWARF64_OFF %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 -target x86_64-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_ON %s
+// RUN: %clang -### -gdwarf-4 -gdwarf64 -target i386-linux-gnu %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=GDWARF64_32ARCH %s
+//
+// GDWARF64_ON:  "-gdwarf64"
+// GDWARF64_OFF:  error: invalid argument '-gdwarf64' only allowed with 
'-gdwarf-5, -gdwarf-4, -gdwarf-3'
+// GDWARF64_32ARCH: error: invalid argument '-gdwarf64' only allowed with '64 
bit architecutre'
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1005,6 +1005,7 @@
   Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
   Opts.DumpCoverageMapping = Args.hasArg(OPT_dump_coverage_mapping);
   Opts.AsmVerbose = !Args.hasArg(OPT_fno_verbose_asm);
+  Opts.Dwarf64 = Args.hasArg(OPT_gdwarf64);
   Opts.PreserveAsmComments = !Args.hasArg(OPT_fno_preserve_as_comments);
   Opts.AssumeSaneOperatorNew = !Args.hasArg(OPT_fno_assume_sane_operator_new);
   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4015,6 +4015,22 @@
   if (DebuggerTuning == llvm::DebuggerKind::SCE)
 CmdArgs.push_back("-dwarf-explicit-import");
 
+  if (Args.hasArg(options::OPT_gdwarf64)) {
+const Arg *A = Args.getLastArg(options::OPT_gdwarf64);
+const llvm::Triple  = TC.getTriple();
+if (DWARFVersion < 3)
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "-gdwarf-5, -gdwarf-4, -gdwarf-3";
+else if (!RawTriple.isArch64Bit())
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "64 bit architecutre";
+else if (!RawTriple.isOSBinFormatELF())
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << A->getAsString(Args) << "elf output format.";
+else
+  CmdArgs.push_back("-gdwarf64");
+  }
+
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);
 }
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -563,6 +563,7 @@
   Options.MCOptions.MCFatalWarnings = CodeGenOpts.FatalWarnings;
   Options.MCOptions.MCNoWarn = CodeGenOpts.NoWarn;
   Options.MCOptions.AsmVerbose = CodeGenOpts.AsmVerbose;
+  Options.MCOptions.Dwarf64 = CodeGenOpts.Dwarf64;
   Options.MCOptions.PreserveAsmComments = CodeGenOpts.PreserveAsmComments;
   Options.MCOptions.ABIName = TargetOpts.ABI;
   for (const auto  : HSOpts.UserEntries)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2142,6 +2142,8 @@
   HelpText<"Generate source-level debug information with dwarf version 4">;
 def gdwarf_5 : Flag<["-"], "gdwarf-5">, Group,
   HelpText<"Generate source-level debug information with dwarf version 5">;
+def gdwarf64 : Flag<["-"], "gdwarf64">, Group, Flags<[CC1Option]>,
+  HelpText<"Generate DWARF64 debug information.">;
 
 def gcodeview : Flag<["-"], "gcodeview">,
   HelpText<"Generate CodeView debug information">,
Index: clang/include/clang/Basic/CodeGenOptions.def
===

[PATCH] D91565: Guard init_priority attribute within libc++

2020-11-18 Thread Zbigniew Sarbinowski via Phabricator via cfe-commits
zibi updated this revision to Diff 306183.
zibi marked an inline comment as done.
zibi added a comment.

Moving macro to common place as requested. This makes assumption that MS and 
Apple restrictions were lifted at some point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91565

Files:
  clang/include/clang/Basic/Attr.td
  libcxx/include/__config
  libcxx/src/experimental/memory_resource.cpp
  libcxx/src/iostream.cpp


Index: libcxx/src/iostream.cpp
===
--- libcxx/src/iostream.cpp
+++ libcxx/src/iostream.cpp
@@ -77,7 +77,7 @@
 #endif
 ;
 
-_LIBCPP_HIDDEN ios_base::Init __start_std_streams 
__attribute__((init_priority(101)));
+_LIBCPP_HIDDEN ios_base::Init __start_std_streams _LIBCPP_INIT_PRIORITY_MAX;
 
 // On Windows the TLS storage for locales needs to be initialized before we 
create
 // the standard streams, otherwise it may not be alive during program 
termination
Index: libcxx/src/experimental/memory_resource.cpp
===
--- libcxx/src/experimental/memory_resource.cpp
+++ libcxx/src/experimental/memory_resource.cpp
@@ -76,16 +76,6 @@
   ~ResourceInitHelper() {}
 };
 
-// Detect if the init_priority attribute is supported.
-#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
-  || defined(_LIBCPP_COMPILER_MSVC)
-// GCC on Apple doesn't support the init priority attribute,
-// and MSVC doesn't support any GCC attributes.
-# define _LIBCPP_INIT_PRIORITY_MAX
-#else
-# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
-#endif
-
 // When compiled in C++14 this initialization should be a constant expression.
 // Only in C++11 is "init_priority" needed to ensure initialization order.
 #if _LIBCPP_STD_VER > 11
Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -1435,6 +1435,12 @@
 #define _LIBCPP_HAS_NO_FGETPOS_FSETPOS
 #endif
 
+#if __has_attribute(init_priority)
+# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
+#else
+# define _LIBCPP_INIT_PRIORITY_MAX
+#endif
+
 #endif // __cplusplus
 
 #endif // _LIBCPP_CONFIG
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -381,6 +381,9 @@
   let ObjectFormats = ["ELF"];
 }
 
+def ExcludeTarget : TargetSpec {
+  let CustomCode = [{ !Target.getTriple().isOSzOS() }];
+}
 // Attribute subject match rules that are used for #pragma clang attribute.
 //
 // A instance of AttrSubjectMatcherRule represents an individual match rule.
@@ -2221,7 +2224,7 @@
   let Documentation = [Undocumented];
 }
 
-def InitPriority : InheritableAttr {
+def InitPriority : InheritableAttr, TargetSpecificAttr {
   let Spellings = [GCC<"init_priority", /*AllowInC*/0>];
   let Args = [UnsignedArgument<"Priority">];
   let Subjects = SubjectList<[Var], ErrorDiag>;


Index: libcxx/src/iostream.cpp
===
--- libcxx/src/iostream.cpp
+++ libcxx/src/iostream.cpp
@@ -77,7 +77,7 @@
 #endif
 ;
 
-_LIBCPP_HIDDEN ios_base::Init __start_std_streams __attribute__((init_priority(101)));
+_LIBCPP_HIDDEN ios_base::Init __start_std_streams _LIBCPP_INIT_PRIORITY_MAX;
 
 // On Windows the TLS storage for locales needs to be initialized before we create
 // the standard streams, otherwise it may not be alive during program termination
Index: libcxx/src/experimental/memory_resource.cpp
===
--- libcxx/src/experimental/memory_resource.cpp
+++ libcxx/src/experimental/memory_resource.cpp
@@ -76,16 +76,6 @@
   ~ResourceInitHelper() {}
 };
 
-// Detect if the init_priority attribute is supported.
-#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
-  || defined(_LIBCPP_COMPILER_MSVC)
-// GCC on Apple doesn't support the init priority attribute,
-// and MSVC doesn't support any GCC attributes.
-# define _LIBCPP_INIT_PRIORITY_MAX
-#else
-# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
-#endif
-
 // When compiled in C++14 this initialization should be a constant expression.
 // Only in C++11 is "init_priority" needed to ensure initialization order.
 #if _LIBCPP_STD_VER > 11
Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -1435,6 +1435,12 @@
 #define _LIBCPP_HAS_NO_FGETPOS_FSETPOS
 #endif
 
+#if __has_attribute(init_priority)
+# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
+#else
+# define _LIBCPP_INIT_PRIORITY_MAX
+#endif
+
 #endif // __cplusplus
 
 #endif // _LIBCPP_CONFIG
Index: clang/include/clang/Basic/Attr.td

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

2020-11-18 Thread Mircea Trofin via Phabricator via cfe-commits
mtrofin added a comment.

In D91567#2403544 , @aeubanks wrote:

> In D91567#2403252 , @mtrofin wrote:
>
>> In D91567#2403236 , @aeubanks wrote:
>>
>>> One thing that would be nice would be to have both inliners in the same 
>>> CGSCC pass manager to avoid doing SCC construction twice, but that would 
>>> require some shuffling of module/cgscc passes in ModuleInlinerWrapperPass. 
>>> Maybe as a future cleanup.
>>
>> There's that benefit to simplifying the module with the always inliner 
>> before doing inlining "in earnest" I was pointing earlier at: for the ML 
>> policies work, we plan on capturing (sub)graph information. Using the same 
>> SCC would not help because the "higher" (callers) parts of the graph would 
>> have these mandatory inlinings not completed yet, and thus offer a less 
>> accurate picture of the problem space.
>
> Oh I see, caller information is useful.
>
> For compile times: 
> http://llvm-compile-time-tracker.com/?config=O3=instructions=aeubanks.
> The previous version of this patch (perf/npmalways) running a couple passes 
> has some small but measurable overhead on some benchmarks, 0.5%.
> The version of running everything (perf/npmalways2) hugely increases compile 
> times, almost by 50% in one case.

Thanks for doing this! Really good to have this data.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91567

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


[PATCH] D88676: [PPC][AIX] Add vector callee saved registers for AIX extended vector ABI

2020-11-18 Thread Sean Fertile via Phabricator via cfe-commits
sfertile added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:235
+return TM.isPPC64()
+   ? (Subtarget.hasAltivec() ? CSR_64_AllRegs_Altivec_RegMask
+ : CSR_PPC64_RegMask)

`CSR_64_AllRegs_Altivec_RegMask` should be `CSR_PPC64_Altivec_RegMask`.  FWIW I 
don't think this is testable without D86476. If that's the case, then it should 
go in that patch, not this patch. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88676

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


[PATCH] D91442: [clang][Driver] Handle risvc in Baremetal.cpp.

2020-11-18 Thread Hafiz Abid Qadeer via Phabricator via cfe-commits
abidh updated this revision to Diff 306180.
abidh added a comment.
Herald added a subscriber: MaskRay.

This update contains following changes.

1. Address issue raised by lenary. I have introduced a static function that 
checks for the presence of gcc toolchain first by --gcc-toolchain argument or 
in the same prefix where clang is installed. If found, RISCVToolChain is 
instantiated. I have added tests in riscv64-toolchain-extra.c and 
riscv32-toolchain-extra.c to check this case.

2. Fixed some formatting things pointed out by jrtc27.

3. In the initial revision, jroelofs commented that other place where Baremetal 
is intantiated should have the similar check. I put that in 2nd version of the 
patch. But I think it does quite make sense in the current patch so I have 
removed it.

4. Rebased baremetal.cpp tests after recent changes and fixed some typos.


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

https://reviews.llvm.org/D91442

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.h
  clang/test/Driver/baremetal.cpp
  clang/test/Driver/riscv-gnutools.c
  clang/test/Driver/riscv32-toolchain-extra.c
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain-extra.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -1,11 +1,15 @@
 // A basic clang -cc1 command-line, and simple environment check.
 // REQUIRES: platform-linker
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv64"
 
 // Test interaction with -fuse-ld=lld, if lld is available.
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld 2>&1 | FileCheck -check-prefix=LLD %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 -fuse-ld=lld \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=LLD %s
 // LLD: {{(error: invalid linker name in argument '-fuse-ld=lld')|(ld.lld)}}
 
 // In the below tests, --rtlib=platform is used so that the driver ignores
@@ -133,6 +137,7 @@
 
 // Check that --rtlib can be used to override the used runtime library
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv64-unknown-elf --rtlib=libgcc 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-RTLIB-LIBGCC-LP64 %s
 // C-RV64-RTLIB-LIBGCC-LP64: "{{.*}}crt0.o"
@@ -141,6 +146,7 @@
 // C-RV64-RTLIB-LIBGCC-LP64: "{{.*}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_elf_sdk \
 // RUN:   -target riscv64-unknown-elf --rtlib=compiler-rt 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-RTLIB-COMPILERRT-LP64 %s
 // C-RV64-RTLIB-COMPILERRT-LP64: "{{.*}}crt0.o"
Index: clang/test/Driver/riscv64-toolchain-extra.c
===
--- clang/test/Driver/riscv64-toolchain-extra.c
+++ clang/test/Driver/riscv64-toolchain-extra.c
@@ -22,6 +22,10 @@
 // RUN:-target riscv64-unknown-elf --rtlib=platform 2>&1 \
 // RUN:| FileCheck -check-prefix=C-RV64-BAREMETAL-LP64-NOGCC %s
 
+// RUN: %T/testroot-riscv64-baremetal-nogcc/bin/clang %s -### -no-canonical-prefixes \
+// RUN:-target riscv64-unknown-elf --rtlib=platform 2>&1 \
+// RUN:| FileCheck -check-prefix=C-RV64-BAREMETAL-LP64-NOGCC %s
+
 // C-RV64-BAREMETAL-LP64-NOGCC: "-internal-isystem" "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/../riscv64-unknown-elf/include"
 // C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/riscv64-unknown-elf-ld"
 // C-RV64-BAREMETAL-LP64-NOGCC: "{{.*}}Output/testroot-riscv64-baremetal-nogcc/bin/../riscv64-unknown-elf/lib/crt0.o"
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -1,11 +1,15 @@
 // A basic clang -cc1 command-line, and simple environment check.
 // REQUIRES: platform-linker
 
-// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clang %s -### -no-canonical-prefixes -target riscv32 \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "riscv32"
 
 // Test interaction with -fuse-ld=lld, if lld is available.
-// RUN: %clang %s -### -no-canonical-prefixes -target 

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

2020-11-18 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment.

Thanks for adding the Driver test. I was thinking of something to test the 
CompilerInvocation changes, similar to your test using opt, that ensures the 
option has the desired behavior when invoked via clang. Looks like there is an 
existing test clang/test/Frontend/optimization-remark-with-hotness.c that 
perhaps could be extended or leveraged?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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


[PATCH] D91495: [clang-tidy] false-positive for bugprone-redundant-branch-condition in case of passed-by-ref params

2020-11-18 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:968
+  }
+}
+

aaron.ballman wrote:
> Another test that would be interesting is:
> ```
> if (tryToExtinguish(isSet) && isSet) {
>   if (tryToExtinguishByVal(isSet) && isSet) { // Dupe check of isSet should 
> be diagnosed
> scream();
>   }
> }
> 
> if (tryToExtinguishByVal(isSet) && isSet) {
>   if (tryToExtinguish(isSet) && isSet) { // Ok
> scream();
>   }
> }
> ```
Nice catch! Thanks. Looks like isChangedBefore is too rough and must be refined



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91495

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


[PATCH] D86853: [modules] Fix crash in call to `FunctionDecl::setPure()`

2020-11-18 Thread Xun Li via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc6c8d4a13ebd: [modules] Fix crash in call to 
`FunctionDecl::setPure()` (authored by andrewjcg, committed by lxfind).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86853

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/set-pure-crash/a.h
  clang/test/Modules/Inputs/set-pure-crash/b.h
  clang/test/Modules/Inputs/set-pure-crash/c.h
  clang/test/Modules/Inputs/set-pure-crash/module.modulemap
  clang/test/Modules/set-pure-crash.cpp

Index: clang/test/Modules/set-pure-crash.cpp
===
--- /dev/null
+++ clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = simple();
Index: clang/test/Modules/Inputs/set-pure-crash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+  header "a.h"
+}
+
+module b {
+  header "b.h"
+}
+
+module c {
+  header "c.h"
+}
Index: clang/test/Modules/Inputs/set-pure-crash/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template 
+struct simple {
+};
Index: clang/test/Modules/Inputs/set-pure-crash/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+  void func() {
+foo([]() {});
+  }
+};
Index: clang/test/Modules/Inputs/set-pure-crash/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template 
+class Base {
+public:
+  virtual void func() = 0;
+};
+
+Base bar();
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -868,7 +868,10 @@
   FD->setInlineSpecified(Record.readInt());
   FD->setImplicitlyInline(Record.readInt());
   FD->setVirtualAsWritten(Record.readInt());
-  FD->setPure(Record.readInt());
+  // We defer calling `FunctionDecl::setPure()` here as for methods of
+  // `CXXTemplateSpecializationDecl`s, we may not have connected up the
+  // definition (which is required for `setPure`).
+  const bool Pure = Record.readInt();
   FD->setHasInheritedPrototype(Record.readInt());
   FD->setHasWrittenPrototype(Record.readInt());
   FD->setDeletedAsWritten(Record.readInt());
@@ -1015,6 +1018,10 @@
   }
   }
 
+  // Defer calling `setPure` until merging above has guaranteed we've set
+  // `DefinitionData` (as this will need to access it).
+  FD->setPure(Pure);
+
   // Read in the parameters.
   unsigned NumParams = Record.readInt();
   SmallVector Params;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] c6c8d4a - [modules] Fix crash in call to `FunctionDecl::setPure()`

2020-11-18 Thread Xun Li via cfe-commits

Author: Andrew Gallagher
Date: 2020-11-18T11:55:29-08:00
New Revision: c6c8d4a13ebd5ce1c3c7e8632312ab8c2dc6afa0

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

LOG: [modules] Fix crash in call to `FunctionDecl::setPure()`

In some cases, when deserializing a `CXXMethodDecl` of a 
`CXXSpecializationTemplateDecl`,
the call to `FunctionDecl::setPure()` happens before the `DefinitionData` 
member has been
populated (which appears to happen lower down in a `mergeRedeclarable` call), 
causing a
crash (https://reviews.llvm.org/P8228).

This diff fixes this by deferring the `FunctionDecl::setPure()` till after the 
`DefinitionData` has
been filled in.

Reviewed By: lxfind

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

Added: 
clang/test/Modules/Inputs/set-pure-crash/a.h
clang/test/Modules/Inputs/set-pure-crash/b.h
clang/test/Modules/Inputs/set-pure-crash/c.h
clang/test/Modules/Inputs/set-pure-crash/module.modulemap
clang/test/Modules/set-pure-crash.cpp

Modified: 
clang/lib/Serialization/ASTReaderDecl.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 797232885687..6bfb9bd783b5 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -868,7 +868,10 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
   FD->setInlineSpecified(Record.readInt());
   FD->setImplicitlyInline(Record.readInt());
   FD->setVirtualAsWritten(Record.readInt());
-  FD->setPure(Record.readInt());
+  // We defer calling `FunctionDecl::setPure()` here as for methods of
+  // `CXXTemplateSpecializationDecl`s, we may not have connected up the
+  // definition (which is required for `setPure`).
+  const bool Pure = Record.readInt();
   FD->setHasInheritedPrototype(Record.readInt());
   FD->setHasWrittenPrototype(Record.readInt());
   FD->setDeletedAsWritten(Record.readInt());
@@ -1015,6 +1018,10 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
   }
   }
 
+  // Defer calling `setPure` until merging above has guaranteed we've set
+  // `DefinitionData` (as this will need to access it).
+  FD->setPure(Pure);
+
   // Read in the parameters.
   unsigned NumParams = Record.readInt();
   SmallVector Params;

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/a.h 
b/clang/test/Modules/Inputs/set-pure-crash/a.h
new file mode 100644
index ..f52458e0daeb
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template 
+class Base {
+public:
+  virtual void func() = 0;
+};
+
+Base bar();

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/b.h 
b/clang/test/Modules/Inputs/set-pure-crash/b.h
new file mode 100644
index ..eef7c2b9faaa
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+  void func() {
+foo([]() {});
+  }
+};

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/c.h 
b/clang/test/Modules/Inputs/set-pure-crash/c.h
new file mode 100644
index ..d5b7cd19461f
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template 
+struct simple {
+};

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/module.modulemap 
b/clang/test/Modules/Inputs/set-pure-crash/module.modulemap
new file mode 100644
index ..50bbe84e5d07
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+  header "a.h"
+}
+
+module b {
+  header "b.h"
+}
+
+module c {
+  header "c.h"
+}

diff  --git a/clang/test/Modules/set-pure-crash.cpp 
b/clang/test/Modules/set-pure-crash.cpp
new file mode 100644
index ..197e0cb00d27
--- /dev/null
+++ b/clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = simple();



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


[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3938
+  if ((FLAGS)::CC1Option) {
\
+const auto  = EXTRACTOR(this->KEYPATH);  
\
+if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE) 
\

dexonsmith wrote:
> jansvoboda11 wrote:
> > Bigcheese wrote:
> > > Will this ever have an issue with lifetime? I can see various values for 
> > > `EXTRACTOR` causing issues here. https://abseil.io/tips/107
> > > 
> > > 
> > > It would be good to at least document somewhere the restrictions on 
> > > `EXTRACTOR`.
> > Mentioned the reference lifetime extension in a comment near extractor 
> > definitions.
> It might be safer to refactor as:
> ```
> // Capture the extracted value as a lambda argument to avoid potential
> // lifetime extension issues.
> [&](const auto ) {
>   if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))
> DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);
> }(EXTRACTOR(this->KEYPATH));
> ```
> 
Might be even better to avoid the generic lambda:
```
// Capture the extracted value as a lambda argument to avoid potential
// lifetime extension issues.
using ExtractedType =
std::remove_const_tKEYPATH))>>
[&](const ExtractedType ) {
  if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))
DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);
}(EXTRACTOR(this->KEYPATH));
```
(since generic vs. non-generic could affect compile-time of 
CompilerInvocation.cpp given how many instances there will be).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83211

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


[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

2020-11-18 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D91567#2403252 , @mtrofin wrote:

> In D91567#2403236 , @aeubanks wrote:
>
>> One thing that would be nice would be to have both inliners in the same 
>> CGSCC pass manager to avoid doing SCC construction twice, but that would 
>> require some shuffling of module/cgscc passes in ModuleInlinerWrapperPass. 
>> Maybe as a future cleanup.
>
> There's that benefit to simplifying the module with the always inliner before 
> doing inlining "in earnest" I was pointing earlier at: for the ML policies 
> work, we plan on capturing (sub)graph information. Using the same SCC would 
> not help because the "higher" (callers) parts of the graph would have these 
> mandatory inlinings not completed yet, and thus offer a less accurate picture 
> of the problem space.

Oh I see, caller information is useful.

For compile times: 
http://llvm-compile-time-tracker.com/?config=O3=instructions=aeubanks.
The previous version of this patch (perf/npmalways) running a couple passes has 
some small but measurable overhead on some benchmarks, 0.5%.
The version of running everything (perf/npmalways2) hugely increases compile 
times, almost by 50% in one case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91567

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


[PATCH] D91625: [clang] Do not crash on pointer wchar_t pointer assignment.

2020-11-18 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91625

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


[PATCH] D72184: [BPF] support atomic instructions

2020-11-18 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 306172.
yonghong-song edited the summary of this revision.
yonghong-song added a comment.

add support for a barrier function. The correspond C code is 
__sync_synchronize().
If we want to have variant like barrier for read/write/rw, etc. we may need to 
define bpf specific barrier functions as __sync_synchronize() does not allow 
taking additional parameters. I tentatively use one particular encoding but we 
can certainly change that later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

Files:
  clang/lib/Basic/Targets/BPF.cpp
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/lib/Target/BPF/BPF.td
  llvm/lib/Target/BPF/BPFInstrFormats.td
  llvm/lib/Target/BPF/BPFInstrInfo.td
  llvm/lib/Target/BPF/BPFSubtarget.cpp
  llvm/lib/Target/BPF/BPFSubtarget.h
  llvm/lib/Target/BPF/Disassembler/BPFDisassembler.cpp
  llvm/lib/Target/BPF/MCTargetDesc/BPFMCCodeEmitter.cpp
  llvm/test/CodeGen/BPF/atomics.ll
  llvm/test/CodeGen/BPF/atomics_2.ll

Index: llvm/test/CodeGen/BPF/atomics_2.ll
===
--- /dev/null
+++ llvm/test/CodeGen/BPF/atomics_2.ll
@@ -0,0 +1,242 @@
+; RUN: llc < %s -march=bpfel -verify-machineinstrs -show-mc-encoding -mcpu=v4 | FileCheck %s
+;
+; Source:
+;   char test_load_sub_8(char *p, char v) {
+; return __sync_fetch_and_sub(p, v);
+;   }
+;   short test_load_sub_16(short *p, short v) {
+; return __sync_fetch_and_sub(p, v);
+;   }
+;   int test_load_sub_32(int *p, int v) {
+; return __sync_fetch_and_sub(p, v);
+;   }
+;   int test_load_sub_64(long *p, long v) {
+; return __sync_fetch_and_sub(p, v);
+;   }
+;   int test_xchg_8(char *p, char v) {
+; return __sync_lock_test_and_set(p, v);
+;   }
+;   int test_xchg_16(short *p, short v) {
+; return __sync_lock_test_and_set(p, v);
+;   }
+;   int test_xchg_32(int *p, int v) {
+; return __sync_lock_test_and_set(p, v);
+;   }
+;   int test_xchg_64(long *p, long v) {
+; return __sync_lock_test_and_set(p, v);
+;   }
+;   int test_cas_32(int *p, int old, int new) {
+; return __sync_val_compare_and_swap(p, old, new);
+;   }
+;   long test_cas_64(long *p, long old, long new) {
+; return __sync_val_compare_and_swap(p, old, new);
+;   }
+;   int test_load_and_32(int *p, int v) {
+; return __sync_fetch_and_and(p, v);
+;   }
+;   int test_load_and_64(long *p, long v) {
+; return __sync_fetch_and_and(p, v);
+;   }
+;   int test_load_or_32(int *p, int v) {
+; return __sync_fetch_and_or(p, v);
+;   }
+;   int test_load_or_64(long *p, long v) {
+; return __sync_fetch_and_or(p, v);
+;   }
+;   int test_load_xor_32(int *p, int v) {
+; return __sync_fetch_and_xor(p, v);
+;   }
+;   int test_load_xor_64(long *p, long v) {
+; return __sync_fetch_and_xor(p, v);
+;   }
+;   int test_sync(int *p, int *q)
+;   {
+; *p = 1;
+; __sync_synchronize();
+; *q = 2;
+; return 0;
+;   }
+
+; CHECK-LABEL: test_load_sub_8
+; CHECK: w0 = w2
+; CHECK: w0 = atomic_fetch_sub((u8 *)(r1 + 0), w0)
+; CHECK: encoding: [0xd3,0x01,0x00,0x00,0x11,0x00,0x00,0x00]
+define dso_local signext i8 @test_load_sub_8(i8* nocapture %p, i8 signext %v) local_unnamed_addr #0 {
+entry:
+  %0 = atomicrmw sub i8* %p, i8 %v seq_cst
+  ret i8 %0
+}
+
+; CHECK-LABEL: test_load_sub_16
+; CHECK: w0 = w2
+; CHECK: w0 = atomic_fetch_sub((u16 *)(r1 + 0), w0)
+; CHECK: encoding: [0xcb,0x01,0x00,0x00,0x11,0x00,0x00,0x00]
+define dso_local signext i16 @test_load_sub_16(i16* nocapture %p, i16 signext %v) local_unnamed_addr #0 {
+entry:
+  %0 = atomicrmw sub i16* %p, i16 %v seq_cst
+  ret i16 %0
+}
+
+; CHECK-LABEL: test_load_sub_32
+; CHECK: w0 = w2
+; CHECK: w0 = atomic_fetch_sub((u32 *)(r1 + 0), w0)
+; CHECK: encoding: [0xc3,0x01,0x00,0x00,0x11,0x00,0x00,0x00]
+define dso_local i32 @test_load_sub_32(i32* nocapture %p, i32 %v) local_unnamed_addr {
+entry:
+  %0 = atomicrmw sub i32* %p, i32 %v seq_cst
+  ret i32 %0
+}
+
+; CHECK-LABEL: test_load_sub_64
+; CHECK: r0 = r2
+; CHECK: r0 = atomic_fetch_sub((u64 *)(r1 + 0), r0)
+; CHECK: encoding: [0xdb,0x01,0x00,0x00,0x11,0x00,0x00,0x00]
+define dso_local i32 @test_load_sub_64(i64* nocapture %p, i64 %v) local_unnamed_addr {
+entry:
+  %0 = atomicrmw sub i64* %p, i64 %v seq_cst
+  %conv = trunc i64 %0 to i32
+  ret i32 %conv
+}
+
+; CHECK-LABEL: test_xchg_8
+; CHECK: w0 = w2
+; CHECK: w0 = xchg32_8(r1 + 0, w0)
+; CHECK: encoding: [0xd3,0x01,0x00,0x00,0xe1,0x00,0x00,0x00]
+define dso_local i32 @test_xchg_8(i8* nocapture %p, i8 signext %v) local_unnamed_addr {
+entry:
+  %0 = atomicrmw xchg i8* %p, i8 %v seq_cst
+  %conv = sext i8 %0 to i32
+  ret i32 %conv
+}
+
+; CHECK-LABEL: test_xchg_16
+; CHECK: w0 = w2
+; CHECK: w0 = xchg32_16(r1 + 0, w0)
+; CHECK: encoding: [0xcb,0x01,0x00,0x00,0xe1,0x00,0x00,0x00]
+define dso_local i32 @test_xchg_16(i16* nocapture %p, i16 signext %v) local_unnamed_addr {
+entry:
+  %0 = 

[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4062
+  if ((FLAGS)::CC1Option) {
\
+const auto  = EXTRACTOR(this->KEYPATH);  
\
+if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))   
\

IIUC, then we can do this more simply as:
```
bool Extracted = EXTRACTOR(this->KEYPATH);
```
That might clarify why we don't have lifetime extension concerns here.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3938
+  if ((FLAGS)::CC1Option) {
\
+const auto  = EXTRACTOR(this->KEYPATH);  
\
+if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE) 
\

jansvoboda11 wrote:
> Bigcheese wrote:
> > Will this ever have an issue with lifetime? I can see various values for 
> > `EXTRACTOR` causing issues here. https://abseil.io/tips/107
> > 
> > 
> > It would be good to at least document somewhere the restrictions on 
> > `EXTRACTOR`.
> Mentioned the reference lifetime extension in a comment near extractor 
> definitions.
It might be safer to refactor as:
```
// Capture the extracted value as a lambda argument to avoid potential
// lifetime extension issues.
[&](const auto ) {
  if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))
DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);
}(EXTRACTOR(this->KEYPATH));
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83211

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


[PATCH] D91565: Guard init_priority attribute within libc++

2020-11-18 Thread Louis Dionne via Phabricator via cfe-commits
ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.



Comment at: libcxx/include/experimental/__config:80
+// Detect if the init_priority attribute is supported.
+#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
+  || defined(_LIBCPP_COMPILER_MSVC) || defined(__MVS__)

Just remove that entirely and use the one from the main `<__config>` file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91565

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


[PATCH] D91673: [PGO] Enable preinline and cleanup when optimize for size

2020-11-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

Zequan, to build clang with PGO, you can follow the steps in Chrome's script to 
build clang with PGO:
https://source.chromium.org/chromium/chromium/src/+/master:tools/clang/scripts/build.py;l=703?q=clang%2Fscripts%2F%20build.py=chromium

Regarding -Oz / minsize / SizeLevel 2, what we have discovered is that *not* 
running the preinliner makes the instrumented binary almost unusably large. 
Even if 75 is the wrong inline threshold for minsize, it's better than not 
running the preinliner. I think we should run with it.




Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:334
   // We will not do this inline for context sensitive PGO (when IsCS is true).
-  if (OptLevel > 0 && SizeLevel == 0 && !DisablePreInliner &&
-  PGOSampleUse.empty() && !IsCS) {
+  if (OptLevel > 0 && !DisablePreInliner && PGOSampleUse.empty() && !IsCS) {
 // Create preinline pass. We construct an InlineParams object and specify

This code is duplicated for the NPM and the old PM. Please keep them in sync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91673

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


[PATCH] D83691: Port Comment option flags to new parsing system

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM after you fix a couple of nits.




Comment at: clang/include/clang/Driver/Options.td:405
 
+// Comment Options
+

Nit: please end the comment with a period to make this a sentence; I'm not sure 
if "Options" is really a proper noun; I would have thought "Comment options.".



Comment at: clang/include/clang/Driver/Options.td:407
+
+def fparse_all_comments : Flag<["-"], "fparse-all-comments">, 
Group, Flags<[CC1Option]>,
+  MarshallingInfoFlag<"LangOpts->CommentOpts.ParseAllComments">;

Nit: is moving this option and adding a comment a necessary part of the patch, 
or is it just a cleanup that could be done before in an NFC commit? If it's 
possible, I would suggest moving it in a separate commit.



Comment at: clang/include/clang/Driver/Options.td:410
+
 // Standard Options
 

Might be nice to clean up this comment in a separate NFC commit ahead of time, 
to match what you do with "Comment Options".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83691

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


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

2020-11-18 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/include/clang/AST/StmtDataCollectors.td:67-75
+if (auto *ParamTypeDecl = dyn_cast(TemplParam)) {
+  const clang::Type *ParamType = ParamTypeDecl->getTypeForDecl();
+  if (auto *autoT = ParamType->getAs()) {
+if (autoT->getAs()) {
+  IncludeType = true;
+}
+  } else if (ParamType->getAs()) {

This case can't happen. The only way that a template parameter can be a 
`TypeDecl` is if it's a `TemplateTypeParmDecl`, which represents ` 
TemplateTypeParmType`, never a `DeducedType`.



Comment at: clang/include/clang/AST/StmtDataCollectors.td:76
+  }
+} else if (auto *ParamValueDecl = dyn_cast(TemplParam)) 
{
+  const clang::Type *ParamType = 
ParamValueDecl->getType().getTypePtr();

Check directly for a `NonTypeTemplateParamDecl` here.



Comment at: clang/include/clang/AST/StmtDataCollectors.td:77
+} else if (auto *ParamValueDecl = dyn_cast(TemplParam)) 
{
+  const clang::Type *ParamType = 
ParamValueDecl->getType().getTypePtr();
+  if (auto *autoT = ParamType->getAs()) {

Don't use `getTypePtr()` here, just use `QualType`'s `operator->`. Eg: 
`IncludeType = ParamValueDecl->getType()->getContainedDeducedType();`.



Comment at: clang/include/clang/AST/StmtDataCollectors.td:78-84
+  if (auto *autoT = ParamType->getAs()) {
+if (autoT->getAs()) {
+  IncludeType = true;
+}
+  } else if (ParamType->getAs()) {
+IncludeType = true;
+  }

You don't need to check for both of these; `AutoType` derives from 
`DeducedType`. Also, you're only looking for a top-level deduced type here; use 
`getContainedDeducedType()` instead to properly handle `auto*` and the like.



Comment at: clang/lib/AST/ASTTypeTraits.cpp:131-135
+  if (const TemplateArgument *TA = get()) {
+TA->print(PP, OS, /*IncludeType*/ true);
+  } else if (const TemplateArgumentLoc *TAL = get()) {
+TAL->getArgument().print(PP, OS, /*IncludeType*/ true);
+  } else if (const TemplateName *TN = get())

No braces here, please.



Comment at: clang/lib/AST/Decl.cpp:1630
   const TemplateArgumentList  = Spec->getTemplateArgs();
+  // FIXME: Only pass template parameter list if template isn't overloaded
   printTemplateArgumentList(

Class templates are never overloaded.



Comment at: clang/lib/AST/Decl.cpp:2859
+  TPL = TD->getTemplateParameters();
+// FIXME: Only pass template parameter list if template isn't overloaded
+printTemplateArgumentList(OS, TemplateArgs->asArray(), Policy, TPL);

We don't know in this case, so I think if we're not going to check, then we 
shouldn't pass the parameter list. This change as-is can lead to us printing 
out ambiguous names for function templates, both due to argument types and due 
to omitting arguments equivalent to default arguments.



Comment at: clang/lib/AST/NestedNameSpecifier.cpp:291
 Record->printName(OS);
-printTemplateArgumentList(OS, Record->getTemplateArgs().asArray(),
-  Policy);
+// FIXME: Only pass parameter list if function template is overloaded
+printTemplateArgumentList(

This is a class template, not a function template, so can't be overloaded. 
(Also I think this comment is backwards.)



Comment at: clang/lib/AST/NestedNameSpecifier.cpp:321
+TPL = TD->getTemplateParameters();
+  // FIXME: Only pass parameter list if template is overloaded
+  printTemplateArgumentList(OS, SpecType->template_arguments(), 
InnerPolicy,

Type templates can't be overloaded. We need the types if and only if we didn't 
resolve the template name to a specific template decl, which matches what this 
patch does, so you can just remove the FIXMEs.



Comment at: clang/lib/AST/NestedNameSpecifier.cpp:330-332
+  const TemplateParameterList *TPL = nullptr;
+  if (auto *TD = dyn_cast(Record))
+TPL = TD->getTemplateParameters();

I think `Record` will always be null here, so this `dyn_cast` will crash. For a 
dependent template specialization type, I don't think there's any situation in 
which we can know what the template parameter list is, so we should always 
include the types for template arguments (that is, pass `nullptr` as the 
template parameter list).



Comment at: clang/lib/AST/StmtPrinter.cpp:999
+  const TemplateParameterList *TPL = nullptr;
+  // FIXME: Get list of corresponding template parameters
   if (Node->hasExplicitTemplateArgs())

I 

[PATCH] D17993: [CodeGen] Apply 'nonnull' and 'dereferenceable(N)' to 'this' pointer arguments.

2020-11-18 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2169
+if (!CodeGenOpts.NullPointerIsValid &&
+getContext().getTargetAddressSpace(FI.arg_begin()->type) == 0) {
+  Attrs.addAttribute(llvm::Attribute::NonNull);

arichardson wrote:
> rjmccall wrote:
> > rsmith wrote:
> > > jdoerfert wrote:
> > > > rjmccall wrote:
> > > > > rsmith wrote:
> > > > > > jdoerfert wrote:
> > > > > > > arichardson wrote:
> > > > > > > > Isn't the `this` pointer also nonnull in other address spaces?
> > > > > > > > 
> > > > > > > > In our CHERI fork we use AS200 for the this pointer and would 
> > > > > > > > quite like to have the nonnull attribute.
> > > > > > > > I can obviously change this line locally when I next merge from 
> > > > > > > > upstream, but I would like to avoid diffs and it seems to me 
> > > > > > > > like this restriction is unnecessary.
> > > > > > > I also think `NullPointerIsValid` is sufficient. 
> > > > > > It's my understanding that:
> > > > > > * The LLVM `null` value in any address space is the all-zero-bits 
> > > > > > value.
> > > > > > * In address space zero, the `null` value does not correspond to 
> > > > > > addressable memory, but this is not assumed to hold in other 
> > > > > > address spaces.
> > > > > > * An address-space-zero `null` value that is addressspacecast to a 
> > > > > > different address space might not be the `null` in the target 
> > > > > > address space.
> > > > > > * The `nonnull` attribute implies that the pointer value is not the 
> > > > > > `null` value.
> > > > > > * A null pointer in the frontend in a non-zero address space 
> > > > > > corresponds to the value produced by an addressspacecast of an 
> > > > > > address-space-zero `null` value to the target address space.
> > > > > > 
> > > > > > That being the case, there is simply no connection between the C 
> > > > > > and C++ notion of a null pointer and a `null` LLVM pointer value in 
> > > > > > a non-zero address space in general, so it is not correct to use 
> > > > > > the `nonnull` attribute in a non-zero address space in general. 
> > > > > > Only if we know that a C++ null pointer is actually represented by 
> > > > > > the LLVM `null` value in the corresponding address space can we use 
> > > > > > the `nonnull` attribute to expose that fact to LLVM. And we do not 
> > > > > > know that in general.
> > > > > I think all of this is correct except that the frontend does know 
> > > > > what the bit-pattern of the null pointer is in any particular 
> > > > > language-level address space, and it knows what the language-level 
> > > > > address space of `this` is.  So we should be able to ask whether the 
> > > > > null value in the `this` address space is the all-zero value and use 
> > > > > that to decide whether to apply `nonnull`.
> > > > Hm, I think the problem is that we don't couple `NullPointerIsValid` 
> > > > with the address space. As I said before. In LLVM-IR, if we don't have 
> > > > the `null-pointer-is-valid` property, then "memory access" implies 
> > > > `dereferenceable` implies `nonnull`. Now, we usually assume non-0 
> > > > address spaces to have the above property, but that should be 
> > > > decoupled. The query if "null is valid" should take the function and 
> > > > the AS, as it does conceptually in LLVM-core, and then decide if 
> > > > `null-pointer-is-valid` or not. If we had that, @arichardson could 
> > > > define that AS200 does not have valid null pointers. If you do that 
> > > > right now the IR passes will at least deduce `nonnull` based on 
> > > > `derferenceable`.
> > > > I think all of this is correct except that the frontend does know what 
> > > > the bit-pattern of the null pointer is in any particular language-level 
> > > > address space
> > > 
> > > Interesting. How do we get at that? Do we ask the middle-end to 
> > > constant-fold `addrspacecast i8* null to i8* addrspace(N)` and see if we 
> > > get a `null` back, or is there a better way?
> > > 
> > > In any case, this patch follows the same pattern we use for return values 
> > > of reference type, parameters of reference type, and decayed array 
> > > function parameters with static array bounds, all of which apply 
> > > `nonnull` only in address space 0. If we want to use a different pattern, 
> > > to consider whether LLVM's `nonnull` means "not a null pointer" rather 
> > > than assuming that holds only in address space 0, we should make a 
> > > holistic change for that throughout CGCall.cpp, rather than touching only 
> > > the handling of the `this` pointer.
> > > 
> > > It'd also be interesting to consider what we want 
> > > `__attribute__((nonnull))` to mean in address spaces where the null 
> > > pointer isn't the zero pointer: should it mean non-zero at the source 
> > > level / non-null in LLVM IR, or should it mean non-null at the source 
> > > level (which might be unrepresentable in LLVM IR, but static analysis 
> > > 

[PATCH] D83693: Port analyzer flags to new option parsing system

2020-11-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

This LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83693

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


[PATCH] D91565: Guard init_priority attribute within libc++

2020-11-18 Thread Zbigniew Sarbinowski via Phabricator via cfe-commits
zibi updated this revision to Diff 306162.
zibi marked an inline comment as done.
zibi added a comment.
Herald added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I moved the logic from `memory_resource.cpp` to `__config` as since I'm not 
familiar with Apple and Microsoft restrictions are still correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91565

Files:
  clang/include/clang/Basic/Attr.td
  libcxx/include/__config
  libcxx/include/experimental/__config
  libcxx/src/experimental/memory_resource.cpp
  libcxx/src/iostream.cpp


Index: libcxx/src/iostream.cpp
===
--- libcxx/src/iostream.cpp
+++ libcxx/src/iostream.cpp
@@ -77,7 +77,7 @@
 #endif
 ;
 
-_LIBCPP_HIDDEN ios_base::Init __start_std_streams 
__attribute__((init_priority(101)));
+_LIBCPP_HIDDEN ios_base::Init __start_std_streams _LIBCPP_INIT_PRIORITY_MAX;
 
 // On Windows the TLS storage for locales needs to be initialized before we 
create
 // the standard streams, otherwise it may not be alive during program 
termination
Index: libcxx/src/experimental/memory_resource.cpp
===
--- libcxx/src/experimental/memory_resource.cpp
+++ libcxx/src/experimental/memory_resource.cpp
@@ -76,16 +76,6 @@
   ~ResourceInitHelper() {}
 };
 
-// Detect if the init_priority attribute is supported.
-#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
-  || defined(_LIBCPP_COMPILER_MSVC)
-// GCC on Apple doesn't support the init priority attribute,
-// and MSVC doesn't support any GCC attributes.
-# define _LIBCPP_INIT_PRIORITY_MAX
-#else
-# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
-#endif
-
 // When compiled in C++14 this initialization should be a constant expression.
 // Only in C++11 is "init_priority" needed to ensure initialization order.
 #if _LIBCPP_STD_VER > 11
Index: libcxx/include/experimental/__config
===
--- libcxx/include/experimental/__config
+++ libcxx/include/experimental/__config
@@ -76,4 +76,14 @@
 #define _LIBCPP_NATIVE_SIMD_WIDTH_IN_BYTES 16
 #endif
 
+// Detect if the init_priority attribute is supported.
+#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
+  || defined(_LIBCPP_COMPILER_MSVC) || defined(__MVS__)
+// GCC on Apple doesn't support the init priority
+// attribute, same for MSVC and z/OS.
+# define _LIBCPP_INIT_PRIORITY_MAX
+#else
+# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
+#endif
+
 #endif
Index: libcxx/include/__config
===
--- libcxx/include/__config
+++ libcxx/include/__config
@@ -1435,6 +1435,12 @@
 #define _LIBCPP_HAS_NO_FGETPOS_FSETPOS
 #endif
 
+#if __has_attribute(init_priority)
+# define _LIBCPP_INIT_PRIORITY_MAX __attribute__((init_priority(101)))
+#else
+# define _LIBCPP_INIT_PRIORITY_MAX
+#endif
+
 #endif // __cplusplus
 
 #endif // _LIBCPP_CONFIG
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -381,6 +381,9 @@
   let ObjectFormats = ["ELF"];
 }
 
+def ExcludeTarget : TargetSpec {
+  let CustomCode = [{ !Target.getTriple().isOSzOS() }];
+}
 // Attribute subject match rules that are used for #pragma clang attribute.
 //
 // A instance of AttrSubjectMatcherRule represents an individual match rule.
@@ -2221,7 +2224,7 @@
   let Documentation = [Undocumented];
 }
 
-def InitPriority : InheritableAttr {
+def InitPriority : InheritableAttr, TargetSpecificAttr {
   let Spellings = [GCC<"init_priority", /*AllowInC*/0>];
   let Args = [UnsignedArgument<"Priority">];
   let Subjects = SubjectList<[Var], ErrorDiag>;


Index: libcxx/src/iostream.cpp
===
--- libcxx/src/iostream.cpp
+++ libcxx/src/iostream.cpp
@@ -77,7 +77,7 @@
 #endif
 ;
 
-_LIBCPP_HIDDEN ios_base::Init __start_std_streams __attribute__((init_priority(101)));
+_LIBCPP_HIDDEN ios_base::Init __start_std_streams _LIBCPP_INIT_PRIORITY_MAX;
 
 // On Windows the TLS storage for locales needs to be initialized before we create
 // the standard streams, otherwise it may not be alive during program termination
Index: libcxx/src/experimental/memory_resource.cpp
===
--- libcxx/src/experimental/memory_resource.cpp
+++ libcxx/src/experimental/memory_resource.cpp
@@ -76,16 +76,6 @@
   ~ResourceInitHelper() {}
 };
 
-// Detect if the init_priority attribute is supported.
-#if (defined(_LIBCPP_COMPILER_GCC) && defined(__APPLE__)) \
-  || defined(_LIBCPP_COMPILER_MSVC)
-// GCC on Apple doesn't support the init priority attribute,
-// and MSVC doesn't support 

[PATCH] D86502: [CSSPGO] A Clang switch -fpseudo-probe-for-profiling for pseudo-probe instrumentation.

2020-11-18 Thread Hongtao Yu via Phabricator via cfe-commits
hoy added a comment.

In D86502#2245734 , @wmi wrote:

> In D86502#2245578 , @hoy wrote:
>
>> In D86502#2245460 , @wmi wrote:
>>
 The early instrumentation also allows the inliner to duplicate probes for 
 inlined instances. When a probe along with the other instructions of a 
 callee function are inlined into its caller function, the GUID of the 
 callee function goes with the probe. This allows samples collected on 
 inlined probes to be reported for the original callee function.
>>>
>>> Just get a question from reading the above. Suppose `A` only has one BB and 
>>> the BB has one PseudoProbe in it. If function `A` is inlined into `B1` and 
>>> `B2` and both `B1` and `B2` inlined into `C`, the PseudoProbe from `A` will 
>>> have two copies in `C` both carrying GUID of `A`. How the samples collected 
>>> from `A` inlined into `B1` inlined into `C` are categorized differently 
>>> from `A` inlined into `B2` inlined into `C`, especially when debug 
>>> information is not enabled (so no inline stack information in the binary)?
>>
>> This is a very good question. Inlined functions are differentiated by their 
>> original callsites. A pseudo probe is allocated for each callsite in the 
>> `SampleProfileProbe` pass. Nested inlining will produce a stack of pseudo 
>> probes, similar with the Dwarf inline stack. The work is not included in the 
>> first set of patches.
>
> Thanks, then how does the pseudo probe for a callsite after inline to 
> represent the inline scope it covers?

It is represented by the dwarf inline stack for that inlined pseudo probe. I'm 
about to send out a patch for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86502

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


[PATCH] D91544: [clang-tidy] Allow `TransformerClangTidyCheck` clients to set the rule directly.

2020-11-18 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG068da2c749a5: [clang-tidy] Allow `TransformerClangTidyCheck` 
clients to set the rule directly. (authored by ymandel).

Changed prior to commit:
  https://reviews.llvm.org/D91544?vs=305516=306151#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91544

Files:
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h

Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
@@ -19,7 +19,7 @@
 namespace tidy {
 namespace utils {
 
-// A base class for defining a ClangTidy check based on a `RewriteRule`.
+/// A base class for defining a ClangTidy check based on a `RewriteRule`.
 //
 // For example, given a rule `MyCheckAsRewriteRule`, one can define a tidy check
 // as follows:
@@ -38,24 +38,23 @@
 //  includes.
 class TransformerClangTidyCheck : public ClangTidyCheck {
 public:
-  // \p MakeRule generates the rewrite rule to be used by the check, based on
-  // the given language and clang-tidy options. It can return \c None to handle
-  // cases where the options disable the check.
-  //
-  // All cases in the rule generated by \p MakeRule must have a non-null \c
-  // Explanation, even though \c Explanation is optional for RewriteRule in
-  // general. Because the primary purpose of clang-tidy checks is to provide
-  // users with diagnostics, we assume that a missing explanation is a bug.  If
-  // no explanation is desired, indicate that explicitly (for example, by
-  // passing `text("no explanation")` to `makeRule` as the `Explanation`
-  // argument).
+  TransformerClangTidyCheck(StringRef Name, ClangTidyContext *Context);
+
+  /// DEPRECATED: prefer the two argument constructor in conjunction with
+  /// \c setRule.
+  ///
+  /// \p MakeRule generates the rewrite rule to be used by the check, based on
+  /// the given language and clang-tidy options. It can return \c None to handle
+  /// cases where the options disable the check.
+  ///
+  /// See \c setRule for constraints on the rule.
   TransformerClangTidyCheck(std::function(
 const LangOptions &, const OptionsView &)>
 MakeRule,
 StringRef Name, ClangTidyContext *Context);
 
-  // Convenience overload of the constructor when the rule doesn't depend on any
-  // of the language or clang-tidy options.
+  /// Convenience overload of the constructor when the rule doesn't have any
+  /// dependies.
   TransformerClangTidyCheck(transformer::RewriteRule R, StringRef Name,
 ClangTidyContext *Context);
 
@@ -68,8 +67,17 @@
   /// the overridden method.
   void storeOptions(ClangTidyOptions::OptionMap ) override;
 
+  /// Set the rule that this check implements.  All cases in the rule must have
+  /// a non-null \c Explanation, even though \c Explanation is optional for
+  /// RewriteRule in general. Because the primary purpose of clang-tidy checks
+  /// is to provide users with diagnostics, we assume that a missing explanation
+  /// is a bug.  If no explanation is desired, indicate that explicitly (for
+  /// example, by passing `text("no explanation")` to `makeRule` as the
+  /// `Explanation` argument).
+  void setRule(transformer::RewriteRule R);
+
 private:
-  Optional Rule;
+  transformer::RewriteRule Rule;
   IncludeInserter Inserter;
 };
 
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -21,6 +21,18 @@
 }
 #endif
 
+static void verifyRule(const RewriteRule ) {
+  assert(llvm::all_of(Rule.Cases, hasExplanation) &&
+ "clang-tidy checks must have an explanation by default;"
+ " explicitly provide an empty explanation if none is desired");
+}
+
+TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  Inserter(
+  Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {}
+
 // This constructor cannot dispatch to the simpler one (below), because, in
 // order to get meaningful results from `getLangOpts` and `Options`, we need the
 // `ClangTidyCheck()` constructor to have been called. If we were to dispatch,
@@ -31,24 +43,21 @@
 const OptionsView &)>
 MakeRule,
 StringRef Name, ClangTidyContext *Context)
-: 

[clang-tools-extra] 068da2c - [clang-tidy] Allow `TransformerClangTidyCheck` clients to set the rule directly.

2020-11-18 Thread Yitzhak Mandelbaum via cfe-commits

Author: Yitzhak Mandelbaum
Date: 2020-11-18T18:25:21Z
New Revision: 068da2c749a58b46bd59381890a6a137d6e3128e

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

LOG: [clang-tidy] Allow `TransformerClangTidyCheck` clients to set the rule 
directly.

Adds support for setting the `Rule` field. In the process, refactors the code 
that accesses that field and adds a constructor that doesn't require a rule 
argument.

This feature is needed by checks that must set the rule *after* the check class
is constructed. For example, any check that maintains state to be accessed from
the rule needs this support. Since the object's fields are not initialized when
the superclass constructor is called, they can't be (safely) captured by a rule
passed to the existing constructor.  This patch allows constructing the check
superclass fully before setting the rule.

As a driveby fix, removed the "optional" from the rule, since rules are just a
set of cases, so empty rules are evident.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp 
b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
index ae2ae86f4665..e93f3fbf21d7 100644
--- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -21,6 +21,18 @@ static bool hasExplanation(const RewriteRule::Case ) {
 }
 #endif
 
+static void verifyRule(const RewriteRule ) {
+  assert(llvm::all_of(Rule.Cases, hasExplanation) &&
+ "clang-tidy checks must have an explanation by default;"
+ " explicitly provide an empty explanation if none is desired");
+}
+
+TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  Inserter(
+  Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {}
+
 // This constructor cannot dispatch to the simpler one (below), because, in
 // order to get meaningful results from `getLangOpts` and `Options`, we need 
the
 // `ClangTidyCheck()` constructor to have been called. If we were to dispatch,
@@ -31,24 +43,21 @@ TransformerClangTidyCheck::TransformerClangTidyCheck(
 const OptionsView &)>
 MakeRule,
 StringRef Name, ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context), Rule(MakeRule(getLangOpts(), Options)),
-  Inserter(
-  Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {
-  if (Rule)
-assert(llvm::all_of(Rule->Cases, hasExplanation) &&
-   "clang-tidy checks must have an explanation by default;"
-   " explicitly provide an empty explanation if none is desired");
+: TransformerClangTidyCheck(Name, Context) {
+  if (Optional R = MakeRule(getLangOpts(), Options))
+setRule(std::move(*R));
 }
 
 TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
  StringRef Name,
  ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context), Rule(std::move(R)),
-  Inserter(
-  Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {
-  assert(llvm::all_of(Rule->Cases, hasExplanation) &&
- "clang-tidy checks must have an explanation by default;"
- " explicitly provide an empty explanation if none is desired");
+: TransformerClangTidyCheck(Name, Context) {
+  setRule(std::move(R));
+}
+
+void TransformerClangTidyCheck::setRule(transformer::RewriteRule R) {
+  verifyRule(R);
+  Rule = std::move(R);
 }
 
 void TransformerClangTidyCheck::registerPPCallbacks(
@@ -58,8 +67,8 @@ void TransformerClangTidyCheck::registerPPCallbacks(
 
 void TransformerClangTidyCheck::registerMatchers(
 ast_matchers::MatchFinder *Finder) {
-  if (Rule)
-for (auto  : transformer::detail::buildMatchers(*Rule))
+  if (!Rule.Cases.empty())
+for (auto  : transformer::detail::buildMatchers(Rule))
   Finder->addDynamicMatcher(Matcher, this);
 }
 
@@ -68,8 +77,7 @@ void TransformerClangTidyCheck::check(
   if (Result.Context->getDiagnostics().hasErrorOccurred())
 return;
 
-  assert(Rule && "check() should not fire if Rule is None");
-  RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, 
*Rule);
+  RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, Rule);
   Expected> Edits = Case.Edits(Result);
   if (!Edits) {
 llvm::errs() << 

[PATCH] D72184: [BPF] support atomic instructions

2020-11-18 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song updated this revision to Diff 306146.
yonghong-song edited the summary of this revision.
yonghong-song added a comment.

add fetch_and_{and,or,xor} support
force cmpxchg insn return results in r0 like r0 = cmpxchg(addr, r0, new)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72184

Files:
  clang/lib/Basic/Targets/BPF.cpp
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/lib/Target/BPF/BPF.td
  llvm/lib/Target/BPF/BPFInstrFormats.td
  llvm/lib/Target/BPF/BPFInstrInfo.td
  llvm/lib/Target/BPF/BPFSubtarget.cpp
  llvm/lib/Target/BPF/BPFSubtarget.h
  llvm/lib/Target/BPF/Disassembler/BPFDisassembler.cpp
  llvm/lib/Target/BPF/MCTargetDesc/BPFMCCodeEmitter.cpp
  llvm/test/CodeGen/BPF/atomics.ll
  llvm/test/CodeGen/BPF/atomics_2.ll

Index: llvm/test/CodeGen/BPF/atomics_2.ll
===
--- /dev/null
+++ llvm/test/CodeGen/BPF/atomics_2.ll
@@ -0,0 +1,202 @@
+; RUN: llc < %s -march=bpfel -verify-machineinstrs -show-mc-encoding -mcpu=v4 | FileCheck %s
+;
+; Source:
+;   char test_load_sub_8(char *p, char v) {
+; return __sync_fetch_and_sub(p, v);
+;   }
+;   short test_load_sub_16(short *p, short v) {
+; return __sync_fetch_and_sub(p, v);
+;   }
+;   int test_load_sub_32(int *p, int v) {
+; return __sync_fetch_and_sub(p, v);
+;   }
+;   int test_load_sub_64(long *p, long v) {
+; return __sync_fetch_and_sub(p, v);
+;   }
+;   int test_xchg_8(char *p, char v) {
+; return __sync_lock_test_and_set(p, v);
+;   }
+;   int test_xchg_16(short *p, short v) {
+; return __sync_lock_test_and_set(p, v);
+;   }
+;   int test_xchg_32(int *p, int v) {
+; return __sync_lock_test_and_set(p, v);
+;   }
+;   int test_xchg_64(long *p, long v) {
+; return __sync_lock_test_and_set(p, v);
+;   }
+;   int test_cas_32(int *p, int old, int new) {
+; return __sync_val_compare_and_swap(p, old, new);
+;   }
+;   long test_cas_64(long *p, long old, long new) {
+; return __sync_val_compare_and_swap(p, old, new);
+;   }
+
+; CHECK-LABEL: test_load_sub_8
+; CHECK: w0 = w2
+; CHECK: w0 = atomic_fetch_sub((u8 *)(r1 + 0), w0)
+; CHECK: encoding: [0xd3,0x01,0x00,0x00,0x11,0x00,0x00,0x00]
+define dso_local signext i8 @test_load_sub_8(i8* nocapture %p, i8 signext %v) local_unnamed_addr #0 {
+entry:
+  %0 = atomicrmw sub i8* %p, i8 %v seq_cst
+  ret i8 %0
+}
+
+; CHECK-LABEL: test_load_sub_16
+; CHECK: w0 = w2
+; CHECK: w0 = atomic_fetch_sub((u16 *)(r1 + 0), w0)
+; CHECK: encoding: [0xcb,0x01,0x00,0x00,0x11,0x00,0x00,0x00]
+define dso_local signext i16 @test_load_sub_16(i16* nocapture %p, i16 signext %v) local_unnamed_addr #0 {
+entry:
+  %0 = atomicrmw sub i16* %p, i16 %v seq_cst
+  ret i16 %0
+}
+
+; CHECK-LABEL: test_load_sub_32
+; CHECK: w0 = w2
+; CHECK: w0 = atomic_fetch_sub((u32 *)(r1 + 0), w0)
+; CHECK: encoding: [0xc3,0x01,0x00,0x00,0x11,0x00,0x00,0x00]
+define dso_local i32 @test_load_sub_32(i32* nocapture %p, i32 %v) local_unnamed_addr {
+entry:
+  %0 = atomicrmw sub i32* %p, i32 %v seq_cst
+  ret i32 %0
+}
+
+; CHECK-LABEL: test_load_sub_64
+; CHECK: r0 = r2
+; CHECK: r0 = atomic_fetch_sub((u64 *)(r1 + 0), r0)
+; CHECK: encoding: [0xdb,0x01,0x00,0x00,0x11,0x00,0x00,0x00]
+define dso_local i32 @test_load_sub_64(i64* nocapture %p, i64 %v) local_unnamed_addr {
+entry:
+  %0 = atomicrmw sub i64* %p, i64 %v seq_cst
+  %conv = trunc i64 %0 to i32
+  ret i32 %conv
+}
+
+; CHECK-LABEL: test_xchg_8
+; CHECK: w0 = w2
+; CHECK: w0 = xchg32_8(r1 + 0, w0)
+; CHECK: encoding: [0xd3,0x01,0x00,0x00,0xe1,0x00,0x00,0x00]
+define dso_local i32 @test_xchg_8(i8* nocapture %p, i8 signext %v) local_unnamed_addr {
+entry:
+  %0 = atomicrmw xchg i8* %p, i8 %v seq_cst
+  %conv = sext i8 %0 to i32
+  ret i32 %conv
+}
+
+; CHECK-LABEL: test_xchg_16
+; CHECK: w0 = w2
+; CHECK: w0 = xchg32_16(r1 + 0, w0)
+; CHECK: encoding: [0xcb,0x01,0x00,0x00,0xe1,0x00,0x00,0x00]
+define dso_local i32 @test_xchg_16(i16* nocapture %p, i16 signext %v) local_unnamed_addr {
+entry:
+  %0 = atomicrmw xchg i16* %p, i16 %v seq_cst
+  %conv = sext i16 %0 to i32
+  ret i32 %conv
+}
+
+; CHECK-LABEL: test_xchg_32
+; CHECK: w0 = w2
+; CHECK: w0 = xchg32_32(r1 + 0, w0)
+; CHECK: encoding: [0xc3,0x01,0x00,0x00,0xe1,0x00,0x00,0x00]
+define dso_local i32 @test_xchg_32(i32* nocapture %p, i32 %v) local_unnamed_addr {
+entry:
+  %0 = atomicrmw xchg i32* %p, i32 %v seq_cst
+  ret i32 %0
+}
+
+; CHECK-LABEL: test_xchg_64
+; CHECK: r0 = r2
+; CHECK: r0 = xchg_64(r1 + 0, r0)
+; CHECK: encoding: [0xdb,0x01,0x00,0x00,0xe1,0x00,0x00,0x00]
+define dso_local i32 @test_xchg_64(i64* nocapture %p, i64 %v) local_unnamed_addr {
+entry:
+  %0 = atomicrmw xchg i64* %p, i64 %v seq_cst
+  %conv = trunc i64 %0 to i32
+  ret i32 %conv
+}
+
+; CHECK-LABEL: test_cas_32
+; CHECK: w0 = w2
+; CHECK: w0 = cmpxchg32_32(r1 + 0, w0, w3)
+; CHECK: encoding: [0xc3,0x31,0x00,0x00,0xf1,0x00,0x00,0x00]
+define dso_local i32 @test_cas_32(i32* 

[PATCH] D91567: [llvm][inliner] Reuse the inliner pass to implement 'always inliner'

2020-11-18 Thread Mircea Trofin via Phabricator via cfe-commits
mtrofin added a comment.

In D91567#2403236 , @aeubanks wrote:

> One thing that would be nice would be to have both inliners in the same CGSCC 
> pass manager to avoid doing SCC construction twice, but that would require 
> some shuffling of module/cgscc passes in ModuleInlinerWrapperPass. Maybe as a 
> future cleanup.

There's that benefit to simplifying the module with the always inliner before 
doing inlining "in earnest" I was pointing earlier at: for the ML policies 
work, we plan on capturing (sub)graph information. Using the same SCC would not 
help because the "higher" (callers) parts of the graph would have these 
mandatory inlinings not completed yet, and thus offer a less accurate picture 
of the problem space.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91567

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


[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

2020-11-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 306143.
njames93 added a comment.

Added test cases to ensure clang-tidy doesn't crash.
Expanded auto out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91485

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-pp-no-crash.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp
@@ -226,3 +226,89 @@
   }
   return;
 }
+
+void testPPConditionals() {
+
+  // These cases the return isn't inside the conditional so diagnose as normal.
+  if (true) {
+return;
+#if 1
+#endif
+  } else {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+return;
+  }
+  if (true) {
+#if 1
+#endif
+return;
+  } else {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+return;
+  }
+
+  // No return here in the AST, no special handling needed.
+  if (true) {
+#if 0
+return;
+#endif
+  } else {
+return;
+  }
+
+  // Return here is inside a preprocessor conditional block, ignore this case.
+  if (true) {
+#if 1
+return;
+#endif
+  } else {
+return;
+  }
+
+  // These cases, same as above but with an #else block.
+  if (true) {
+#if 1
+return;
+#else
+#endif
+  } else {
+return;
+  }
+  if (true) {
+#if 0
+#else
+return;
+#endif
+  } else {
+return;
+  }
+
+// Ensure it can handle macros.
+#define RETURN return
+  if (true) {
+#if 1
+RETURN;
+#endif
+  } else {
+return;
+  }
+#define ELSE else
+  if (true) {
+#if 1
+return;
+#endif
+  }
+  ELSE {
+return;
+  }
+
+  // Whole statement is in a conditional block so diagnose as normal.
+#if 1
+  if (true) {
+return;
+  } else {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use 'else' after 'return'
+return;
+  }
+#endif
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-pp-no-crash.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-pp-no-crash.cpp
@@ -0,0 +1,22 @@
+// RUN: clang-tidy %s -checks=-*,readability-else-after-return -- -std=c++11
+
+// We aren't concerned about the output here, just want to ensure clang-tidy doesn't crash.
+void foo() {
+#if 1
+  if (true) {
+return;
+#else
+  {
+#endif
+  } else {
+return;
+  }
+
+  if (true) {
+#if 1
+return;
+  } else {
+#endif
+return;
+  }
+}
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ELSEAFTERRETURNCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseMap.h"
 
 namespace clang {
 namespace tidy {
@@ -23,12 +24,18 @@
   ElseAfterReturnCheck(StringRef Name, ClangTidyContext *Context);
 
   void storeOptions(ClangTidyOptions::OptionMap ) override;
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
 
+  using ConditionalBranchMap =
+  llvm::DenseMap>;
+
 private:
   const bool WarnOnUnfixable;
   const bool WarnOnConditionVariables;
+  ConditionalBranchMap PPConditionals;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/FixIt.h"
 #include "llvm/ADT/SmallVector.h"
 
@@ -19,10 +20,30 @@
 namespace tidy {
 namespace readability {
 
-static const char ReturnStr[] = "return";
-static const char ContinueStr[] = "continue";
-static const char BreakStr[] = "break";
-static const char ThrowStr[] = "throw";
+namespace {
+
+class PPConditionalCollector : public PPCallbacks {
+public:
+  PPConditionalCollector(
+  

[clang-tools-extra] 130da80 - Revert "Revert "[clangd] Implement textDocument/implementation (Xref layer)""

2020-11-18 Thread Utkarsh Saxena via cfe-commits

Author: Utkarsh Saxena
Date: 2020-11-18T19:09:16+01:00
New Revision: 130da802ff6e59d59ac5afce9e6a4235f3fe4959

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

LOG: Revert "Revert "[clangd] Implement textDocument/implementation (Xref 
layer)""

This reverts commit 0016ab6f3632968e52eb83de021908f0c94bbb10.

Fix: Consume error from Expected.

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index db61452e0f17..0cd8695da92d 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -1124,6 +1124,58 @@ std::vector 
findDocumentHighlights(ParsedAST ,
   return Result;
 }
 
+std::vector findImplementations(ParsedAST , Position Pos,
+   const SymbolIndex *Index) {
+  // We rely on index to find the implementations in subclasses.
+  // FIXME: Index can be stale, so we may loose some latest results from the
+  // main file.
+  if (!Index)
+return {};
+  const SourceManager  = AST.getSourceManager();
+  auto MainFilePath =
+  getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
+  if (!MainFilePath) {
+elog("Failed to get a path for the main file, so no implementations.");
+return {};
+  }
+  auto CurLoc = sourceLocationInMainFile(SM, Pos);
+  if (!CurLoc) {
+elog("Failed to convert position to source location: {0}",
+ CurLoc.takeError());
+return {};
+  }
+  std::vector Results;
+  DeclRelationSet Relations =
+  DeclRelation::TemplatePattern | DeclRelation::Alias;
+  RelationsRequest Req;
+  Req.Predicate = RelationKind::OverriddenBy;
+  for (const NamedDecl *ND : getDeclAtPosition(AST, *CurLoc, Relations))
+if (const CXXMethodDecl *CXXMD = llvm::dyn_cast(ND))
+  if (CXXMD->isVirtual())
+Req.Subjects.insert(getSymbolID(ND));
+
+  if (Req.Subjects.empty())
+return Results;
+  Index->relations(Req, [&](const SymbolID , const Symbol ) {
+auto DeclLoc =
+indexToLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
+if (!DeclLoc) {
+  elog("Find implementation: {0}", DeclLoc.takeError());
+  return;
+}
+LocatedSymbol Loc;
+Loc.Name = Object.Name.str();
+Loc.PreferredDeclaration = *DeclLoc;
+auto DefLoc = indexToLSPLocation(Object.Definition, *MainFilePath);
+if (DefLoc)
+  Loc.Definition = *DefLoc;
+else
+  llvm::consumeError(DefLoc.takeError());
+Results.push_back(Loc);
+  });
+  return Results;
+}
+
 ReferencesResult findReferences(ParsedAST , Position Pos, uint32_t Limit,
 const SymbolIndex *Index) {
   if (!Limit)

diff  --git a/clang-tools-extra/clangd/XRefs.h 
b/clang-tools-extra/clangd/XRefs.h
index 521c28f934be..fac1a992a12f 100644
--- a/clang-tools-extra/clangd/XRefs.h
+++ b/clang-tools-extra/clangd/XRefs.h
@@ -82,6 +82,11 @@ struct ReferencesResult {
   std::vector References;
   bool HasMore = false;
 };
+
+/// Returns implementations of the virtual function at a specified \p Pos.
+std::vector findImplementations(ParsedAST , Position Pos,
+   const SymbolIndex *Index);
+
 /// Returns references of the symbol at a specified \p Pos.
 /// \p Limit limits the number of results returned (0 means no limit).
 ReferencesResult findReferences(ParsedAST , Position Pos, uint32_t Limit,

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp 
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index efca92fd0e9f..31d2a37f64a7 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -43,6 +43,7 @@ using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::UnorderedElementsAre;
 using ::testing::UnorderedElementsAreArray;
+using ::testing::UnorderedPointwise;
 
 MATCHER_P2(FileRange, File, Range, "") {
   return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
@@ -1161,12 +1162,12 @@ TEST(LocateSymbol, Alias) {
 )cpp",
   };
 
-  for (const auto* Case : Tests) {
+  for (const auto *Case : Tests) {
 SCOPED_TRACE(Case);
 auto T = Annotations(Case);
 auto AST = TestTU::withCode(T.code()).build();
 EXPECT_THAT(locateSymbolAt(AST, T.point()),
-::testing::UnorderedPointwise(DeclRange(), T.ranges()));
+UnorderedPointwise(DeclRange(), T.ranges()));
   }
 }
 
@@ -1464,6 +1465,67 @@ TEST(LocateSymbol, NearbyIdentifier) {
   }
 }
 
+TEST(FindImplementations, Inheritance) {
+  llvm::StringRef Test = R"cpp(
+struct Base {
+  virtual void F$1^oo();
+  void 

  1   2   3   >