[PATCH] D79744: clang: Use byref for aggregate kernel arguments

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Arguably we should add this attribute to all indirect arguments.  I can 
understand not wanting to update all the test cases, but you could probably 
avoid adding a new IndirectByRef kind of ABIArgInfo by treating kernels 
specially in ConstructAttributeList.

Or, sorry, I forget — is this semantically necessary because the argument is to 
constant memory and the callee has to copy it to form the mutable local?  If 
so, I think (1) the above statement about theoretically using `byref` on all 
arguments still applies and (2) we do need a new ABIArgInfo kind, but we should 
name it something like IndirectAliased.


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

https://reviews.llvm.org/D79744



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

[clang] 6aea36f - Follow-on fixes for get/isIntegerConstantExpression

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2020-07-21T21:51:59-07:00
New Revision: 6aea36fb98ed1e0c89cd6e3a24b76339a75aef68

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

LOG: Follow-on fixes for get/isIntegerConstantExpression

Added: 


Modified: 
clang/lib/Sema/SemaType.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 09def48a158e..3d6c249c2c17 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -7689,14 +7689,17 @@ static bool isPermittedNeonBaseType(QualType ,
 static bool verifyValidIntegerConstantExpr(Sema , const ParsedAttr ,
llvm::APSInt ) {
   const auto *AttrExpr = Attr.getArgAsExpr(0);
-  if (AttrExpr->isTypeDependent() || AttrExpr->isValueDependent() ||
-  !AttrExpr->isIntegerConstantExpr(Result, S.Context)) {
-S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
-<< Attr << AANT_ArgumentIntegerConstant << AttrExpr->getSourceRange();
-Attr.setInvalid();
-return false;
+  if (!AttrExpr->isTypeDependent() && !AttrExpr->isValueDependent()) {
+if (Optional Res =
+AttrExpr->getIntegerConstantExpr(S.Context)) {
+  Result = *Res;
+  return true;
+}
   }
-  return true;
+  S.Diag(Attr.getLoc(), diag::err_attribute_argument_type)
+  << Attr << AANT_ArgumentIntegerConstant << AttrExpr->getSourceRange();
+  Attr.setInvalid();
+  return false;
 }
 
 /// HandleNeonVectorTypeAttr - The "neon_vector_type" and
@@ -7737,7 +7740,7 @@ static void HandleNeonVectorTypeAttr(QualType , 
const ParsedAttr ,
 
   // The total size of the vector must be 64 or 128 bits.
   unsigned typeSize = static_cast(S.Context.getTypeSize(CurType));
-  unsigned numElts = static_cast(numEltsInt->getZExtValue());
+  unsigned numElts = static_cast(numEltsInt.getZExtValue());
   unsigned vecSize = typeSize * numElts;
   if (vecSize != 64 && vecSize != 128) {
 S.Diag(Attr.getLoc(), diag::err_attribute_bad_neon_vector_size) << CurType;



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

Buildbot numbers for the week of 07/12/2020 - 07/18/2020

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the last week of 07/12/2020 -
07/18/2020.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from green to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername| was_red
--+-
 sanitizer-x86_64-linux   | 75:51:01
 clang-s390x-linux-lnt| 64:53:02
 clang-cmake-aarch64-lld  | 40:26:36
 clang-cmake-aarch64-full | 40:17:00
 sanitizer-x86_64-linux-bootstrap | 37:51:22
 clang-with-thin-lto-ubuntu   | 37:35:16
 sanitizer-x86_64-linux-bootstrap-ubsan   | 37:20:48
 clang-with-lto-ubuntu| 35:59:42
 sanitizer-x86_64-linux-bootstrap-msan| 34:58:20
 fuchsia-x86_64-linux | 34:38:49
 clang-x86_64-debian-new-pass-manager-fast| 31:45:03
 clang-ppc64be-linux-lnt  | 20:47:08
 clang-ppc64be-linux  | 20:29:33
 clang-s390x-linux| 20:28:41
 clang-ppc64be-linux-multistage   | 20:25:08
 sanitizer-windows| 19:54:14
 llvm-avr-linux   | 19:25:45
 clang-s390x-linux-multistage | 17:50:24
 sanitizer-x86_64-linux-android   | 08:54:01
 clang-x64-windows-msvc   | 07:41:40
 clang-ppc64le-linux-multistage   | 06:47:19
 sanitizer-x86_64-linux-fast  | 06:25:10
 clang-ppc64le-linux-lnt  | 06:13:33
 lldb-x64-windows-ninja   | 06:10:38
 clang-ppc64le-linux  | 05:53:00
 sanitizer-ppc64be-linux  | 05:51:59
 clang-cmake-armv8-lld| 05:03:42
 clang-native-arm-lnt-perf| 03:56:23
 llvm-clang-x86_64-expensive-checks-ubuntu| 03:54:32
 llvm-clang-x86_64-expensive-checks-win   | 03:52:36
 llvm-clang-x86_64-expensive-checks-debian| 03:38:01
 llvm-clang-win-x-armv7l  | 03:18:47
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 03:07:52
 lld-x86_64-win   | 02:46:34
 sanitizer-x86_64-linux-fuzzer| 02:43:41
 mlir-windows | 02:32:56
 llvm-clang-win-x-aarch64 | 02:30:30
 clang-ppc64le-rhel   | 02:25:32
 openmp-clang-x86_64-linux-debian | 02:25:31
 llvm-clang-x86_64-win-fast   | 02:19:24
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx17   | 02:09:40
 ppc64le-lld-multistage-test  | 02:07:43
 libcxx-libcxxabi-x86_64-linux-ubuntu-asan| 02:05:12
 libcxx-libcxxabi-x86_64-linux-ubuntu-ubsan   | 02:05:11
 clang-cmake-aarch64-global-isel  | 02:04:35
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx2a   | 02:04:18
 libcxx-libcxxabi-x86_64-linux-ubuntu-msan| 02:01:22
 clang-cmake-x86_64-avx2-linux| 01:59:43
 clang-cmake-armv7-quick  | 01:46:31
 lld-x86_64-ubuntu-fast   | 01:46:20
 clang-cmake-aarch64-quick| 01:40:58
 clang-cmake-x86_64-sde-avx512-linux  | 01:39:59
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast   | 01:38:02
 clang-x86_64-debian-fast | 01:35:30
 clang-cmake-armv7-global-isel| 01:32:40
 openmp-gcc-x86_64-linux-debian   | 01:31:17
 clang-armv7-linux-build-cache| 01:24:34
 llvm-sphinx-docs | 01:22:16
 flang-aarch64-ubuntu | 01:21:12
 lld-x86_64-darwin| 01:20:36
 mlir-nvidia  | 01:20:23
 clang-cmake-armv7-lnt| 01:19:58
 lldb-arm-ubuntu  | 01:11:04
 lldb-aarch64-ubuntu  | 01:08:17
 libc-x86_64-debian-dbg   | 01:04:41
 sanitizer-x86_64-linux-autoconf  | 01:04:12
 publish-sphinx-docs  | 01:03:56
 libc-x86_64-debian-dbg-asan  | 01:02:37
 clang-x86_64-linux-abi-test  | 00:59:22
 libc-x86_64-debian 

Buildbot numbers for the week of 07/5/2020 - 07/11/2020

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the week of 07/5/2020 - 07/11/2020.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from green to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername   |  was_red
-+--
 clang-cmake-aarch64-lld | 153:30:07
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc5-cxx11 | 122:32:08
 llvm-sphinx-docs| 110:37:57
 llvm-clang-x86_64-expensive-checks-win  | 60:10:24
 aosp-O3-polly-before-vectorizer-unprofitable| 57:15:01
 polly-arm-linux | 53:32:33
 polly-x86_64-linux  | 51:00:58
 clang-cmake-aarch64-full| 34:36:56
 ppc64le-lld-multistage-test | 33:11:21
 clang-ppc64le-linux-lnt | 31:51:42
 llvm-clang-x86_64-expensive-checks-ubuntu   | 31:26:43
 llvm-clang-x86_64-expensive-checks-debian   | 31:26:20
 sanitizer-ppc64be-linux | 29:34:38
 clang-ppc64le-linux-multistage  | 29:07:07
 clang-ppc64be-linux | 28:09:45
 libcxx-libcxxabi-libunwind-armv7-linux  | 25:49:12
 libcxx-libcxxabi-libunwind-armv7-linux-noexceptions | 25:49:01
 clang-ppc64le-linux | 25:44:30
 llvm-clang-win-x-armv7l | 25:07:57
 clang-ppc64be-linux-lnt | 25:02:58
 llvm-clang-win-x-aarch64| 23:27:56
 libcxx-libcxxabi-libunwind-armv8-linux  | 17:24:00
 libcxx-libcxxabi-libunwind-armv8-linux-noexceptions | 17:05:24
 fuchsia-x86_64-linux| 16:52:25
 libcxx-libcxxabi-x86_64-linux-ubuntu-32bit  | 14:46:40
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc-tot-latest-std | 14:46:12
 mlir-windows| 14:35:36
 clang-s390x-linux-lnt   | 13:25:12
 lldb-x64-windows-ninja  | 12:43:01
 sanitizer-x86_64-linux  | 10:36:26
 libc-x86_64-debian-dbg  | 09:49:27
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 07:41:12
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 06:47:19
 clang-x64-windows-msvc  | 06:35:30
 clang-with-lto-ubuntu   | 06:25:30
 libcxx-libcxxabi-libunwind-aarch64-linux| 05:30:01
 clang-ppc64le-rhel  | 05:28:01
 clang-with-thin-lto-ubuntu  | 05:27:09
 clang-cmake-armv8-lld   | 04:49:39
 lldb-arm-ubuntu | 03:38:27
 lld-x86_64-win  | 03:16:22
 clang-s390x-linux   | 02:48:34
 clang-x86_64-debian-fast| 02:11:52
 clang-cmake-aarch64-global-isel | 02:04:29
 clang-cmake-aarch64-quick   | 01:53:33
 sanitizer-x86_64-linux-bootstrap| 01:52:52
 flang-aarch64-ubuntu| 01:45:23
 sanitizer-x86_64-linux-bootstrap-msan   | 01:42:58
 clang-cmake-armv7-global-isel   | 01:36:33
 llvm-avr-linux  | 01:33:45
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx11  | 01:32:15
 sanitizer-x86_64-linux-fast | 01:29:56
 libcxx-libcxxabi-x86_64-linux-ubuntu-ubsan  | 01:28:44
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx2a  | 01:28:11
 sanitizer-x86_64-linux-android  | 01:28:04
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx17  | 01:27:59
 libcxx-libcxxabi-x86_64-linux-ubuntu-asan   | 01:27:44
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx14  | 01:25:06
 clang-cmake-armv7-quick | 01:20:23
 lld-x86_64-darwin   | 01:20:02
 clang-x86_64-debian-new-pass-manager-fast   | 01:18:40
 libcxx-libcxxabi-libunwind-x86_64-linux-ubuntu  | 01:17:48
 

[clang] 36036aa - Reapply "Rename/refactor isIntegerConstantExpression to getIntegerConstantExpression"

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2020-07-21T20:57:12-07:00
New Revision: 36036aa70ec1df7b51b5d30b2dd8090ad2b6e783

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

LOG: Reapply "Rename/refactor isIntegerConstantExpression to 
getIntegerConstantExpression"

Reapply 49e5f603d40083dce9c05796e3cde3a185c3beba
which had been reverted in c94332919bd922032e979b3ae3ced5ca5bdf9650.

Originally reverted because I hadn't updated it in quite a while when I
got around to committing it, so there were a bunch of missing changes to
new code since I'd written the patch.

Reviewers: aaron.ballman

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

Added: 


Modified: 
clang/include/clang/AST/Expr.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/AST/MicrosoftMangle.cpp
clang/lib/AST/OpenMPClause.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/SemaAttr.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaStmtAttr.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaType.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index ea8f688452eb..24bface15d3e 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -522,16 +522,15 @@ class Expr : public ValueStmt {
   /// semantically correspond to a bool.
   bool isKnownToHaveBooleanValue(bool Semantic = true) const;
 
-  /// isIntegerConstantExpr - Return true if this expression is a valid integer
-  /// constant expression, and, if so, return its value in Result.  If not a
-  /// valid i-c-e, return false and fill in Loc (if specified) with the 
location
-  /// of the invalid expression.
+  /// isIntegerConstantExpr - Return the value if this expression is a valid
+  /// integer constant expression.  If not a valid i-c-e, return None and fill
+  /// in Loc (if specified) with the location of the invalid expression.
   ///
   /// Note: This does not perform the implicit conversions required by C++11
   /// [expr.const]p5.
-  bool isIntegerConstantExpr(llvm::APSInt , const ASTContext ,
- SourceLocation *Loc = nullptr,
- bool isEvaluated = true) const;
+  Optional getIntegerConstantExpr(const ASTContext ,
+SourceLocation *Loc = nullptr,
+bool isEvaluated = true) const;
   bool isIntegerConstantExpr(const ASTContext ,
  SourceLocation *Loc = nullptr) const;
 

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 2ba643f12a82..807028885652 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -9471,17 +9471,15 @@ QualType ASTContext::mergeTypes(QualType LHS, QualType 
RHS,
   const ConstantArrayType* CAT)
   -> std::pair {
 if (VAT) {
-  llvm::APSInt TheInt;
+  Optional TheInt;
   Expr *E = VAT->getSizeExpr();
-  if (E && E->isIntegerConstantExpr(TheInt, *this))
-return std::make_pair(true, TheInt);
-  else
-return std::make_pair(false, TheInt);
-} else if (CAT) {
-return std::make_pair(true, CAT->getSize());
-} else {
-return std::make_pair(false, llvm::APInt());
+  if (E && (TheInt = E->getIntegerConstantExpr(*this)))
+return std::make_pair(true, *TheInt);
+  return std::make_pair(false, llvm::APSInt());
 }
+if (CAT)
+  return std::make_pair(true, CAT->getSize());
+return std::make_pair(false, llvm::APInt());
   };
 
   bool HaveLSize, HaveRSize;

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 41a4ae4b91c8..5f8ad18c0ed2 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -14892,16 +14892,22 @@ bool Expr::isIntegerConstantExpr(const ASTContext 
,
   return true;
 }
 
-bool Expr::isIntegerConstantExpr(llvm::APSInt , const ASTContext ,
- SourceLocation *Loc, bool isEvaluated) const {
+Optional Expr::getIntegerConstantExpr(const ASTContext ,
+SourceLocation *Loc,
+bool isEvaluated) const {
   assert(!isValueDependent() &&
  "Expression evaluator can't be called on a dependent expression.");
 
-  if (Ctx.getLangOpts().CPlusPlus11)
-return 

[PATCH] D84291: [PowerPC][Power10] Fix the Test LSB by Byte (xvtlsbb) Builtins Implementation

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: power-llvm-team, PowerPC, nemanjai, rzurob.
amyk added projects: LLVM, clang, PowerPC.
Herald added subscribers: shchenz, hiraditya.

The implementation of the `xvtlsbb` builtins/intrinsics were not correct as the 
intrinsics previously
used `i1` as an argument type. This patch changes the `i1` argument type used 
in these intrinsics
to be `i32` instead, as having the second as an `i1` can lead to issues in the 
backend.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84291

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll


Index: llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
@@ -6,7 +6,7 @@
 ; These test cases aims to test the builtins for the Power10 VSX vector
 ; instructions introduced in ISA 3.1.
 
-declare i32 @llvm.ppc.vsx.xvtlsbb(<16 x i8>, i1)
+declare i32 @llvm.ppc.vsx.xvtlsbb(<16 x i8>, i32)
 
 define signext i32 @test_vec_test_lsbb_all_ones(<16 x i8> %vuca) {
 ; CHECK-LABEL: test_vec_test_lsbb_all_ones:
@@ -17,7 +17,7 @@
 ; CHECK-NEXT:extsw r3, r3
 ; CHECK-NEXT:blr
 entry:
-  %0 = tail call i32 @llvm.ppc.vsx.xvtlsbb(<16 x i8> %vuca, i1 1)
+  %0 = tail call i32 @llvm.ppc.vsx.xvtlsbb(<16 x i8> %vuca, i32 1)
   ret i32 %0
 }
 
@@ -30,6 +30,6 @@
 ; CHECK-NEXT:extsw r3, r3
 ; CHECK-NEXT:blr
 entry:
-  %0 = tail call i32 @llvm.ppc.vsx.xvtlsbb(<16 x i8> %vuca, i1 0)
+  %0 = tail call i32 @llvm.ppc.vsx.xvtlsbb(<16 x i8> %vuca, i32 0)
   ret i32 %0
 }
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -1052,7 +1052,7 @@
 (v4i32 (COPY_TO_REGCLASS (XXGENPCVWM $VRB, imm:$IMM), VRRC))>;
   def : Pat<(v2i64 (int_ppc_vsx_xxgenpcvdm v2i64:$VRB, imm:$IMM)),
 (v2i64 (COPY_TO_REGCLASS (XXGENPCVDM $VRB, imm:$IMM), VRRC))>;
-  def : Pat<(i32 (int_ppc_vsx_xvtlsbb v16i8:$XB, -1)),
+  def : Pat<(i32 (int_ppc_vsx_xvtlsbb v16i8:$XB, 1)),
 (EXTRACT_SUBREG (XVTLSBB (COPY_TO_REGCLASS $XB, VSRC)), sub_lt)>;
   def : Pat<(i32 (int_ppc_vsx_xvtlsbb v16i8:$XB, 0)),
 (EXTRACT_SUBREG (XVTLSBB (COPY_TO_REGCLASS $XB, VSRC)), sub_eq)>;
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -1069,7 +1069,7 @@
 [IntrNoMem]>;
 def int_ppc_vsx_xvtlsbb :
   PowerPC_VSX_Intrinsic<"xvtlsbb", [llvm_i32_ty],
-[llvm_v16i8_ty, llvm_i1_ty], [IntrNoMem]>;
+[llvm_v16i8_ty, llvm_i32_ty], [IntrNoMem]>;
 def int_ppc_vsx_xxeval :
   PowerPC_VSX_Intrinsic<"xxeval", [llvm_v2i64_ty],
[llvm_v2i64_ty, llvm_v2i64_ty,
Index: clang/test/CodeGen/builtins-ppc-p10vector.c
===
--- clang/test/CodeGen/builtins-ppc-p10vector.c
+++ clang/test/CodeGen/builtins-ppc-p10vector.c
@@ -583,13 +583,13 @@
 }
 
 int test_vec_test_lsbb_all_ones(void) {
-  // CHECK: @llvm.ppc.vsx.xvtlsbb(<16 x i8> %{{.+}}, i1 true
+  // CHECK: @llvm.ppc.vsx.xvtlsbb(<16 x i8> %{{.+}}, i32 1
   // CHECK-NEXT: ret i32
   return vec_test_lsbb_all_ones(vuca);
 }
 
 int test_vec_test_lsbb_all_zeros(void) {
-  // CHECK: @llvm.ppc.vsx.xvtlsbb(<16 x i8> %{{.+}}, i1 false
+  // CHECK: @llvm.ppc.vsx.xvtlsbb(<16 x i8> %{{.+}}, i32 0
   // CHECK-NEXT: ret i32
   return vec_test_lsbb_all_zeros(vuca);
 }
Index: clang/include/clang/Basic/BuiltinsPPC.def
===
--- clang/include/clang/Basic/BuiltinsPPC.def
+++ clang/include/clang/Basic/BuiltinsPPC.def
@@ -467,7 +467,7 @@
 
 BUILTIN(__builtin_vsx_xxeval, "V2ULLiV2ULLiV2ULLiV2ULLiIi", "")
 
-BUILTIN(__builtin_vsx_xvtlsbb, "iV16Ucb", "")
+BUILTIN(__builtin_vsx_xvtlsbb, "iV16UcUi", "")
 
 // P10 Vector Permute Extended built-in.
 BUILTIN(__builtin_vsx_xxpermx, "V16UcV16UcV16UcV16UcIi", "")


Index: llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-p10vsx.ll
@@ -6,7 +6,7 @@
 ; These test cases aims to test the builtins for the Power10 VSX vector
 ; instructions introduced in ISA 3.1.
 
-declare i32 @llvm.ppc.vsx.xvtlsbb(<16 x i8>, i1)
+declare i32 @llvm.ppc.vsx.xvtlsbb(<16 x i8>, i32)
 
 define signext i32 @test_vec_test_lsbb_all_ones(<16 x i8> %vuca) {
 ; 

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D83592#2165818 , @thakis wrote:

> Looks like this breaks tests on Windows: 
> http://45.33.8.238/win/20315/step_7.txt
>
> Please take a look, and if it takes a while to investigate please revert 
> while you look.


Fixed


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

https://reviews.llvm.org/D83592



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

[clang] 3930c4e - [Coverage] fix failed test case.

[Zequan Wu via cfe-commits]

Author: Zequan Wu
Date: 2020-07-21T19:16:18-07:00
New Revision: 3930c4e7d1aa486ccb4245214c0b383f40574a52

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

LOG: [Coverage] fix failed test case.

Added: 


Modified: 
clang/test/CoverageMapping/includehell.cpp

Removed: 




diff  --git a/clang/test/CoverageMapping/includehell.cpp 
b/clang/test/CoverageMapping/includehell.cpp
index 33656a7d34d5..c92f12e5e80d 100644
--- a/clang/test/CoverageMapping/includehell.cpp
+++ b/clang/test/CoverageMapping/includehell.cpp
@@ -1,5 +1,5 @@
-// RUN: %strip_comments > %t.stripped.cpp
-// RUN: %clang_cc1 -I/%S -fprofile-instrument=clang -fcoverage-mapping 
-dump-coverage-mapping -emit-llvm-only -main-file-name includehell.cpp 
%t.stripped.cpp > %tmapping
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping 
-dump-coverage-mapping -emit-llvm-only -main-file-name includehell.cpp %s > 
%tmapping
+
 int main() {
   int x = 0;
 



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests on Windows: http://45.33.8.238/win/20315/step_7.txt

Please take a look, and if it takes a while to investigate please revert while 
you look.


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

https://reviews.llvm.org/D83592



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

[PATCH] D79290: Update suffix check and cast non-suffix types

[nigan2008 via Phabricator via cfe-commits]

BATManVSGig added a comment.

this good idea.


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

https://reviews.llvm.org/D79290



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

zequanwu marked an inline comment as done.
zequanwu added inline comments.



Comment at: compiler-rt/test/profile/coverage_comments.cpp:9
+int y = 0; /* comment */   // CHECK:  [[# @LINE]]| 1|  int y = 0; 
/* comment */
+int z = 0; // comment  // CHECK:  [[# @LINE]]| 1|  int z = 0; 
// comment
+// comment // CHECK:  [[# @LINE]]|  |  // comment

MaskRay wrote:
> I think you can just use `CHECK-NEXT:` for each line and remove duplicated 
> code on the right side. See some recent `gcov-*` tests I added.
Thanks, I removed the duplicated code when fixed a coverage test failure.


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

https://reviews.llvm.org/D83592



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

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D82598#2164363 , @Szelethus wrote:

> > Hmm, interesting. I don't really understand why do we need to keep that 
> > block live, as we definitely won't use any of the value it provides (since 
> > it does not provide a value at all).
>
> Actually, what I said initially is true:
>
> > [...] the only non-expression statements it **queried** are 
> > ObjCForCollectionStmts [...]
>
> so I think it'd be okay to simply drop this.


Yeah, that sounds about right; you observed the current behavior to be a 
counterexample but found no evidence that the current behavior makes any sense.

P.S. We've found an example where `ObjCForCollectionStmt`s break (i.e., your 
recently added assertion gets hit), i'll reduce and think of a patch. Still 
shocked this wasn't covered with tests.




Comment at: clang/lib/Analysis/LiveVariables.cpp:317-320
   for (Stmt *Child : S->children()) {
-if (Child)
-  AddLiveStmt(val.liveStmts, LV.SSetFact, Child);
+if (const auto *E = dyn_cast_or_null(Child))
+  AddLiveExpr(val.liveExprs, LV.SSetFact, E);
   }

Szelethus wrote:
> xazax.hun wrote:
> > Szelethus wrote:
> > > ..this part of the code caused the issue. Looking at the related CFG,
> > > ```lang=c++
> > > void test_lambda_refcapture()
> > >  [B2 (ENTRY)]
> > >Succs (1): B1
> > > 
> > >  [B1]
> > >1: 6
> > >2: int a = 6;
> > >3: operator()
> > >4: [B1.3] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(int &) 
> > > const)
> > >5: [&](int ) {
> > > a = 42;
> > > }
> > >6: [B1.5]
> > >7: [B1.6] (ImplicitCastExpr, NoOp, const class (lambda at 
> > > /home/szelethus/Documents/llvm-project/clang/test/Analysis/live-stmts.cpp:183:3))
> > >8: a
> > >9: [B1.7]([B1.8]) (OperatorCall)
> > >   10: clang_analyzer_eval
> > >   11: [B1.10] (ImplicitCastExpr, FunctionToPointerDecay, void (*)(_Bool))
> > >   12: a
> > >   13: [B1.12] (ImplicitCastExpr, LValueToRValue, int)
> > >   14: 42
> > >   15: [B1.13] == [B1.14]
> > >   16: [B1.11]([B1.15])
> > >Preds (1): B2
> > >Succs (1): B0
> > > 
> > >  [B0 (EXIT)]
> > >Preds (1): B1
> > > ```
> > > its clear that element 5 added the live statement, and I think that that 
> > > this entire CFG just simply isn't right. Shouldn't we have a distinct 
> > > element for the assignment?
> > > 
> > >  Shouldn't we have a distinct element for the assignment?
> > 
> > Strictly speaking, we have CFGs for a function. The assignment is **not** 
> > in this function, it is in the `operator()` of the class representing this 
> > lambda expression. 
> > 
> > So basically, we do have a `LambdaExpr` to represent the expression, but 
> > the body of the lambda is in a separate entity.
> Well, `debug.DumpCFG` definitely doesn't indulge me with a separate lambda 
> CFG, so I figured this is a (rightful) optimization or compression.
> 
> My point is, this entire code snippet is a seriously error prone, best-effort 
> heuristic. Switch casing every small little corner case might be tedious, 
> troublesome in terms of scaling, and I for sure don't want to maintain it 
> 'til eternity, but we have to acknowledge that this isn't a perfect solution 
> either.
> Well, debug.DumpCFG definitely doesn't indulge me with a separate lambda CFG

Does that mean that `checkASTCodeBody` doesn't get run on lambda bodies, and 
neither do any of our path-insensitive checks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

zequanwu closed this revision.
zequanwu added a comment.

Landed here, https://reviews.llvm.org/rGabd45154bdb6b76c5b480455eacc8c75b08242aa
I put the wrong diff ID..


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

https://reviews.llvm.org/D83592



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: compiler-rt/test/profile/coverage_comments.cpp:9
+int y = 0; /* comment */   // CHECK:  [[# @LINE]]| 1|  int y = 0; 
/* comment */
+int z = 0; // comment  // CHECK:  [[# @LINE]]| 1|  int z = 0; 
// comment
+// comment // CHECK:  [[# @LINE]]|  |  // comment

I think you can just use `CHECK-NEXT:` for each line and remove duplicated code 
on the right side. See some recent `gcov-*` tests I added.


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

https://reviews.llvm.org/D83592



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

[clang] abd4515 - [Coverage] Add comment to skipped regions

[Zequan Wu via cfe-commits]

Author: Zequan Wu
Date: 2020-07-21T17:34:18-07:00
New Revision: abd45154bdb6b76c5b480455eacc8c75b08242aa

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

LOG: [Coverage] Add comment to skipped regions

Bug filled here: https://bugs.llvm.org/show_bug.cgi?id=45757.
Add comment to skipped regions so we don't track execution count for lines 
containing only comments.

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

Added: 
compiler-rt/test/profile/coverage_comments.cpp

Modified: 
clang/include/clang/Lex/Preprocessor.h
clang/lib/CodeGen/CodeGenAction.cpp
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/lib/CodeGen/CoverageMappingGen.h
clang/lib/Lex/Preprocessor.cpp
clang/test/CoverageMapping/break.c
clang/test/CoverageMapping/builtinmacro.c
clang/test/CoverageMapping/classtemplate.cpp
clang/test/CoverageMapping/comment-in-macro.c
clang/test/CoverageMapping/continue.c
clang/test/CoverageMapping/coroutine.cpp
clang/test/CoverageMapping/deferred-region.cpp
clang/test/CoverageMapping/if.cpp
clang/test/CoverageMapping/includehell.cpp
clang/test/CoverageMapping/label.cpp
clang/test/CoverageMapping/logical.cpp
clang/test/CoverageMapping/loops.cpp
clang/test/CoverageMapping/macro-expressions.cpp
clang/test/CoverageMapping/macroparams2.c
clang/test/CoverageMapping/macros.c
clang/test/CoverageMapping/macroscopes.cpp
clang/test/CoverageMapping/moremacros.c
clang/test/CoverageMapping/objc.m
clang/test/CoverageMapping/pr32679.cpp
clang/test/CoverageMapping/preprocessor.c
clang/test/CoverageMapping/return.c
clang/test/CoverageMapping/switch.cpp
clang/test/CoverageMapping/switchmacro.c
clang/test/CoverageMapping/test.c
clang/test/CoverageMapping/trycatch.cpp
clang/test/CoverageMapping/unreachable-macro.c
clang/test/CoverageMapping/while.c
clang/test/lit.cfg.py
compiler-rt/test/profile/Inputs/instrprof-comdat.h

Removed: 




diff  --git a/clang/include/clang/Lex/Preprocessor.h 
b/clang/include/clang/Lex/Preprocessor.h
index 5cd017fa925f..b0dd363555ab 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -419,6 +419,9 @@ class Preprocessor {
   /// The number of (LexLevel 0) preprocessor tokens.
   unsigned TokenCount = 0;
 
+  /// Preprocess every token regardless of LexLevel.
+  bool PreprocessToken = false;
+
   /// The maximum number of (LexLevel 0) tokens before issuing a -Wmax-tokens
   /// warning, or zero for unlimited.
   unsigned MaxTokens = 0;
@@ -1038,6 +1041,8 @@ class Preprocessor {
 OnToken = std::move(F);
   }
 
+  void setPreprocessToken(bool Preprocess) { PreprocessToken = Preprocess; }
+
   bool isMacroDefined(StringRef Id) {
 return isMacroDefined((Id));
   }

diff  --git a/clang/lib/CodeGen/CodeGenAction.cpp 
b/clang/lib/CodeGen/CodeGenAction.cpp
index 55925110708e..5a6ce0f5dbd5 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -990,11 +990,9 @@ CodeGenAction::CreateASTConsumer(CompilerInstance , 
StringRef InFile) {
 
   CoverageSourceInfo *CoverageInfo = nullptr;
   // Add the preprocessor callback only when the coverage mapping is generated.
-  if (CI.getCodeGenOpts().CoverageMapping) {
-CoverageInfo = new CoverageSourceInfo;
-CI.getPreprocessor().addPPCallbacks(
-
std::unique_ptr(CoverageInfo));
-  }
+  if (CI.getCodeGenOpts().CoverageMapping)
+CoverageInfo = CodeGen::CoverageMappingModuleGen::setUpCoverageCallbacks(
+CI.getPreprocessor());
 
   std::unique_ptr Result(new BackendConsumer(
   BA, CI.getDiagnostics(), CI.getHeaderSearchOpts(),

diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 78b268f423cb..6729c7f381f5 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -35,8 +35,40 @@ using namespace clang;
 using namespace CodeGen;
 using namespace llvm::coverage;
 
+CoverageSourceInfo *
+CoverageMappingModuleGen::setUpCoverageCallbacks(Preprocessor ) {
+  CoverageSourceInfo *CoverageInfo = new CoverageSourceInfo;
+  PP.addPPCallbacks(std::unique_ptr(CoverageInfo));
+  PP.addCommentHandler(CoverageInfo);
+  PP.setPreprocessToken(true);
+  PP.setTokenWatcher([CoverageInfo](clang::Token Tok) {
+// Update previous token location.
+CoverageInfo->PrevTokLoc = Tok.getLocation();
+CoverageInfo->updateNextTokLoc(Tok.getLocation());
+  });
+  return CoverageInfo;
+}
+
 void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range, SourceLocation) 
{
-  SkippedRanges.push_back(Range);
+  SkippedRanges.push_back({Range});
+}
+
+bool 

[PATCH] D84225: [CFE] Add nomerge function attribute to inline assembly.

[Pengfei Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG18581fd2c441: [CFE] Add nomerge function attribute to inline 
assembly. (authored by pengfei).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84225

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGen/attr-nomerge.cpp


Index: clang/test/CodeGen/attr-nomerge.cpp
===
--- clang/test/CodeGen/attr-nomerge.cpp
+++ clang/test/CodeGen/attr-nomerge.cpp
@@ -10,6 +10,7 @@
   [[clang::nomerge]] f(bar(), bar());
   [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the 
anonymous function call
   [[clang::nomerge]] for (bar(); bar(); bar()) {}
+  [[clang::nomerge]] { asm("nop"); }
   bar();
 }
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR:[0-9]+]]
@@ -22,5 +23,7 @@
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call void asm {{.*}} #[[NOMERGEATTR2:[0-9]+]]
 // CHECK: call zeroext i1 @_Z3barv()
 // CHECK: attributes #[[NOMERGEATTR]] = { nomerge }
+// CHECK: attributes #[[NOMERGEATTR2]] = { nomerge nounwind }
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -183,6 +183,7 @@
   bool foundCallExpr() { return FoundCallExpr; }
 
   void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
+  void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; }
 
   void Visit(const Stmt *St) {
 if (!St)
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1954,12 +1954,16 @@
 }
 
 static void UpdateAsmCallInst(llvm::CallBase , bool HasSideEffect,
-  bool ReadOnly, bool ReadNone, const AsmStmt ,
+  bool ReadOnly, bool ReadNone, bool NoMerge,
+  const AsmStmt ,
   const std::vector ,
   CodeGenFunction ,
   std::vector ) {
   Result.addAttribute(llvm::AttributeList::FunctionIndex,
   llvm::Attribute::NoUnwind);
+  if (NoMerge)
+Result.addAttribute(llvm::AttributeList::FunctionIndex,
+llvm::Attribute::NoMerge);
   // Attach readnone and readonly attributes.
   if (!HasSideEffect) {
 if (ReadNone)
@@ -2334,12 +2338,14 @@
 Builder.CreateCallBr(IA, Fallthrough, Transfer, Args);
 EmitBlock(Fallthrough);
 UpdateAsmCallInst(cast(*Result), HasSideEffect, ReadOnly,
-  ReadNone, S, ResultRegTypes, *this, RegResults);
+  ReadNone, InNoMergeAttributedStmt, S, ResultRegTypes,
+  *this, RegResults);
   } else {
 llvm::CallInst *Result =
 Builder.CreateCall(IA, Args, getBundlesForFunclet(IA));
 UpdateAsmCallInst(cast(*Result), HasSideEffect, ReadOnly,
-  ReadNone, S, ResultRegTypes, *this, RegResults);
+  ReadNone, InNoMergeAttributedStmt, S, ResultRegTypes,
+  *this, RegResults);
   }
 
   assert(RegResults.size() == ResultRegTypes.size());


Index: clang/test/CodeGen/attr-nomerge.cpp
===
--- clang/test/CodeGen/attr-nomerge.cpp
+++ clang/test/CodeGen/attr-nomerge.cpp
@@ -10,6 +10,7 @@
   [[clang::nomerge]] f(bar(), bar());
   [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the anonymous function call
   [[clang::nomerge]] for (bar(); bar(); bar()) {}
+  [[clang::nomerge]] { asm("nop"); }
   bar();
 }
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR:[0-9]+]]
@@ -22,5 +23,7 @@
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call void asm {{.*}} #[[NOMERGEATTR2:[0-9]+]]
 // CHECK: call zeroext i1 @_Z3barv()
 // CHECK: attributes #[[NOMERGEATTR]] = { nomerge }
+// CHECK: attributes #[[NOMERGEATTR2]] = { nomerge nounwind }
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -183,6 +183,7 @@
   bool foundCallExpr() { return FoundCallExpr; }
 
   void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
+  void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; }
 
   void Visit(const Stmt *St) {
 if (!St)
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ 

[clang] 18581fd - [CFE] Add nomerge function attribute to inline assembly.

[via cfe-commits]

Author: Wang, Pengfei
Date: 2020-07-22T08:22:58+08:00
New Revision: 18581fd2c441eac052a25e4cbe9bd74d6ff605ad

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

LOG: [CFE] Add nomerge function attribute to inline assembly.

Sometimes we also want to avoid merging inline assembly. This patch add
the nomerge function attribute to inline assembly.

Reviewed By: zequanwu

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

Added: 


Modified: 
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Sema/SemaStmtAttr.cpp
clang/test/CodeGen/attr-nomerge.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 672909849bb7..c2bd17a238d0 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -1954,12 +1954,16 @@ static llvm::MDNode *getAsmSrcLocInfo(const 
StringLiteral *Str,
 }
 
 static void UpdateAsmCallInst(llvm::CallBase , bool HasSideEffect,
-  bool ReadOnly, bool ReadNone, const AsmStmt ,
+  bool ReadOnly, bool ReadNone, bool NoMerge,
+  const AsmStmt ,
   const std::vector ,
   CodeGenFunction ,
   std::vector ) {
   Result.addAttribute(llvm::AttributeList::FunctionIndex,
   llvm::Attribute::NoUnwind);
+  if (NoMerge)
+Result.addAttribute(llvm::AttributeList::FunctionIndex,
+llvm::Attribute::NoMerge);
   // Attach readnone and readonly attributes.
   if (!HasSideEffect) {
 if (ReadNone)
@@ -2334,12 +2338,14 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt ) {
 Builder.CreateCallBr(IA, Fallthrough, Transfer, Args);
 EmitBlock(Fallthrough);
 UpdateAsmCallInst(cast(*Result), HasSideEffect, ReadOnly,
-  ReadNone, S, ResultRegTypes, *this, RegResults);
+  ReadNone, InNoMergeAttributedStmt, S, ResultRegTypes,
+  *this, RegResults);
   } else {
 llvm::CallInst *Result =
 Builder.CreateCall(IA, Args, getBundlesForFunclet(IA));
 UpdateAsmCallInst(cast(*Result), HasSideEffect, ReadOnly,
-  ReadNone, S, ResultRegTypes, *this, RegResults);
+  ReadNone, InNoMergeAttributedStmt, S, ResultRegTypes,
+  *this, RegResults);
   }
 
   assert(RegResults.size() == ResultRegTypes.size());

diff  --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index e9d3c755eb23..eb56245e1954 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -183,6 +183,7 @@ class CallExprFinder : public 
ConstEvaluatedExprVisitor {
   bool foundCallExpr() { return FoundCallExpr; }
 
   void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; }
+  void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; }
 
   void Visit(const Stmt *St) {
 if (!St)

diff  --git a/clang/test/CodeGen/attr-nomerge.cpp 
b/clang/test/CodeGen/attr-nomerge.cpp
index 284ce7827966..3405ea737df1 100644
--- a/clang/test/CodeGen/attr-nomerge.cpp
+++ b/clang/test/CodeGen/attr-nomerge.cpp
@@ -10,6 +10,7 @@ void foo(int i) {
   [[clang::nomerge]] f(bar(), bar());
   [[clang::nomerge]] [] { bar(); bar(); }(); // nomerge only applies to the 
anonymous function call
   [[clang::nomerge]] for (bar(); bar(); bar()) {}
+  [[clang::nomerge]] { asm("nop"); }
   bar();
 }
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR:[0-9]+]]
@@ -22,5 +23,7 @@ void foo(int i) {
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
 // CHECK: call zeroext i1 @_Z3barv() #[[NOMERGEATTR]]
+// CHECK: call void asm {{.*}} #[[NOMERGEATTR2:[0-9]+]]
 // CHECK: call zeroext i1 @_Z3barv()
 // CHECK: attributes #[[NOMERGEATTR]] = { nomerge }
+// CHECK: attributes #[[NOMERGEATTR2]] = { nomerge nounwind }



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Vedant Kumar via Phabricator via cfe-commits]

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

Thanks, lgtm.


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

https://reviews.llvm.org/D83592



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

[PATCH] D84087: [NFC] Clean up doc comment and implementation for Module::isSubModuleOf.

[Varun Gandhi via Phabricator via cfe-commits]

varungandhi-apple added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84087



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

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

[Logan Smith via Phabricator via cfe-commits]

logan-5 added a comment.

In D84244#2165592 , @dblaikie wrote:

> Looks OK (for future reference, this sort of stuff should've been cleaned up 
> before enabling the flag so as to avoid this kind of hurry/breakage/etc)


Loud and clear. I honestly thought I had sifted through and dealt with all 
these before enabling it, but I was wrong. My mistake, I'm sorry about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84244



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

[PATCH] D84277: [PowerPC][Power10] Fix vins*vlx instructions to have i32 arguments.

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: power-llvm-team, nemanjai, PowerPC, rzurob.
amyk added projects: LLVM, clang, PowerPC.
Herald added subscribers: shchenz, hiraditya.

Previously, the vins*vlx instructions were incorrectly defined with i64 as the 
second argument. 
This patches fixes this issue by correcting the second argument of the vins*vlx 
instructions/intrinsics to be i32.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84277

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-p10permute.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-p10permute.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-p10permute.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-p10permute.ll
@@ -170,67 +170,67 @@
 }
 declare <2 x i64> @llvm.ppc.altivec.vinsdrx(<2 x i64>, i64, i64)
 
-define <16 x i8> @testVINSBVLX(<16 x i8> %a, i64 %b, <16 x i8> %c) {
+define <16 x i8> @testVINSBVLX(<16 x i8> %a, i32 %b, <16 x i8> %c) {
 ; CHECK-LABEL: testVINSBVLX:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:vinsbvlx v2, r5, v3
 ; CHECK-NEXT:blr
 entry:
-  %0 = tail call <16 x i8> @llvm.ppc.altivec.vinsbvlx(<16 x i8> %a, i64 %b, <16 x i8> %c)
+  %0 = tail call <16 x i8> @llvm.ppc.altivec.vinsbvlx(<16 x i8> %a, i32 %b, <16 x i8> %c)
   ret <16 x i8> %0
 }
-declare <16 x i8> @llvm.ppc.altivec.vinsbvlx(<16 x i8>, i64, <16 x i8>)
+declare <16 x i8> @llvm.ppc.altivec.vinsbvlx(<16 x i8>, i32, <16 x i8>)
 
-define <16 x i8> @testVINSBVRX(<16 x i8> %a, i64 %b, <16 x i8> %c) {
+define <16 x i8> @testVINSBVRX(<16 x i8> %a, i32 %b, <16 x i8> %c) {
 ; CHECK-LABEL: testVINSBVRX:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:vinsbvrx v2, r5, v3
 ; CHECK-NEXT:blr
 entry:
-  %0 = tail call <16 x i8> @llvm.ppc.altivec.vinsbvrx(<16 x i8> %a, i64 %b, <16 x i8> %c)
+  %0 = tail call <16 x i8> @llvm.ppc.altivec.vinsbvrx(<16 x i8> %a, i32 %b, <16 x i8> %c)
   ret <16 x i8> %0
 }
-declare <16 x i8> @llvm.ppc.altivec.vinsbvrx(<16 x i8>, i64, <16 x i8>)
+declare <16 x i8> @llvm.ppc.altivec.vinsbvrx(<16 x i8>, i32, <16 x i8>)
 
-define <8 x i16> @testVINSHVLX(<8 x i16> %a, i64 %b, <8 x i16> %c) {
+define <8 x i16> @testVINSHVLX(<8 x i16> %a, i32 %b, <8 x i16> %c) {
 ; CHECK-LABEL: testVINSHVLX:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:vinshvlx v2, r5, v3
 ; CHECK-NEXT:blr
 entry:
-  %0 = tail call <8 x i16> @llvm.ppc.altivec.vinshvlx(<8 x i16> %a, i64 %b, <8 x i16> %c)
+  %0 = tail call <8 x i16> @llvm.ppc.altivec.vinshvlx(<8 x i16> %a, i32 %b, <8 x i16> %c)
   ret <8 x i16> %0
 }
-declare <8 x i16> @llvm.ppc.altivec.vinshvlx(<8 x i16>, i64, <8 x i16>)
+declare <8 x i16> @llvm.ppc.altivec.vinshvlx(<8 x i16>, i32, <8 x i16>)
 
-define <8 x i16> @testVINSHVRX(<8 x i16> %a, i64 %b, <8 x i16> %c) {
+define <8 x i16> @testVINSHVRX(<8 x i16> %a, i32 %b, <8 x i16> %c) {
 entry:
-  %0 = tail call <8 x i16> @llvm.ppc.altivec.vinshvrx(<8 x i16> %a, i64 %b, <8 x i16> %c)
+  %0 = tail call <8 x i16> @llvm.ppc.altivec.vinshvrx(<8 x i16> %a, i32 %b, <8 x i16> %c)
   ret <8 x i16> %0
 }
-declare <8 x i16> @llvm.ppc.altivec.vinshvrx(<8 x i16>, i64, <8 x i16>)
+declare <8 x i16> @llvm.ppc.altivec.vinshvrx(<8 x i16>, i32, <8 x i16>)
 
-define <4 x i32> @testVINSWVLX(<4 x i32> %a, i64 %b, <4 x i32> %c) {
+define <4 x i32> @testVINSWVLX(<4 x i32> %a, i32 %b, <4 x i32> %c) {
 ; CHECK-LABEL: testVINSWVLX:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:vinswvlx v2, r5, v3
 ; CHECK-NEXT:blr
 entry:
-  %0 = tail call <4 x i32> @llvm.ppc.altivec.vinswvlx(<4 x i32> %a, i64 %b, <4 x i32> %c)
+  %0 = tail call <4 x i32> @llvm.ppc.altivec.vinswvlx(<4 x i32> %a, i32 %b, <4 x i32> %c)
   ret <4 x i32> %0
 }
-declare <4 x i32> @llvm.ppc.altivec.vinswvlx(<4 x i32>, i64, <4 x i32>)
+declare <4 x i32> @llvm.ppc.altivec.vinswvlx(<4 x i32>, i32, <4 x i32>)
 
-define <4 x i32> @testVINSWVRX(<4 x i32> %a, i64 %b, <4 x i32> %c) {
+define <4 x i32> @testVINSWVRX(<4 x i32> %a, i32 %b, <4 x i32> %c) {
 ; CHECK-LABEL: testVINSWVRX:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:vinswvrx v2, r5, v3
 ; CHECK-NEXT:blr
 entry:
-  %0 = tail call <4 x i32> @llvm.ppc.altivec.vinswvrx(<4 x i32> %a, i64 %b, <4 x i32> %c)
+  %0 = tail call <4 x i32> @llvm.ppc.altivec.vinswvrx(<4 x i32> %a, i32 %b, <4 x i32> %c)
   ret <4 x i32> %0
 }
-declare <4 x i32> @llvm.ppc.altivec.vinswvrx(<4 x i32>, i64, <4 x i32>)
+declare <4 x i32> @llvm.ppc.altivec.vinswvrx(<4 x i32>, i32, <4 x i32>)
 
 define <4 x i32> @testVINSW(<4 x i32> %a, i32 %b) {
 ; CHECK-LABEL: testVINSW:
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -245,7 +245,7 @@
 // VX-Form: 

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

[David Blaikie via Phabricator via cfe-commits]

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

Looks OK (for future reference, this sort of stuff should've been cleaned up 
before enabling the flag so as to avoid this kind of hurry/breakage/etc)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84244



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

[clang] a361aa5 - [clang] Disable -Wsuggest-override for unittests/

[Logan Smith via cfe-commits]

Author: Logan Smith
Date: 2020-07-21T16:38:35-07:00
New Revision: a361aa5249856e333a373df90947dabf34cd6aab

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

LOG: [clang] Disable -Wsuggest-override for unittests/

Added: 


Modified: 
clang/unittests/CMakeLists.txt

Removed: 




diff  --git a/clang/unittests/CMakeLists.txt b/clang/unittests/CMakeLists.txt
index 4c222e24599f..64168f44f843 100644
--- a/clang/unittests/CMakeLists.txt
+++ b/clang/unittests/CMakeLists.txt
@@ -10,6 +10,10 @@ if(CLANG_BUILT_STANDALONE)
   endif()
 endif()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_definitions("-Wno-suggest-override")
+endif()
+
 # add_clang_unittest(test_dirname file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against the clang



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

[PATCH] D84244: [clang] Disable -Wsuggest-override for unittests/

[Logan Smith via Phabricator via cfe-commits]

logan-5 added a comment.

Committing this now to fix the bots, as per discussion in 
https://reviews.llvm.org/D84126. I'll happily revert and approach it 
differently later if anyone requests that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84244



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 279661.
zequanwu added a comment.

remove unnecessary header.


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

https://reviews.llvm.org/D83592

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h
  clang/lib/Lex/Preprocessor.cpp
  clang/test/CoverageMapping/break.c
  clang/test/CoverageMapping/builtinmacro.c
  clang/test/CoverageMapping/classtemplate.cpp
  clang/test/CoverageMapping/comment-in-macro.c
  clang/test/CoverageMapping/continue.c
  clang/test/CoverageMapping/coroutine.cpp
  clang/test/CoverageMapping/deferred-region.cpp
  clang/test/CoverageMapping/if.cpp
  clang/test/CoverageMapping/includehell.cpp
  clang/test/CoverageMapping/label.cpp
  clang/test/CoverageMapping/logical.cpp
  clang/test/CoverageMapping/loops.cpp
  clang/test/CoverageMapping/macro-expressions.cpp
  clang/test/CoverageMapping/macroparams2.c
  clang/test/CoverageMapping/macros.c
  clang/test/CoverageMapping/macroscopes.cpp
  clang/test/CoverageMapping/moremacros.c
  clang/test/CoverageMapping/objc.m
  clang/test/CoverageMapping/pr32679.cpp
  clang/test/CoverageMapping/preprocessor.c
  clang/test/CoverageMapping/return.c
  clang/test/CoverageMapping/switch.cpp
  clang/test/CoverageMapping/switchmacro.c
  clang/test/CoverageMapping/test.c
  clang/test/CoverageMapping/trycatch.cpp
  clang/test/CoverageMapping/unreachable-macro.c
  clang/test/CoverageMapping/while.c
  clang/test/lit.cfg.py
  compiler-rt/test/profile/Inputs/instrprof-comdat.h
  compiler-rt/test/profile/coverage_comments.cpp

Index: compiler-rt/test/profile/coverage_comments.cpp
===
--- /dev/null
+++ compiler-rt/test/profile/coverage_comments.cpp
@@ -0,0 +1,71 @@
+// RUN: %clangxx_profgen -fcoverage-mapping -Wno-comment -o %t %s
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw
+// RUN: llvm-cov show %t -instr-profile %t.profdata -path-equivalence=/tmp,%S 2>&1 | FileCheck %s
+
+int main() {   // CHECK:  [[# @LINE]]| 1|int main() {
+/* comment */ int x = 0;   // CHECK:  [[# @LINE]]| 1|  /* comment */ int x = 0;
+int y = 0; /* comment */   // CHECK:  [[# @LINE]]| 1|  int y = 0; /* comment */
+int z = 0; // comment  // CHECK:  [[# @LINE]]| 1|  int z = 0; // comment
+// comment // CHECK:  [[# @LINE]]|  |  // comment
+   // CHECK:  [[# @LINE]]|  |
+x = 0; /*  // CHECK:  [[# @LINE]]| 1|  x = 0; /*
+comment// CHECK:  [[# @LINE]]|  |comment
+*/ // CHECK:  [[# @LINE]]|  |*/
+   // CHECK:  [[# @LINE]]|  |
+/* // CHECK:  [[# @LINE]]|  |/*
+comment// CHECK:  [[# @LINE]]|  |comment
+*/ x = 0;  // CHECK:  [[# @LINE]]| 1|*/ x = 0;
+   // CHECK:  [[# @LINE]]|  |
+/* comment */  // CHECK:  [[# @LINE]]|  |/* comment */
+// comment // CHECK:  [[# @LINE]]|  |// comment
+/* comment */  // CHECK:  [[# @LINE]]|  |/* comment */
+z =// CHECK:  [[# @LINE]]| 1|z =
+x // comment   // CHECK:  [[# @LINE]]| 1|x // comment
+// comment // CHECK:  [[# @LINE]]|  |// comment
++ /*   // CHECK:  [[# @LINE]]| 1|+ /*
+comment// CHECK:  [[# @LINE]]|  |comment
+*/ // CHECK:  [[# @LINE]]|  |*/
+/* // CHECK:  [[# @LINE]]|  |/*
+comment// CHECK:  [[# @LINE]]|  |comment
+*/y;   // CHECK:  [[# @LINE]]| 1|*/y;
+   // CHECK:  [[# @LINE]]|  |
+// Comments inside directives. // CHECK:  [[# @LINE]]|  |// Comments inside directives.
+#if 0 //comment// CHECK:  [[# @LINE]]|  |#if 0 //comment
+/* comment */ x = 0;   // CHECK:  [[# @LINE]]|  |/* comment */ x = 0;
+y = 0; /* comment */   // CHECK:  [[# @LINE]]|  |y = 0; /* comment */
+z = 0; // comment  // CHECK:  [[# @LINE]]|  |z = 0; // comment
+// comment // CHECK:  [[# @LINE]]|  |// comment
+   // CHECK:  [[# @LINE]]|  |
+x = 0; /*  // CHECK:  [[# @LINE]]|  |x = 0; /*
+comment// CHECK:  [[# @LINE]]|  |

[PATCH] D83814: [clangd] Add Random Forest runtime for code completion.

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Is there some additional context for what this patch is aiming to do? The 
description sounds interesting but I don't really understand it.

What is a "feature" in this context?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83814



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

[PATCH] D84087: [NFC] Clean up doc comment and implementation for Module::isSubModuleOf.

[Adrian Prantl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb907ad539a90: [NFC] Clean up doc comment and implementation 
for Module::isSubModuleOf. (authored by aprantl).

Changed prior to commit:
  https://reviews.llvm.org/D84087?vs=278972=279659#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84087

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/Basic/Module.cpp


Index: clang/lib/Basic/Module.cpp
===
--- clang/lib/Basic/Module.cpp
+++ clang/lib/Basic/Module.cpp
@@ -173,14 +173,10 @@
 }
 
 bool Module::isSubModuleOf(const Module *Other) const {
-  const Module *This = this;
-  do {
-if (This == Other)
+  for (auto *Parent = this; Parent; Parent = Parent->Parent) {
+if (Parent == Other)
   return true;
-
-This = This->Parent;
-  } while (This);
-
+  }
   return false;
 }
 
Index: clang/include/clang/Basic/Module.h
===
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -457,8 +457,12 @@
   /// Determine whether this module is a submodule.
   bool isSubModule() const { return Parent != nullptr; }
 
-  /// Determine whether this module is a submodule of the given other
-  /// module.
+  /// Check if this module is a (possibly transitive) submodule of \p Other.
+  ///
+  /// The 'A is a submodule of B' relation is a partial order based on the
+  /// the parent-child relationship between individual modules.
+  ///
+  /// Returns \c false if \p Other is \c nullptr.
   bool isSubModuleOf(const Module *Other) const;
 
   /// Determine whether this module is a part of a framework,


Index: clang/lib/Basic/Module.cpp
===
--- clang/lib/Basic/Module.cpp
+++ clang/lib/Basic/Module.cpp
@@ -173,14 +173,10 @@
 }
 
 bool Module::isSubModuleOf(const Module *Other) const {
-  const Module *This = this;
-  do {
-if (This == Other)
+  for (auto *Parent = this; Parent; Parent = Parent->Parent) {
+if (Parent == Other)
   return true;
-
-This = This->Parent;
-  } while (This);
-
+  }
   return false;
 }
 
Index: clang/include/clang/Basic/Module.h
===
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -457,8 +457,12 @@
   /// Determine whether this module is a submodule.
   bool isSubModule() const { return Parent != nullptr; }
 
-  /// Determine whether this module is a submodule of the given other
-  /// module.
+  /// Check if this module is a (possibly transitive) submodule of \p Other.
+  ///
+  /// The 'A is a submodule of B' relation is a partial order based on the
+  /// the parent-child relationship between individual modules.
+  ///
+  /// Returns \c false if \p Other is \c nullptr.
   bool isSubModuleOf(const Module *Other) const;
 
   /// Determine whether this module is a part of a framework,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b907ad5 - [NFC] Clean up doc comment and implementation for Module::isSubModuleOf.

[Adrian Prantl via cfe-commits]

Author: Adrian Prantl
Date: 2020-07-21T16:23:36-07:00
New Revision: b907ad539a900279443dc8ef8816b6b5a76b1ea1

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

LOG: [NFC] Clean up doc comment and implementation for Module::isSubModuleOf.

Patch by Varun Gandhi!

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

Added: 


Modified: 
clang/include/clang/Basic/Module.h
clang/lib/Basic/Module.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Module.h 
b/clang/include/clang/Basic/Module.h
index 6b932a9a84d0..94dd21537966 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -457,8 +457,12 @@ class Module {
   /// Determine whether this module is a submodule.
   bool isSubModule() const { return Parent != nullptr; }
 
-  /// Determine whether this module is a submodule of the given other
-  /// module.
+  /// Check if this module is a (possibly transitive) submodule of \p Other.
+  ///
+  /// The 'A is a submodule of B' relation is a partial order based on the
+  /// the parent-child relationship between individual modules.
+  ///
+  /// Returns \c false if \p Other is \c nullptr.
   bool isSubModuleOf(const Module *Other) const;
 
   /// Determine whether this module is a part of a framework,

diff  --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index b3daaa3a4442..b25248de6832 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -173,14 +173,10 @@ bool Module::isAvailable(const LangOptions , 
const TargetInfo ,
 }
 
 bool Module::isSubModuleOf(const Module *Other) const {
-  const Module *This = this;
-  do {
-if (This == Other)
+  for (auto *Parent = this; Parent; Parent = Parent->Parent) {
+if (Parent == Other)
   return true;
-
-This = This->Parent;
-  } while (This);
-
+  }
   return false;
 }
 



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

[PATCH] D84082: [tsan] Allow TSan in the Clang driver for Apple Silicon Macs

[Kuba (Brecka) Mracek via Phabricator via cfe-commits]

kubamracek added a comment.

Let's handle simulators separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84082



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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D71726#2165494 , @jyknight wrote:

> In D71726#2165445 , @yaxunl wrote:
>
> > In D71726#2165424 , @jyknight 
> > wrote:
> >
> > > Why not have clang always emit atomicrmw for floats, and let 
> > > AtomicExpandPass handle legalizing that into integer atomics if 
> > > necessary, rather than adding a target hook in clang?
> >
> >
> > Not all targets can legalize fp atomics by AtomicExpandPass. Some targets 
> > need library support.
>
>
> That isn't true, because you can do so generically with a cmpxchg loop, 
> assuming that size of atomic is supported by the target. This might not be 
> the most efficient lowering choice, but it's always possible as a fallback. 
> (And if the size is too large, then AtomicExpandPass will lower the cmpxchg 
> to the libatomic call.)
>
> If a target wants to tell AtomicExpandPass that fp add/sub are supported, and 
> then lower the resulting ATOMIC_LOAD_FSUB sdag node into a libcall of its 
> choice, that's also ok (as long as the libcall is lock-free).


how about other fp types e.g. bf16, half, long double? Do we need to diagnose 
them or not?


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

https://reviews.llvm.org/D71726



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

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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3842-3846
+  if (EmitDwarf &&
+  Args.hasFlag(options::OPT_fno_eliminate_unused_debug_types,
+   options::OPT_feliminate_unused_debug_types, false) &&
+  DebugInfoKind >= codegenoptions::DebugInfoConstructor)
+DebugInfoKind = codegenoptions::UnusedTypeInfo;

Would this be tidier if it were rolled into the if/checking around 380 since 
it's a very similar option?



Comment at: clang/test/CodeGen/debug-info-unused-types.cpp:24-30
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
+// NODBG-NOT: !DIEnumerator(name: "BAZ"
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// NODBG-NOT: !DIEnumerator(name: "Z"
+// NODBG-NOT: !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_class_type, name: "y"

Maybe simpler to test the NODBG by `NODBG-NOT: DI{{[a-zA-Z]*}}Type` ? Not sure 
if that'd quite work, but might be adequate.



Comment at: clang/test/CodeGen/debug-info-unused-types.cpp:15-21
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
+// CHECK: !DIEnumerator(name: "BAZ"
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// CHECK: !DIEnumerator(name: "Z"
+// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "y"

nickdesaulniers wrote:
> dblaikie wrote:
> > nickdesaulniers wrote:
> > > dblaikie wrote:
> > > > Probably good to check these end up in the retained types list
> > > What's the best way to check this?  In particular, I worry about the 
> > > `!` number changing in the future, resulting in this test 
> > > spuriously failing.  For this test case, we have:
> > > 
> > > ```
> > > !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, 
> > > producer: "Nick Desaulniers clang version 12.0.0 
> > > (g...@github.com:llvm/llvm-project.git 
> > > e541558d11ca82b3664b907b350da5187f23c0c9)", isOptimized: false, 
> > > runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: 
> > > !15, splitDebugInlining: false, nameTableKind: None)
> > > ```
> > > which tells us the `retainedTypes` is `!15`. `!15` looks like:
> > > ```
> > > !15 = !{!16, !17, !3, !5, !18, !8}
> > > ```
> > > In this case, should I just add the numbers?  If we used regex patterns 
> > > `[0-9]`, I worry that we couldn't ensure the list would match.  Is it ok 
> > > to just hardcode these?
> > You can use captures in FileCheck to make references a little more 
> > flexible. Surprised there aren't /any/ tests in LLVM testing the 
> > retainedTypes list... test/CodeGen/debug-info-extern-call.c used to use the 
> > retained types list & still has some testing remnants of it you could draw 
> > from, something like::
> > ```
> > // CHECK: !DICompileUnit({{.*}}retainedTypes: [[RETTYPES:![0-9]+]]
> > // CHECK: [[RETTYPES]] = !{[[T1:![0-9]+]], [[T2:![0-9]+]], ...}
> > ...
> > // CHECK: [[T1]] = !DICompositeType(...
> > ```
> > 
> > It doesn't protect the test from changes in the order of the retained types 
> > list but otherwise resilient to extraneous metadata nodes being added or 
> > removed.
> So one issue I'm running into with this approach is that the retained type 
> list is emitted in between the other debug nodes (some before it, some after 
> it).  The capture logic seems to have an issue referring to captures that 
> have yet to be defined yet.
> 
> note: uses undefined variable(s): "TYPE7"
> 
> but if I move the `CHECK` for the retained types last, then `FileCheck` 
> errors because it seems it only doesn't one pass for the `CHECK` statements 
> (IIUC), so it's "too late" to parse.
> 
> Is there another way to test this, or am I holding it wrong?
Order dependence is a currently unavoidable part of FileCheck tests, so if the 
content you're trying to test (I'm not entirely clear from your description - 
so guessing a bit) looks something like:
```
!1 = DICompileUnit ... retainedTypes: !3
!2 = DICompositeType "x"
!3 = !{!2, !4}
!4 = DICompositeType "y"
```
Then you'd CHECK this something like:
```
; CHECK: DICompileUnit {{.*}} retainedTypes: [[RET_TY:![0-9]*]]
; CHECK: [[X:![0-9]*]] = DICompositeType "x"
; CHECK: [[RET_TY]] = !{[[X]], [[Y:![0-9]*]]}
; CHECK: [[Y]] = DICompositeType "y"
```
Yes, this does mean that if the types were emitted in a different order, the 
test would fail - but that's a limitation we're living with for pretty much all 
LLVM and Clang testing at the moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80242



___
cfe-commits mailing 

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D84192#2165517 , @cchen wrote:

> In D84192#2165438 , @ABataev wrote:
>
> > In D84192#2165396 , @cchen wrote:
> >
> > > In D84192#2165249 , @ABataev 
> > > wrote:
> > >
> > > > In D84192#2165235 , @cchen 
> > > > wrote:
> > > >
> > > > > In D84192#2164885 , @ABataev 
> > > > > wrote:
> > > > >
> > > > > > Also, what about compatibility with declare mapper? Can you add 
> > > > > > tests for it?
> > > > >
> > > > >
> > > > > There's a restriction for map clause that non-contiguous is not 
> > > > > allowed and I guess it also applies to declare mapper.
> > > >
> > > >
> > > > I see that to/from clauses allow to use mappers too:
> > > >
> > > >   to([mapper(mapper-identifier):]locator-list) 
> > > > from([mapper(mapper-identifier):]locator-list
> > > >   
> > >
> > >
> > > I'm confused of how to use to([mapper(mapper-identifier):]locator-list) 
> > > with array section.
> > >  For this case:
> > >
> > >   class C {
> > >   public:
> > > int a;
> > > double b[3][4][5];
> > >   };
> > >  
> > >   #pragma omp declare mapper(id: C s) map(s.a, s.b[0:3][0:4][0:5])
> > >  
> > >   #pragma omp target update from(mapper(id): c)
> > >
> > >
> > > Clang already has a semantic check in from clause when mapper is used: 
> > > "mapper type must be of struct, union or class type".
> > >  So the only item I can put in from clause in the above example is `c` 
> > > and I cannot put c.b[0:2][1:2][0:2] or any even c.b[0:2].
> >
> >
> > Does clang compile your example? If not, shall it be allowed for to/from 
> > clauses or not?
>
>
> Clang can compile my example but the problem is that Clang do not accept 
> something like `#pragma omp target update from(mapper(id): 
> c.b[0:2][1:2][2:2])` (non-contiguous) or even `#pragma omp target update 
> from(mapper(id): c.b[2][1][0:2])` (contiguous).
>  Actually, Clang now only accepts `c` as struct/union/class type in 
> `from(mapper(id): c)`. And the reason for the restriction is from declare 
> mapper directive - "The type must be of struct, union or class type in C and 
> C++".


And it is fine. How does it work in declare mapper, the main question? Does it 
support extended array sections format mapoers with maps, to and from? Shall it 
be supported in declare mapper at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

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

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D84192#2165438 , @ABataev wrote:

> In D84192#2165396 , @cchen wrote:
>
> > In D84192#2165249 , @ABataev wrote:
> >
> > > In D84192#2165235 , @cchen wrote:
> > >
> > > > In D84192#2164885 , @ABataev 
> > > > wrote:
> > > >
> > > > > Also, what about compatibility with declare mapper? Can you add tests 
> > > > > for it?
> > > >
> > > >
> > > > There's a restriction for map clause that non-contiguous is not allowed 
> > > > and I guess it also applies to declare mapper.
> > >
> > >
> > > I see that to/from clauses allow to use mappers too:
> > >
> > >   to([mapper(mapper-identifier):]locator-list) 
> > > from([mapper(mapper-identifier):]locator-list
> > >   
> >
> >
> > I'm confused of how to use to([mapper(mapper-identifier):]locator-list) 
> > with array section.
> >  For this case:
> >
> >   class C {
> >   public:
> > int a;
> > double b[3][4][5];
> >   };
> >  
> >   #pragma omp declare mapper(id: C s) map(s.a, s.b[0:3][0:4][0:5])
> >  
> >   #pragma omp target update from(mapper(id): c)
> >
> >
> > Clang already has a semantic check in from clause when mapper is used: 
> > "mapper type must be of struct, union or class type".
> >  So the only item I can put in from clause in the above example is `c` and 
> > I cannot put c.b[0:2][1:2][0:2] or any even c.b[0:2].
>
>
> Does clang compile your example? If not, shall it be allowed for to/from 
> clauses or not?


Clang can compile my example but the problem is that Clang do not accept 
something like `#pragma omp target update from(mapper(id): c.b[0:2][1:2][2:2])` 
(non-contiguous) or even `#pragma omp target update from(mapper(id): 
c.b[2][1][0:2])` (contiguous).
Actually, Clang now only accepts `c` as struct/union/class type in 
`from(mapper(id): c)`. And the reason for the restriction is from declare 
mapper directive - "The type must be of struct, union or class type in C and 
C++".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

I'm afraid the patch does not work yet. For example, when the following program

  void *f() {
void g();
g();
return __builtin_return_address(0);
  }

is compiled with

  ./bin/clang -target aarch64-eabi -march=armv8.3-a  
-mbranch-protection=pac-ret -S -O2 h.c

The issue is that the definition of the instructions `XPAC{D,I}` is incorrect: 
it does not mention at all the operand to those insns.
Another problem is that the patch does not work with `-O0`. When compiling 
without optimisations, AArch64 backend used GlobalISel.

I have patches for these two issues. I'll post the one for XPAC{D,O} tomorrow 
and perhaps in a couple of days the GlobalISel one and we're good to go.




Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6119
+SDValue Reg =
+DAG.getCopyToReg(DAG.getEntryNode(), DL, AArch64::X0, ReturnAddress);
+SDNode *St = DAG.getMachineNode(AArch64::XPACI, DL, VT, Reg);

We shouldn't be hardcoding the `X0` register here.  We already have the encoded 
return address in `ReturnAddress`
can simply do:

   SDNode *St = DAG.getMachineNode(AArch64::XPACI, DL, VT, ReturnAddress);



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6124
+// XPACLRI operates on LR therefore we must move the operand accordingly.
+SDValue Reg =
+DAG.getCopyToReg(DAG.getEntryNode(), DL, AArch64::LR, ReturnAddress);

Rename `Reg` to `Chain`.


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

https://reviews.llvm.org/D75044



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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D71726#2165445 , @yaxunl wrote:

> In D71726#2165424 , @jyknight wrote:
>
> > Why not have clang always emit atomicrmw for floats, and let 
> > AtomicExpandPass handle legalizing that into integer atomics if necessary, 
> > rather than adding a target hook in clang?
>
>
> Not all targets can legalize fp atomics by AtomicExpandPass. Some targets 
> need library support.


That isn't true, because you can do so generically with a cmpxchg loop, 
assuming that size of atomic is supported by the target. This might not be the 
most efficient lowering choice, but it's always possible as a fallback. (And if 
the size is too large, then AtomicExpandPass will lower the cmpxchg to the 
libatomic call.)

If a target wants to tell AtomicExpandPass that fp add/sub are supported, and 
then lower the resulting ATOMIC_LOAD_FSUB sdag node into a libcall of its 
choice, that's also ok (as long as the libcall is lock-free).


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

https://reviews.llvm.org/D71726



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

[PATCH] D84087: [NFC] Clean up doc comment and implementation for Module::isSubModuleOf.

[Varun Gandhi via Phabricator via cfe-commits]

varungandhi-apple added a comment.

(Could you land the patch if it looks good? I don't have commit access.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84087



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

[PATCH] D84087: [NFC] Clean up doc comment and implementation for Module::isSubModuleOf.

[Varun Gandhi via Phabricator via cfe-commits]

varungandhi-apple marked an inline comment as done.
varungandhi-apple added inline comments.



Comment at: clang/lib/Basic/Module.cpp:176
 bool Module::isSubModuleOf(const Module *Other) const {
-  const Module *This = this;
-  do {
+  for (const Module *This = this; This != nullptr; This = This->Parent) {
 if (This == Other)

aprantl wrote:
> ```
> for (const Module *Parent = this; Parent; Parent = Parent->Parent) {
>   if (Parent == Other)
>return true;
> }
> ```
> ?
That seems confusing... I would like to avoid giving a local variable the same 
name as an instance member. I can rename `This` to `Current` if that makes it 
clearer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84087



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

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

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3825-3828
+  if (EmitDwarf &&
+  Args.hasFlag(options::OPT_fno_eliminate_unused_debug_types,
+   options::OPT_feliminate_unused_debug_types, false))
+DebugInfoKind = codegenoptions::UnusedTypeInfo;

dblaikie wrote:
> nickdesaulniers wrote:
> > dblaikie wrote:
> > > How does GCC compose this with -gmlt, for instance? I'd suspect that the 
> > > desired behavior might be for -gmlt to override this -f flag? So maybe 
> > > this should be predicated on "if (EmitDwarf && DebugInfoKind >= 
> > > codegenoptions::LimitedDebugInfo && ... "?
> > > 
> > > (so if someone wants to use only -gmlt in certain parts of their build 
> > > that doesn't get upgraded by the presence of this -f flag)
> > GCC does not yet support `-gmlt`.
> > https://godbolt.org/z/Wrv6sK
> > 
> > IIUC, `-gmlt` ("minimum line tables") is meant to minimize the amount of 
> > debug info produced.  I'm not sure what the user expects if they 
> > simultaneous ask for more debug info (via `-fno-eliminate-unused-types`) 
> > and less debug info (via `-gmlt`).
> > 
> > The current behavior is that `-fno-eliminate-unused-types` "wins" out over 
> > `-gmlt`, resulting in the "fullest" amount of debug info being produced.  
> > (Would you like me to add tests for this behavior to 
> > clang/test/Driver/debug-options.c for this behavior?)
> > 
> > If we don't want that behavior, we should reconsider whether we want 
> > `UnusedTypeInfo` to be part of the `DebugInfoKind` "progression."  I also 
> > don't understand what the suggested added condition to the predicate is 
> > supposed to do; if `EmitDebugInfo` is set, doesn't the default value of 
> > `DebugInfoConstructor` (after 227db86a1b7dd6f96f7df14890fcd071bc4fe1f5, 
> > rebased this patch on top of) mean that we wouldn't use `UnusedTypeInfo` 
> > with `-g -fno-eliminate-unused-type-info?  Or am I misunderstanding the 
> > suggestion?
> GCC's nearest equivalent is -g1 by the looks of it - I'd hazard a guess that 
> -fno-eliminate-unused-types has no effect when compiling with -g1 (as 
> -fstandalone-debug has no effect when -gmlt is specified)
> 
> I thin it's probably appropriate to implement similar behavior in Clang - 
> that -fstandalone-debug/no-standalone-debug/no-eliminate-unused-types only 
> applies to users that already asked for some kind of class debug info. It 
> looks like that functionality's already implemented correctly (at least in 
> one or two small cases I tested) for -fstandalone-debug and 
> -fno-eliminate-unused-types should follow that.
> 
> What should happen between -fstandalone-debug and -fno-eliminate-unused-types 
> is an open question I don't feel too strongly about which order they work in.
Oh! Indeed!

gcc -g1 -fno-eliminate-unused-debug-types foo.c -c -o foo.o

produces no additional debug info. `-g2` does.  Let me fix that and add tests.



Comment at: clang/test/CodeGen/debug-info-unused-types.cpp:15-21
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
+// CHECK: !DIEnumerator(name: "BAZ"
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// CHECK: !DIEnumerator(name: "Z"
+// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "y"

dblaikie wrote:
> nickdesaulniers wrote:
> > dblaikie wrote:
> > > Probably good to check these end up in the retained types list
> > What's the best way to check this?  In particular, I worry about the 
> > `!` number changing in the future, resulting in this test 
> > spuriously failing.  For this test case, we have:
> > 
> > ```
> > !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, 
> > producer: "Nick Desaulniers clang version 12.0.0 
> > (g...@github.com:llvm/llvm-project.git 
> > e541558d11ca82b3664b907b350da5187f23c0c9)", isOptimized: false, 
> > runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !15, 
> > splitDebugInlining: false, nameTableKind: None)
> > ```
> > which tells us the `retainedTypes` is `!15`. `!15` looks like:
> > ```
> > !15 = !{!16, !17, !3, !5, !18, !8}
> > ```
> > In this case, should I just add the numbers?  If we used regex patterns 
> > `[0-9]`, I worry that we couldn't ensure the list would match.  Is it ok to 
> > just hardcode these?
> You can use captures in FileCheck to make references a little more flexible. 
> Surprised there aren't /any/ tests in LLVM testing the retainedTypes list... 
> test/CodeGen/debug-info-extern-call.c used to use the retained types list & 
> still has some testing remnants of it you could draw from, something like::
> ```
> // CHECK: !DICompileUnit({{.*}}retainedTypes: [[RETTYPES:![0-9]+]]
> // CHECK: [[RETTYPES]] = !{[[T1:![0-9]+]], [[T2:![0-9]+]], ...}
> ...
> // CHECK: [[T1]] = 

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

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 279643.
nickdesaulniers marked 4 inline comments as done.
nickdesaulniers added a comment.

- rebase, add -g1 test case, don't emit unless -g2 or higher


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80242

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/CommandGuide/clang.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/DebugInfoOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-unused-types.c
  clang/test/CodeGen/debug-info-unused-types.cpp
  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
@@ -361,3 +361,12 @@
 // GEMBED_2:  error: invalid argument '-gembed-source' only allowed with '-gdwarf-5'
 // NOGEMBED_5-NOT:  "-gembed-source"
 // NOGEMBED_2-NOT:  error: invalid argument '-gembed-source' only allowed with '-gdwarf-5'
+//
+// RUN: %clang -### -g -fno-eliminate-unused-debug-types -c %s 2>&1 \
+// RUN:| FileCheck -check-prefix=DEBUG_UNUSED_TYPES %s
+// DEBUG_UNUSED_TYPES: "-debug-info-kind=unused-types"
+// DEBUG_UNUSED_TYPES-NOT: "-debug-info-kind=limited"
+// RUN: %clang -### -g -feliminate-unused-debug-types -c %s 2>&1 \
+// RUN:| FileCheck -check-prefix=NO_DEBUG_UNUSED_TYPES %s
+// NO_DEBUG_UNUSED_TYPES: "-debug-info-kind=constructor"
+// NO_DEBUG_UNUSED_TYPES-NOT: "-debug-info-kind=unused-types"
Index: clang/test/CodeGen/debug-info-unused-types.cpp
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-unused-types.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang++ -fno-eliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang++ -fno-eliminate-unused-debug-types -g1 -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang++ -feliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang++ -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang++ -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+using foo = int;
+class bar {};
+enum class baz { BAZ };
+
+void quux() {
+  using x = int;
+  class y {};
+  enum class z { Z };
+}
+
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
+// CHECK: !DIEnumerator(name: "BAZ"
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// CHECK: !DIEnumerator(name: "Z"
+// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "y"
+
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "baz"
+// NODBG-NOT: !DIEnumerator(name: "BAZ"
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// NODBG-NOT: !DIEnumerator(name: "Z"
+// NODBG-NOT: !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_class_type, name: "bar"
+// NODBG-NOT: !DICompositeType(tag: DW_TAG_class_type, name: "y"
+
+class unused_class;
+enum class unused_enum_class;
+
+// NODBG-NOT: name: "unused_
Index: clang/test/CodeGen/debug-info-unused-types.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-unused-types.c
@@ -0,0 +1,56 @@
+// RUN: %clang -fno-eliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck %s
+// RUN: %clang -fno-eliminate-unused-debug-types -g1 -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang -feliminate-unused-debug-types -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang -g -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+// RUN: %clang -emit-llvm -S -o - %s | FileCheck --check-prefix=NODBG %s
+typedef int my_int;
+struct foo {};
+enum bar { BAR };
+union baz {};
+
+void quux(void) {
+  typedef int x;
+  struct y {};
+  enum z { Z };
+  union w {};
+}
+
+// Check that debug info is emitted for the typedef, struct, enum, and union
+// when -fno-eliminate-unused-debug-types and -g are set.
+
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "bar"
+// CHECK: !DIEnumerator(name: "BAR"
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// CHECK: !DIEnumerator(name: "Z"
+// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "my_int"
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "foo"
+// CHECK: !DICompositeType(tag: DW_TAG_union_type, name: 

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D71726#2165445 , @yaxunl wrote:

> In D71726#2165424 , @jyknight wrote:
>
> > Why not have clang always emit atomicrmw for floats, and let 
> > AtomicExpandPass handle legalizing that into integer atomics if necessary, 
> > rather than adding a target hook in clang?
>
>
> Not all targets can legalize fp atomics by AtomicExpandPass. Some targets 
> need library support.


What are they missing? It can be expanded to a cmpxchg loop with bitcast to an 
integer type of the same size.


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

https://reviews.llvm.org/D71726



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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D71726#2165424 , @jyknight wrote:

> Why not have clang always emit atomicrmw for floats, and let AtomicExpandPass 
> handle legalizing that into integer atomics if necessary, rather than adding 
> a target hook in clang?


Not all targets can legalize fp atomics by AtomicExpandPass. Some targets need 
library support.


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

https://reviews.llvm.org/D71726



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

[PATCH] D52296: [Clang] - Add '-gsplit-dwarf[=split, =single]' version for '-gsplit-dwarf' option.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D52296#2165370 , @Trass3r wrote:

> In D52296#1285328 , @grimar wrote:
>
> > In D52296#1284130 , @probinson 
> > wrote:
> >
> > > Only DWARF supports this, correct?
> >
> >
> > I am not aware of any kind of debug information splitting except DWARF 
> > splitting.
>
>
> Just for the record, msvc does something very similar to this 
> -gsplit-dwarf=single option (using a linker option /DEBUG:FASTLINK instead of 
> this SHF_EXCLUDE approach).
>
> Some questions:
>
> - How is this handled by dwp/llvm-dwp? I guess not yet since neither supports 
> DWARF5 atm.
> - How does it interact with -gdwarf-5? Should it?


This functionality should be pretty orthogonal to -gdwarf-5 and dwp - though it 
may not be currently, I haven't looked/checked. But seems to me this sort of 
functionality should work with pre-standard/DWARFv4 Split DWARF as well as it 
does with DWARFv5 Split DWARF, unless I'm missing something.

And dwp tools should be made to ignore the non-.dwo-suffixed sections, but they 
may not be made that way currently, I haven't checked.

> - There is some deeply buried information about how gdb requires 
> .debug_gnu_pubnames in the case of split dwarf: 
> https://reviews.llvm.org/D54497?id=173943#1299461. Is this still true for 
> this single split case? I'm asking cause these sections are huge and I wonder 
> if they defeat the split dwarf link time gains.

I believe it still is the case, yes. Though GDB startup times are much better 
with an index with or without Split DWARF - so if you want an index anyway, the 
split (single split, or split split) might still be a good thing independent of 
that. If you have to tradeoff one or the other, yeah, that may be more 
complicated (lld, at least, last I heard, uses more memory to make gdb-index 
than gold does, so while lld's memory usage is less than gold's (& certainly 
less than bfd's) usually, with gdb-index it's a bit worse - so the tradeoffs 
might be more complicated depending on which linker you're using, etc)


Repository:
  rC Clang

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

https://reviews.llvm.org/D52296



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

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D84192#2165396 , @cchen wrote:

> In D84192#2165249 , @ABataev wrote:
>
> > In D84192#2165235 , @cchen wrote:
> >
> > > In D84192#2164885 , @ABataev 
> > > wrote:
> > >
> > > > Also, what about compatibility with declare mapper? Can you add tests 
> > > > for it?
> > >
> > >
> > > There's a restriction for map clause that non-contiguous is not allowed 
> > > and I guess it also applies to declare mapper.
> >
> >
> > I see that to/from clauses allow to use mappers too:
> >
> >   to([mapper(mapper-identifier):]locator-list) 
> > from([mapper(mapper-identifier):]locator-list
> >   
>
>
> I'm confused of how to use to([mapper(mapper-identifier):]locator-list) with 
> array section.
>  For this case:
>
>   class C {
>   public:
> int a;
> double b[3][4][5];
>   };
>  
>   #pragma omp declare mapper(id: C s) map(s.a, s.b[0:3][0:4][0:5])
>  
>   #pragma omp target update from(mapper(id): c)
>
>
> Clang already has a semantic check in from clause when mapper is used: 
> "mapper type must be of struct, union or class type".
>  So the only item I can put in from clause in the above example is `c` and I 
> cannot put c.b[0:2][1:2][0:2] or any even c.b[0:2].


Does clang compile your example? If not, shall it be allowed for to/from 
clauses or not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Why not have clang always emit atomicrmw for floats, and let AtomicExpandPass 
handle legalizing that into integer atomics if necessary, rather than adding a 
target hook in clang?


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

https://reviews.llvm.org/D71726



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

[PATCH] D83772: [Windows] Fix limit on command line size

[Max Kudryavtsev via Phabricator via cfe-commits]

max-kudr added a comment.

Thank you! @sepavloff reverted it in revision 
ac0edc55887b6961ad90fd51f349c9587b1a8a7a 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83772



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

[PATCH] D83772: [Windows] Fix limit on command line size

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added a comment.

In D83772#2164685 , @max-kudr wrote:

> This commit is now failing LLDB Windows buildbot 
>  builds 
> http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/17702. Please 
> fix or revert.
>
>   
> E:\build_slave\lldb-x64-windows-ninja\llvm-project\lldb\source\Host\windows\ProcessLauncherWindows.cpp(53):
>  error C2679: binary '=': no operator found which takes a right-hand operand 
> of type 'llvm::ErrorOr' (or there is no acceptable conversion)
>


I'll fix that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83772



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

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

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D84192#2165249 , @ABataev wrote:

> In D84192#2165235 , @cchen wrote:
>
> > In D84192#2164885 , @ABataev wrote:
> >
> > > Also, what about compatibility with declare mapper? Can you add tests for 
> > > it?
> >
> >
> > There's a restriction for map clause that non-contiguous is not allowed and 
> > I guess it also applies to declare mapper.
>
>
> I see that to/from clauses allow to use mappers too:
>
>   to([mapper(mapper-identifier):]locator-list) 
> from([mapper(mapper-identifier):]locator-list
>   


I'm confused of how to use to([mapper(mapper-identifier):]locator-list) with 
array section.
For this case:

  class C {
  public:
int a;
double b[3][4][5];
  };
  
  #pragma omp declare mapper(id: C s) map(s.a, s.b[0:3][0:4][0:5])
  
  #pragma omp target update from(mapper(id): c)

Clang already has a semantic check in from clause when mapper is used: "mapper 
type must be of struct, union or class type".
So the only item I can put in from clause in the above example is `c` and I 
cannot put c.b[0:2][1:2][0:2] or any even c.b[0:2].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D83955: [PowerPC][Power10] Implementation of 128-bit Binary Vector Multiply builtins

[Albion Fung via Phabricator via cfe-commits]

Conanap updated this revision to Diff 279637.
Conanap marked 2 inline comments as done.
Conanap added a comment.

Changed if def to use pwer10_vector


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83955

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p10vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll

Index: llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/p10-vector-multiply.ll
@@ -0,0 +1,56 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mcpu=pwr10 -ppc-asm-full-reg-names -ppc-vsr-nums-as-vr < %s | \
+; RUN:   FileCheck %s
+; This test case aims to test the vector multiply instructions on Power10.
+
+declare <1 x i128> @llvm.ppc.altivec.vmuleud(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmuloud(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmulesd(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmulosd(<2 x i64>, <2 x i64>) nounwind readnone
+declare <1 x i128> @llvm.ppc.altivec.vmsumcud(<2 x i64>, <2 x i64>, <1 x i128>) nounwind readnone
+
+define <1 x i128> @test_vmuleud(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmuleud:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmuleud v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmuleud(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmuloud(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmuloud:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmuloud v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmuloud(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmulesd(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmulesd:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmulesd v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmulesd(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmulosd(<2 x i64> %x, <2 x i64> %y) nounwind readnone {
+; CHECK-LABEL: test_vmulosd:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmulosd v2, v2, v3
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmulosd(<2 x i64> %x, <2 x i64> %y)
+  ret <1 x i128> %tmp
+}
+
+define <1 x i128> @test_vmsumcud(<2 x i64> %x, <2 x i64> %y, <1 x i128> %z) nounwind readnone {
+; CHECK-LABEL: test_vmsumcud:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmsumcud v2, v2, v3, v4
+; CHECK-NEXT:blr
+  %tmp = tail call <1 x i128> @llvm.ppc.altivec.vmsumcud(<2 x i64> %x, <2 x i64> %y, <1 x i128> %z)
+  ret <1 x i128> %tmp
+}
Index: llvm/lib/Target/PowerPC/PPCInstrPrefix.td
===
--- llvm/lib/Target/PowerPC/PPCInstrPrefix.td
+++ llvm/lib/Target/PowerPC/PPCInstrPrefix.td
@@ -993,16 +993,26 @@
   }
 
   def VMULESD : VXForm_1<968, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmulesd $vD, $vA, $vB", IIC_VecGeneral, []>;
+ "vmulesd $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmulesd v2i64:$vA,
+   v2i64:$vB))]>;
   def VMULEUD : VXForm_1<712, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmuleud $vD, $vA, $vB", IIC_VecGeneral, []>;
+ "vmuleud $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmuleud v2i64:$vA,
+   v2i64:$vB))]>;
   def VMULOSD : VXForm_1<456, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmulosd $vD, $vA, $vB", IIC_VecGeneral, []>;
+ "vmulosd $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmulosd v2i64:$vA,
+   v2i64:$vB))]>;
   def VMULOUD : VXForm_1<200, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
- "vmuloud $vD, $vA, $vB", IIC_VecGeneral, []>;
-  def VMSUMCUD : VAForm_1a<23, (outs vrrc:$vD),
-   (ins vrrc:$vA, vrrc:$vB, vrrc:$vC),
-   "vmsumcud $vD, $vA, $vB, $vC", IIC_VecGeneral, []>;
+ "vmuloud $vD, $vA, $vB", IIC_VecGeneral,
+ [(set v1i128:$vD, (int_ppc_altivec_vmuloud v2i64:$vA,
+   v2i64:$vB))]>;
+  def VMSUMCUD : VAForm_1a<23, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB, vrrc:$vC),
+  

[PATCH] D52296: [Clang] - Add '-gsplit-dwarf[=split, =single]' version for '-gsplit-dwarf' option.

[Trass3r via Phabricator via cfe-commits]

Trass3r added a comment.
Herald added a subscriber: dang.

In D52296#1285328 , @grimar wrote:

> In D52296#1284130 , @probinson wrote:
>
> > Only DWARF supports this, correct?
>
>
> I am not aware of any kind of debug information splitting except DWARF 
> splitting.


Just for the record, msvc does something very similar to this 
-gsplit-dwarf=single option (using a linker option /DEBUG:FASTLINK instead of 
this SHF_EXCLUDE approach).

Some questions:

- How is this handled by dwp/llvm-dwp? I guess not yet since neither supports 
DWARF5 atm.
- How does it interact with -gdwarf-5? Should it?
- There is some deeply buried information about how gdb requires 
.debug_gnu_pubnames in the case of split dwarf: 
https://reviews.llvm.org/D54497?id=173943#1299461. Is this still true for this 
single split case? I'm asking cause these sections are huge and I wonder if 
they defeat the split dwarf link time gains.


Repository:
  rC Clang

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

https://reviews.llvm.org/D52296



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

[PATCH] D79744: clang: Use byref for kernel arguments

[Matt Arsenault via Phabricator via cfe-commits]

arsenm updated this revision to Diff 279635.
arsenm retitled this revision from "clang: Add address space to indirect abi 
info and use it for kernels" to "clang: Use byref for kernel arguments".
arsenm edited the summary of this revision.
arsenm added a comment.

Switch to byref. Doesn't handle the HIP kernel argument promotion case, since 
that requires more work


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

https://reviews.llvm.org/D79744

Files:
  clang/include/clang/CodeGen/CGFunctionInfo.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/kernel-args.cu
  clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
  clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl

Index: clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl
+++ clang/test/CodeGenOpenCL/amdgpu-abi-struct-coerce.cl
@@ -67,7 +67,6 @@
 int i2;
 } struct_of_structs_arg_t;
 
-// CHECK: %union.transparent_u = type { i32 }
 typedef union
 {
   int b1;
@@ -216,7 +215,7 @@
   int w;
 } struct_4regs;
 
-// CHECK: void @kernel_empty_struct_arg(%struct.empty_struct %s.coerce)
+// CHECK: void @kernel_empty_struct_arg(%struct.empty_struct addrspace(4)* nocapture byref(%struct.empty_struct) align 1 {{%.+}})
 __kernel void kernel_empty_struct_arg(empty_struct s) { }
 
 // CHECK: void @kernel_single_element_struct_arg(i32 %arg1.coerce)
@@ -225,28 +224,28 @@
 // CHECK: void @kernel_nested_single_element_struct_arg(i32 %arg1.coerce)
 __kernel void kernel_nested_single_element_struct_arg(nested_single_element_struct_arg_t arg1) { }
 
-// CHECK: void @kernel_struct_arg(%struct.struct_arg %arg1.coerce)
+// CHECK: void @kernel_struct_arg(%struct.struct_arg addrspace(4)* nocapture byref(%struct.struct_arg) align 4 {{%.+}})
 __kernel void kernel_struct_arg(struct_arg_t arg1) { }
 
-// CHECK: void @kernel_struct_padding_arg(%struct.struct_padding_arg %arg1.coerce)
+// CHECK: void @kernel_struct_padding_arg(%struct.struct_padding_arg addrspace(4)* nocapture byref(%struct.struct_padding_arg) align 8 %{{.+}})
 __kernel void kernel_struct_padding_arg(struct_padding_arg arg1) { }
 
-// CHECK: void @kernel_test_struct_of_arrays_arg(%struct.struct_of_arrays_arg %arg1.coerce)
+// CHECK: void @kernel_test_struct_of_arrays_arg(%struct.struct_of_arrays_arg addrspace(4)* nocapture byref(%struct.struct_of_arrays_arg) align 4 %{{.+}})
 __kernel void kernel_test_struct_of_arrays_arg(struct_of_arrays_arg_t arg1) { }
 
-// CHECK: void @kernel_struct_of_structs_arg(%struct.struct_of_structs_arg %arg1.coerce)
+// CHECK: void @kernel_struct_of_structs_arg(%struct.struct_of_structs_arg addrspace(4)* nocapture byref(%struct.struct_of_structs_arg) align 4 %{{.+}})
 __kernel void kernel_struct_of_structs_arg(struct_of_structs_arg_t arg1) { }
 
-// CHECK: void @test_kernel_transparent_union_arg(%union.transparent_u %u.coerce)
+// CHECK: void @test_kernel_transparent_union_arg(i32 %u.coerce)
 __kernel void test_kernel_transparent_union_arg(transparent_u u) { }
 
-// CHECK: void @kernel_single_array_element_struct_arg(%struct.single_array_element_struct_arg %arg1.coerce)
+// CHECK: void @kernel_single_array_element_struct_arg(%struct.single_array_element_struct_arg addrspace(4)* nocapture byref(%struct.single_array_element_struct_arg) align 4 %{{.+}})
 __kernel void kernel_single_array_element_struct_arg(single_array_element_struct_arg_t arg1) { }
 
-// CHECK: void @kernel_single_struct_element_struct_arg(%struct.single_struct_element_struct_arg %arg1.coerce)
+// CHECK: void @kernel_single_struct_element_struct_arg(%struct.single_struct_element_struct_arg addrspace(4)* nocapture byref(%struct.single_struct_element_struct_arg) align 8 %{{.+}})
 __kernel void kernel_single_struct_element_struct_arg(single_struct_element_struct_arg_t arg1) { }
 
-// CHECK: void @kernel_different_size_type_pair_arg(%struct.different_size_type_pair %arg1.coerce)
+// CHECK: void @kernel_different_size_type_pair_arg(%struct.different_size_type_pair addrspace(4)* nocapture byref(%struct.different_size_type_pair) align 8 %{{.+}})
 __kernel void kernel_different_size_type_pair_arg(different_size_type_pair arg1) { }
 
 // CHECK: define void @func_f32_arg(float %arg)
Index: clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
===
--- clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
+++ clang/test/CodeGenOpenCL/addr-space-struct-arg.cl
@@ -94,10 +94,10 @@
 }
 
 // AMDGCN20-LABEL: define void @test_indirect_arg_globl()
-// AMDGCN20:  %[[byval_temp:.*]] = alloca %struct.LargeStructOneMember, align 8, addrspace(5)
-// AMDGCN20:  %[[r0:.*]] = bitcast %struct.LargeStructOneMember addrspace(5)* %[[byval_temp]] to i8 addrspace(5)*
+// AMDGCN20:  %[[byref_temp:.*]] = alloca %struct.LargeStructOneMember, align 8, addrspace(5)
+// AMDGCN20:  %[[r0:.*]] = bitcast 

[clang] ce04d4e - Fix pow and ldexp in HIP header

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2020-07-21T17:39:46-04:00
New Revision: ce04d4e39c936528fa0e0dc49fdf376eaee21f14

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

LOG: Fix pow and ldexp in HIP header

Added: 


Modified: 
clang/lib/Headers/__clang_hip_libdevice_declares.h
clang/lib/Headers/__clang_hip_math.h

Removed: 




diff  --git a/clang/lib/Headers/__clang_hip_libdevice_declares.h 
b/clang/lib/Headers/__clang_hip_libdevice_declares.h
index e1cd49a39c65..711040443440 100644
--- a/clang/lib/Headers/__clang_hip_libdevice_declares.h
+++ b/clang/lib/Headers/__clang_hip_libdevice_declares.h
@@ -78,6 +78,7 @@ __device__ __attribute__((const)) float 
__ocml_len4_f32(float, float, float,
 __device__ __attribute__((pure)) float __ocml_ncdf_f32(float);
 __device__ __attribute__((pure)) float __ocml_ncdfinv_f32(float);
 __device__ __attribute__((pure)) float __ocml_pow_f32(float, float);
+__device__ __attribute__((pure)) float __ocml_pown_f32(float, int);
 __device__ __attribute__((pure)) float __ocml_rcbrt_f32(float);
 __device__ __attribute__((const)) float __ocml_remainder_f32(float, float);
 __device__ float __ocml_remquo_f32(float, float,
@@ -205,6 +206,7 @@ __device__ __attribute__((const)) double 
__ocml_len4_f64(double, double, double,
 __device__ __attribute__((pure)) double __ocml_ncdf_f64(double);
 __device__ __attribute__((pure)) double __ocml_ncdfinv_f64(double);
 __device__ __attribute__((pure)) double __ocml_pow_f64(double, double);
+__device__ __attribute__((pure)) double __ocml_pown_f64(double, int);
 __device__ __attribute__((pure)) double __ocml_rcbrt_f64(double);
 __device__ __attribute__((const)) double __ocml_remainder_f64(double, double);
 __device__ double __ocml_remquo_f64(double, double,
@@ -290,6 +292,7 @@ __device__ __attribute__((const)) _Float16 
__ocml_rsqrt_f16(_Float16);
 __device__ _Float16 __ocml_sin_f16(_Float16);
 __device__ __attribute__((const)) _Float16 __ocml_sqrt_f16(_Float16);
 __device__ __attribute__((const)) _Float16 __ocml_trunc_f16(_Float16);
+__device__ __attribute__((pure)) _Float16 __ocml_pown_f16(_Float16, int);
 
 typedef _Float16 __2f16 __attribute__((ext_vector_type(2)));
 typedef short __2i16 __attribute__((ext_vector_type(2)));
@@ -320,6 +323,7 @@ __device__ __attribute__((const)) __2f16 
__ocml_rsqrt_2f16(__2f16);
 __device__ __2f16 __ocml_sin_2f16(__2f16);
 __device__ __attribute__((const)) __2f16 __ocml_sqrt_2f16(__2f16);
 __device__ __attribute__((const)) __2f16 __ocml_trunc_2f16(__2f16);
+__device__ __attribute__((const)) __2f16 __ocml_pown_2f16(__2f16, __2i16);
 
 } // extern "C"
 

diff  --git a/clang/lib/Headers/__clang_hip_math.h 
b/clang/lib/Headers/__clang_hip_math.h
index cf7014b9aefe..47d3c1717559 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -294,6 +294,8 @@ normf(int __dim,
 __DEVICE__
 inline float powf(float __x, float __y) { return __ocml_pow_f32(__x, __y); }
 __DEVICE__
+inline float powif(float __x, int __y) { return __ocml_pown_f32(__x, __y); }
+__DEVICE__
 inline float rcbrtf(float __x) { return __ocml_rcbrt_f32(__x); }
 __DEVICE__
 inline float remainderf(float __x, float __y) {
@@ -759,6 +761,8 @@ inline double normcdfinv(double __x) { return 
__ocml_ncdfinv_f64(__x); }
 __DEVICE__
 inline double pow(double __x, double __y) { return __ocml_pow_f64(__x, __y); }
 __DEVICE__
+inline double powi(double __x, int __y) { return __ocml_pown_f64(__x, __y); }
+__DEVICE__
 inline double rcbrt(double __x) { return __ocml_rcbrt_f64(__x); }
 __DEVICE__
 inline double remainder(double __x, double __y) {
@@ -1134,6 +1138,7 @@ __DEF_FUN1(double, trunc);
   __DEVICE__   
\
   inline float __func(float __x, int __y) { return __func##f(__x, __y); }
 __DEF_FLOAT_FUN2I(scalbn)
+__DEF_FLOAT_FUN2I(ldexp)
 
 template  __DEVICE__ inline T min(T __arg1, T __arg2) {
   return (__arg1 < __arg2) ? __arg1 : __arg2;
@@ -1173,6 +1178,17 @@ __host__ inline static int max(int __arg1, int __arg2) {
   return std::max(__arg1, __arg2);
 }
 
+__DEVICE__
+inline float pow(float __base, int __iexp) { return powif(__base, __iexp); }
+
+__DEVICE__
+inline double pow(double __base, int __iexp) { return powi(__base, __iexp); }
+
+__DEVICE__
+inline _Float16 pow(_Float16 __base, int __iexp) {
+  return __ocml_pown_f16(__base, __iexp);
+}
+
 #pragma pop_macro("__DEF_FUN1")
 #pragma pop_macro("__DEF_FUN2")
 #pragma pop_macro("__DEF_FUNI")



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

[PATCH] D83519: [NewPM] Support optnone under new pass manager

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:77
+  StandardInstrumentations(bool DebugLogging)
+  : PrintIR(), TimePasses(), OptNone(DebugLogging) {}
 

(No need to change) PrintIR & TimePasses don't need to appear in the 
initializer list. They would be default initialized.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83519



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

[PATCH] D60620: [HIP] Support target id by --offload-arch

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/Driver/Driver.h:332
 
+  llvm::Triple getHIPOffloadTargetTriple() const;
+

This is used exclusively by the Driver.cpp and does not have to be a public API 
call.



Comment at: clang/lib/Basic/TargetID.cpp:18
+
+static const llvm::SmallVector
+getAllPossibleAMDGPUTargetIDFeatures(const llvm::Triple ,

Nit. You could use llvm::SmallVectorImpl -- caller only cares 
that it's an array of StringRef and does not need to know the size hint.
Makes it easier to change the hint w/o having to replace the constant evrywhere.



Comment at: clang/lib/Basic/TargetID.cpp:53
+
+llvm::StringRef parseTargetID(const llvm::Triple ,
+  llvm::StringRef OffloadArch,

A comment describing expected format would be helpful.




Comment at: clang/lib/Basic/TargetID.cpp:53-69
+llvm::StringRef parseTargetID(const llvm::Triple ,
+  llvm::StringRef OffloadArch,
+  llvm::StringMap *FeatureMap,
+  bool *IsValid) {
+  llvm::StringRef ArchStr;
+  auto SetValid = [&](bool Valid) {
+if (IsValid)

tra wrote:
> A comment describing expected format would be helpful.
> 
I'd restructure things a bit.
First, I'd make return type std::optionaland fold IsValid into it.
Then I would make FeatureMap argument a non-optional, so the parsing can 
concentrate on parsing only.
Then I'd add another overload without FeatureMap argument, which would be a 
warpper over the real parser with a temp FeatureMap which will be discarded.

This should make things easier to read.



Comment at: clang/lib/Basic/TargetID.cpp:103
+
+std::string getCanonicalTargetID(llvm::StringRef Processor,
+ const llvm::StringMap ) {

What does 'canonical' mean? A comment would be helpful.



Comment at: clang/lib/Basic/TargetID.cpp:116
+static llvm::StringRef
+parseCanonicalTargetIDWithoutCheck(llvm::StringRef OffloadArch,
+   llvm::StringMap *FeatureMap) {

Perhaps we can further split parsing offloadID vs checking whether it's valid 
and make parseTargetID above call this parse-only helper.

E.g. something like this:

```
something parseTargetIDhelper(something); // Parses targetID 
something isTargetIdValid(something);  // Verivies validity of parsed parts.
std::optional parseTargetID(FeatureMap) {
   parseTargetIDhelper(...);
   if (!targetIDValid())
  return None;
   return Good;
}
std::optional parseTargetID() {
   auto TempFeatureMap;
  return parseTargetID();
}
```



Comment at: clang/lib/Basic/Targets/AMDGPU.cpp:366
+  std::replace(NewF.begin(), NewF.end(), '-', '_');
+  Builder.defineMacro(Twine("__amdgcn_") + Twine(NewF) + Twine("__"),
+  Loc->second ? "1" : "0");

Nit: Should it be "__amdgcn_feature_" to make it more explicit where these 
macros are derived from?



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:601-605
+llvm::MDString::get(
+getModule().getContext(),
+TargetIDStr == ""
+? TargetIDStr
+: (Twine(getTriple().str()) + "-" + TargetIDStr).str()));

I think this may cause problems.

Twine.str() will return a temporary std::string.
MDString::get takes a StringRef as the second parameter, so it will be a 
reference to the temporary. It will then get added to the module's metadata 
which will probably outlive the temporary string. The tests for the MDString do 
appear to use string storage that outlives MDString.




Comment at: clang/lib/Driver/Driver.cpp:2602-2603
 
+  if (!isValidOffloadArchCombination(GpuArchs))
+return true;
+

This is something we may want to diagnose. 


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

https://reviews.llvm.org/D60620



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

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

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D84192#2165249 , @ABataev wrote:

> In D84192#2165235 , @cchen wrote:
>
> > In D84192#2164885 , @ABataev wrote:
> >
> > > Also, what about compatibility with declare mapper? Can you add tests for 
> > > it?
> >
> >
> > There's a restriction for map clause that non-contiguous is not allowed and 
> > I guess it also applies to declare mapper.
>
>
> I see that to/from clauses allow to use mappers too:
>
>   to([mapper(mapper-identifier):]locator-list) 
> from([mapper(mapper-identifier):]locator-list
>   


Oh, I didn't see that. I'll add test for it, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D84192#2165235 , @cchen wrote:

> In D84192#2164885 , @ABataev wrote:
>
> > Also, what about compatibility with declare mapper? Can you add tests for 
> > it?
>
>
> There's a restriction for map clause that non-contiguous is not allowed and I 
> guess it also applies to declare mapper.


I see that to/from clauses allow to use mappers too:

  to([mapper(mapper-identifier):]locator-list) 
from([mapper(mapper-identifier):]locator-list


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D84087: [NFC] Clean up doc comment and implementation for Module::isSubModuleOf.

[Adrian Prantl via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Module.cpp:176
 bool Module::isSubModuleOf(const Module *Other) const {
-  const Module *This = this;
-  do {
+  for (const Module *This = this; This != nullptr; This = This->Parent) {
 if (This == Other)

```
for (const Module *Parent = this; Parent; Parent = Parent->Parent) {
  if (Parent == Other)
   return true;
}
```
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84087



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

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

[Chi Chun Chen via Phabricator via cfe-commits]

cchen updated this revision to Diff 279631.
cchen added a comment.

Fix based on feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192

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

Index: clang/test/OpenMP/target_update_to_messages.cpp
===
--- clang/test/OpenMP/target_update_to_messages.cpp
+++ clang/test/OpenMP/target_update_to_messages.cpp
@@ -79,6 +79,10 @@
 #pragma omp target update to(*(*(this->ptr)+a+this->ptr)) // le45-error {{expected expression containing only member accesses and/or array sections based on named variables}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
 #pragma omp target update to(*(this+this)) // expected-error {{invalid operands to binary expression ('S8 *' and 'S8 *')}}
 {}
+
+double marr[10][5][10];
+#pragma omp target update to(marr [0:1][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+{}
   }
 };
 
Index: clang/test/OpenMP/target_update_messages.cpp
===
--- clang/test/OpenMP/target_update_messages.cpp
+++ clang/test/OpenMP/target_update_messages.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -verify=expected,omp45 -fopenmp -fopenmp-version=45 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp -fopenmp-version=50 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
-// RUN: %clang_cc1 -verify=expected,omp45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
-// RUN: %clang_cc1 -verify=expected,omp50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 -o - -std=c++11 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -fopenmp-version=45 -ferror-limit 100 %s -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le50 -fopenmp-simd -fopenmp-version=50 -ferror-limit 100 %s -Wuninitialized
 
 void xxx(int argc) {
   int x; // expected-note {{initialize the variable 'x' to silence this warning}}
@@ -39,10 +39,33 @@
 foo();
   }
 
+  double marr[10][5][10];
+#pragma omp target update to(marr[0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr[0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+#pragma omp target update to(marr[0:][1:2:2][1:2]) // le50-error {{array section does not specify length for outermost dimension}} le45-error {{expected ']'}} le45-note {{to match this '['}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(marr[0:][1:2:2][1:2]) // le50-error {{array section does not specify length for outermost dimension}} le45-error {{expected ']'}} le45-note {{to match this '['}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  int arr[4][3][2][1];
+#pragma omp target update to(arr[0:2][2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(arr[0:2][2:4][:2][1]) // le45-error {{array section does not specify contiguous storage}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+
+  double ***dptr;
+#pragma omp target update to(dptr[0:2][2:4][1:2]) // le45-error {{array section does not specify contiguous storage}} le50-error 2 {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}} le45-error {{expected at least one 'to' clause or 'from' clause specified to '#pragma omp target update'}}
+  {}
+#pragma omp target update from(dptr[0:2][2:4][1:2]) 

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

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

In D84192#2164885 , @ABataev wrote:

> Also, what about compatibility with declare mapper? Can you add tests for it?


There's a restriction for map clause that non-contiguous is not allowed and I 
guess it also applies to declare mapper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp:1
-// RUN: %check_clang_tidy -std=c++14,c++17 %s 
modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions
+// RUN: %check_clang_tidy -std=c++14-or-later %s 
modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions 
-DCOMMAND_LINE_INT=int
 // FIXME: Fix the checker to work in C++20 mode, it is performing a

aaron.ballman wrote:
> The change to the language standard line makes me wonder if the fixme below 
> it is now stale, or if the test will fail in C++20 mode.
I just ran the tests again using `python .\check_clang_tidy.py -std=c++20 
.\checkers\modernize-use-trailing-return-type.cpp 
modernize-use-trailing-return-type aa -- -- -DCOMMAND_LINE_INT=int` and I did 
not see mentioning of the use of an uninitialized variable. But I run on 
Windows, maybe the issue just surfaces on another OS? Is there a way to trigger 
the CI again?

I removed the FIXME in question in the hope the issue resolved itself.


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

https://reviews.llvm.org/D80514



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

[PATCH] D83759: [clangd] Fixes in lit tests

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D83759#2165116 , @thakis wrote:

> Looks like this breaks tests on macOS (and probably with non-gnu greps): 
> http://45.33.8.238/mac/17611/step_9.txt
>
> Please take a look, and revert while you investigate if it takes a while to 
> fix.


Reverted.

Seems the problem is in `sed`  without gnu extensions, but I wonder how other 
tests with similar `sed` command could pass (e.g. 
`compile-commands-path-in-initialize.test`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83759



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

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber updated this revision to Diff 279625.
bernhardmgruber marked an inline comment as done.
bernhardmgruber edited the summary of this revision.
bernhardmgruber added a comment.

Removed probably stale FIXME.


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

https://reviews.llvm.org/D80514

Files:
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/modernize-use-trailing-return-type.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type-cxx20.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp
@@ -1,6 +1,4 @@
-// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions
-// FIXME: Fix the checker to work in C++20 mode, it is performing a
-// use-of-uninitialized-value.
+// RUN: %check_clang_tidy -std=c++14-or-later %s modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions -DCOMMAND_LINE_INT=int
 
 namespace std {
 template 
@@ -11,6 +9,8 @@
 
 class string;
 
+class ostream;
+
 template 
 auto declval() -> T;
 }
@@ -215,18 +215,28 @@
 // CHECK-FIXES: {{^}}auto e13() -> struct A;{{$}}
 
 //
-// decltype (unsupported if top level expression)
+// deduced return types
 //
 
+const auto ded1();
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto ded1() -> const auto;{{$}}
+const auto& ded2();
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto ded2() -> const auto&;{{$}}
+
+decltype(auto) ded3();
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto ded3() -> decltype(auto);{{$}}
+
+
 decltype(1 + 2) dec1() { return 1 + 2; }
 // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
-// TODO: source range of DecltypeTypeLoc not yet implemented
-// _HECK-FIXES: {{^}}auto dec1() -> decltype(1 + 2) { return 1 + 2; }{{$}}
+// CHECK-FIXES: {{^}}auto dec1() -> decltype(1 + 2) { return 1 + 2; }{{$}}
 template 
 decltype(std::declval(std::declval)) dec2(F f, T t) { return f(t); }
 // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
-// TODO: source range of DecltypeTypeLoc not yet implemented
-// _HECK-FIXES: {{^}}auto dec2(F f, T t) -> decltype(std::declval(std::declval)) { return f(t); }{{$}}
+// CHECK-FIXES: {{^}}auto dec2(F f, T t) -> decltype(std::declval(std::declval)) { return f(t); }{{$}}
 template 
 typename decltype(std::declval())::value_type dec3();
 // CHECK-MESSAGES: :[[@LINE-1]]:50: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
@@ -463,6 +473,13 @@
 CONST_F_MACRO int& h19();
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
 // CHECK-FIXES: {{^}}auto h19() -> CONST_F_MACRO int&;{{$}}
+// Macro COMMAND_LINE_INT is defined on the command line via: -DCOMMAND_LINE_INT=int
+const COMMAND_LINE_INT& h20();
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto h20() -> const COMMAND_LINE_INT&;{{$}}
+decltype(COMMAND_LINE_INT{}) h21();
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto h21() -> decltype(COMMAND_LINE_INT{});{{$}}
 
 //
 // Name collisions
@@ -531,6 +548,14 @@
 return {0};
 }
 
+//
+// bug 44206, no rewrite should happen due to collision with parameter name
+//
+
+using std::ostream;
+ostream& operator<<(ostream& ostream, int i);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}ostream& operator<<(ostream& ostream, int i);{{$}}
 
 //
 // Samples which do not trigger the check
@@ -544,7 +569,6 @@
 template  auto f(T t) -> int;
 
 auto ff();
-decltype(auto) fff();
 
 void c();
 void c(int arg);
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type-cxx20.cpp
===
--- /dev/null
+++ 

[clang-tools-extra] 23ff4e4 - Revert "[clangd] Fixes in lit tests"

[Aleksandr Platonov via cfe-commits]

Author: Aleksandr Platonov
Date: 2020-07-21T23:32:47+03:00
New Revision: 23ff4e4f5d651b4a75aede51defc8e903dfd694d

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

LOG: Revert "[clangd] Fixes in lit tests"

This reverts commit ff63d6be93dc5958bf35d92919ce6fafcc611e89.

Added: 


Modified: 
clang-tools-extra/clangd/test/background-index.test
clang-tools-extra/clangd/test/did-change-configuration-params.test
clang-tools-extra/clangd/test/test-uri-windows.test

Removed: 




diff  --git a/clang-tools-extra/clangd/test/background-index.test 
b/clang-tools-extra/clangd/test/background-index.test
index c17abb43376f..41184443e947 100644
--- a/clang-tools-extra/clangd/test/background-index.test
+++ b/clang-tools-extra/clangd/test/background-index.test
@@ -1,23 +1,23 @@
+# We need to splice paths into file:// URIs for this test.
+# UNSUPPORTED: windows-msvc
+
 # Use a copy of inputs, as we'll mutate it (as will the background index).
-# RUN: rm -rf %/t
-# RUN: cp -r %/S/Inputs/background-index %/t
+# RUN: rm -rf %t
+# RUN: cp -r %S/Inputs/background-index %t
 # Need to embed the correct temp path in the actual JSON-RPC requests.
-# RUN: sed -i -e "s|DIRECTORY|%/t|" %/t/definition.jsonrpc
-# RUN: sed -i -e "s|DIRECTORY|%/t|" %/t/compile_commands.json
-# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
-# (with the extra slash in the front), so we add it here.
-# RUN: sed -i -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' 
%/t/definition.jsonrpc
+# RUN: sed -i -e "s|DIRECTORY|%t|" %t/definition.jsonrpc
+# RUN: sed -i -e "s|DIRECTORY|%t|" %t/compile_commands.json
 
 # We're editing bar.cpp, which includes foo.h.
 # foo() is declared in foo.h and defined in foo.cpp.
 # The background index should allow us to go-to-definition on foo().
 # We should also see indexing progress notifications.
-# RUN: clangd -background-index -lit-test < %/t/definition.jsonrpc | FileCheck 
%/t/definition.jsonrpc --check-prefixes=CHECK,BUILD
+# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck 
%t/definition.jsonrpc --check-prefixes=CHECK,BUILD
 
 # Test that the index is writing files in the expected location.
-# RUN: ls %/t/.cache/clangd/index/foo.cpp.*.idx
-# RUN: ls %/t/sub_dir/.cache/clangd/index/foo.h.*.idx
+# RUN: ls %t/.cache/clangd/index/foo.cpp.*.idx
+# RUN: ls %t/sub_dir/.cache/clangd/index/foo.h.*.idx
 
 # Test the index is read from disk: delete code and restart clangd.
-# RUN: rm %/t/foo.cpp
-# RUN: clangd -background-index -lit-test < %/t/definition.jsonrpc | FileCheck 
%/t/definition.jsonrpc --check-prefixes=CHECK,USE
+# RUN: rm %t/foo.cpp
+# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck 
%t/definition.jsonrpc --check-prefixes=CHECK,USE

diff  --git 
a/clang-tools-extra/clangd/test/did-change-configuration-params.test 
b/clang-tools-extra/clangd/test/did-change-configuration-params.test
index 19d41d0812e2..4aef1011b370 100644
--- a/clang-tools-extra/clangd/test/did-change-configuration-params.test
+++ b/clang-tools-extra/clangd/test/did-change-configuration-params.test
@@ -1,5 +1,5 @@
 # RUN: clangd -compile_args_from=lsp -lit-test < %s 2> %t | FileCheck 
-strict-whitespace %s
-# RUN: FileCheck --check-prefix=ERR --input-file=%t %s
+# RUN: cat %t | FileCheck --check-prefix=ERR %s
 # UNSUPPORTED: windows-gnu,windows-msvc
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---

diff  --git a/clang-tools-extra/clangd/test/test-uri-windows.test 
b/clang-tools-extra/clangd/test/test-uri-windows.test
index 3f03b2985a70..381c48fafc03 100644
--- a/clang-tools-extra/clangd/test/test-uri-windows.test
+++ b/clang-tools-extra/clangd/test/test-uri-windows.test
@@ -1,5 +1,5 @@
 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-# UNSUPPORTED: !(windows-gnu || windows-msvc)
+# REQUIRES: windows-gnu || windows-msvc
 # Test authority-less URI
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---



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

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber added a comment.

Great, I seems I forgot to submit the inline comments. Sorry about that!




Comment at: 
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:293
+StringRef File = SM.getBufferData(Loc.first);
+const char *TokenBegin = File.data() + Loc.second;
+Lexer Lexer(SM.getLocForStartOfFile(Loc.first), LangOpts, File.begin(),

aaron.ballman wrote:
> Should we verify that `End` is valid before doing this pointer arithmetic 
> with a value derived from it? For instance, what if `End` points into the 
> scratch buffer because it relies on a macro defined on the command line -- 
> will the logic still work?
That sounded like a scary scenario! Fortunately, `expandIfMacroId` even expands 
source locations inside the scratch buffer correctly into a location in a real 
file. I added two tests for it and they seem to pass. Thank you for the hint!



Comment at: 
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:430
+  AT->getKeyword() == AutoTypeKeyword::Auto &&
+  !hasAnyNestedLocalQualifiers(F->getDeclaredReturnType()))
+return;

aaron.ballman wrote:
> Why do we need to check that there aren't any nested local qualifiers?
Because I would like the check to rewrite e.g. `const auto f();` into `auto f() 
-> const auto;`. If I omit the check for nested local qualifiers, then those 
kind of declarations would be skipped.



Comment at: 
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:285-302
+SourceLocation End =
+expandIfMacroId(ReturnLoc.getSourceRange().getEnd(), SM);
+SourceLocation BeginNameF = expandIfMacroId(F.getLocation(), SM);
+
+// Extend the ReturnTypeRange until the last token before the function
+// name.
+std::pair Loc = SM.getDecomposedLoc(End);

aaron.ballman wrote:
> bernhardmgruber wrote:
> > Is there an easier way to get the token previous to the function name?
> There isn't, but if you find you're missing source location information for 
> something, you can also consider modifying Clang to add that source location 
> information and mark this as a dependent patch. It's not clear to me whether 
> that would be worth the effort for this patch or not, but my preference is to 
> avoid these little hacks whenever possible.
Thank you for the hint! I considered this for the concept location in an 
`AutoReturnType` and the expression inside a `decltype` `AutoReturnType`. 
Unfortunately I could not wrap my head around what is going on in SemaType.cpp 
:/


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

https://reviews.llvm.org/D80514



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

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Thanks.  It probably won't be soon.  At the very least, I need to find the 
answer to question 3 from the review summary.


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

https://reviews.llvm.org/D83061



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

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D83061#2165100 , @jdenny wrote:

> In D83061#2165093 , @ABataev wrote:
>
> > In D83061#2165089 , @jdenny wrote:
> >
> > > In D83061#2165063 , @ABataev 
> > > wrote:
> > >
> > > > LG.
> > >
> > >
> > > Thanks for the review.
> > >
> > > As discussed in the review summary, please consider the following.  A 
> > > present map type modifier behavior that this patch does not attempt to 
> > > implement is TR8 sec. 2.22.7.1 "map Clause", p. 319, L14-16:
> > >
> > > > If a map clause with a present map-type-modifier is present in a map
> > > >  clause, then the effect of the clause is ordered before all other
> > > >  map clauses that do not have the present modifier.
> > >
> > > Compare to L10-11:
> > >
> > > > For a given construct, the effect of a map clause with the to, from,
> > > >  or tofrom map-type is ordered before the effect of a map clause with
> > > >  the alloc, release, or delete map-type.
> > >
> > > As far as I can tell, Clang does not implement L10-11. Is that correct?  
> > > If not, then I think both passages should be implemented together later.  
> > > Any objections?
> >
> >
> > Looks like you're right. Yes, go ahead and implement it.
>
>
> Are you ok for it to be a later patch after pushing these?


Sure


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

https://reviews.llvm.org/D83061



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

[PATCH] D83759: [clangd] Fixes in lit tests

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this breaks tests on macOS (and probably with non-gnu greps): 
http://45.33.8.238/mac/17611/step_9.txt

Please take a look, and revert while you investigate if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83759



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

[clang] 73bc23f - Fix the data layout mangling specification for 'i686-pc-macho'

[Akira Hatanaka via cfe-commits]

Author: Akira Hatanaka
Date: 2020-07-21T12:58:17-07:00
New Revision: 73bc23ff86655b4e041ce14a991a8bd5cce985e0

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

LOG: Fix the data layout mangling specification for 'i686-pc-macho'

Use 'o' for the mangling specification instead of 'e'. This fixes an
error in the backend caused by a mismatch between the data layouts
generated by the backend and the frontend.

rdar://problem/64168540

Added: 


Modified: 
clang/lib/Basic/Targets/X86.h
clang/test/CodeGen/target-data.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index 72a01d2514c2..50e77937febd 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -358,7 +358,10 @@ class LLVM_LIBRARY_VISIBILITY X86_32TargetInfo : public 
X86TargetInfo {
 LongDoubleWidth = 96;
 LongDoubleAlign = 32;
 SuitableAlign = 128;
-resetDataLayout("e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-"
+resetDataLayout(Triple.isOSBinFormatMachO() ?
+"e-m:o-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-"
+"f80:32-n8:16:32-S128" :
+"e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-"
 "f80:32-n8:16:32-S128");
 SizeType = UnsignedInt;
 PtrDiffType = SignedInt;

diff  --git a/clang/test/CodeGen/target-data.c 
b/clang/test/CodeGen/target-data.c
index 8c740119cd1b..4f2dbe7c8e10 100644
--- a/clang/test/CodeGen/target-data.c
+++ b/clang/test/CodeGen/target-data.c
@@ -14,6 +14,10 @@
 // RUN: FileCheck --check-prefix=I686-CYGWIN %s
 // I686-CYGWIN: target datalayout = 
"e-m:x-p:32:32-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:32-n8:16:32-a:0:32-S32"
 
+// RUN: %clang_cc1 -triple i686-pc-macho -emit-llvm -o - %s | \
+// RUN: FileCheck --check-prefix=I686-MACHO %s
+// I686-MACHO: target datalayout = 
"e-m:o-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-f80:32-n8:16:32-S128"
+
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | \
 // RUN: FileCheck --check-prefix=X86_64 %s
 // X86_64: target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"



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

[PATCH] D83551: [PATCH 2/4][Sema][AArch64] Add semantics for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added a comment.

In D83551#2164984 , @aaron.ballman 
wrote:

> The attribute bits LGTM, thanks!


Thanks for reviewing!


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

https://reviews.llvm.org/D83551



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 2 inline comments as done.
erichkeane added a comment.

I'd attempted to regenerate the cxx_dr_status.html using make_cxx_dr_status, 
but I get a 'no such

In D84048#2165080 , @rsmith wrote:

> In D84048#2164950 , @erichkeane 
> wrote:
>
> > Additionally, I sent out a CWG message on the reflector about this: 
> > https://godbolt.org/z/bEr61T
> >
> > My implementation has this as ambiguous, but the wording makes it 
> > well-formed I think.  I don't think that was the intent of CWG2303, so 
> > unless they confirm that this was their intent (or that I'm wrong), I'll 
> > leave this patch as is.
>
>
> Let's wait to hear back. Either option seems plausible here. The wording as 
> written behaves the same way as name shadowing in member name lookup, so it 
> may be intentional.


Agreed.  I think the pruning mechanism as implemented here is superior for a 
few reasons (and is easier to reason about mentally), but I want to see what 
CWG has to say.

FWIW, I though of the problem with this patch AFTER the fact (or at least, 
completely understood the wording at that point), so I wasn't aware of it when 
I wrote this patch.




Comment at: clang/test/CXX/drs/dr23xx.cpp:118
+#if __cplusplus >= 201103L
+namespace dr2303 {
+template 

rsmith wrote:
> This should include a comment that `make_cxx_dr_status` can parse, such as 
> `// dr2303: 11` to indicate support in Clang 11 onwards.
Our current clang-version is 12.0.0, so 12 is correct here, right?

I've not been able to get make_cxx_dr_status work unfortunately. It seems to 
generate a blank version of the file (well, it HAS html, but none of the 
content).

I used the cwg_index.html from here: 
http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_index.html




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

https://reviews.llvm.org/D84048



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

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D83061#2165093 , @ABataev wrote:

> In D83061#2165089 , @jdenny wrote:
>
> > In D83061#2165063 , @ABataev wrote:
> >
> > > LG.
> >
> >
> > Thanks for the review.
> >
> > As discussed in the review summary, please consider the following.  A 
> > present map type modifier behavior that this patch does not attempt to 
> > implement is TR8 sec. 2.22.7.1 "map Clause", p. 319, L14-16:
> >
> > > If a map clause with a present map-type-modifier is present in a map
> > >  clause, then the effect of the clause is ordered before all other
> > >  map clauses that do not have the present modifier.
> >
> > Compare to L10-11:
> >
> > > For a given construct, the effect of a map clause with the to, from,
> > >  or tofrom map-type is ordered before the effect of a map clause with
> > >  the alloc, release, or delete map-type.
> >
> > As far as I can tell, Clang does not implement L10-11. Is that correct?  If 
> > not, then I think both passages should be implemented together later.  Any 
> > objections?
>
>
> Looks like you're right. Yes, go ahead and implement it.


Are you ok for it to be a later patch after pushing these?


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

https://reviews.llvm.org/D83061



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

[clang] 13bfe4b - [OPENMP]Fix PR46012: declare target pointer cannot be accessed in target region.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-07-21T15:48:32-04:00
New Revision: 13bfe4b226d2f158c46b9e351f4f0c224b899b96

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

LOG: [OPENMP]Fix PR46012: declare target pointer cannot be accessed in target 
region.

Summary:
Need to avoid an optimization for base pointer mapping for target data
directives.

Reviewers: jdoerfert, ye-luo

Subscribers: yaxunl, guansong, cfe-commits, sstefan1, caomhin

Tags: #clang

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/target_data_codegen.cpp
clang/test/OpenMP/target_data_use_device_ptr_codegen.cpp
clang/test/OpenMP/target_update_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index f6d36bd84385..079f57b1799f 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7379,6 +7379,9 @@ class MappableExprsHandler {
 //
 // map(p[1:24])
 // p, [1], 24*sizeof(float), TARGET_PARAM | TO | FROM
+// for data directives
+// p, p, sizeof(float*), TARGET_PARAM | TO | FROM
+// p, [1], 24*sizeof(float), PTR_AND_OBJ | TO | FROM
 //
 // map(s)
 // , , sizeof(S2), TARGET_PARAM | TO | FROM
@@ -7557,9 +7560,15 @@ class MappableExprsHandler {
   if (Ty->isAnyPointerType() && std::next(I) != CE) {
 BP = CGF.EmitLoadOfPointer(BP, Ty->castAs());
 
-// We do not need to generate individual map information for the
-// pointer, it can be associated with the combined storage.
-++I;
+// For non-data directives, we do not need to generate individual map
+// information for the pointer, it can be associated with the combined
+// storage.
+if (CGF.CGM.getOpenMPRuntime().hasRequiresUnifiedSharedMemory() ||
+!CurDir.is() ||
+!isOpenMPTargetDataManagementDirective(
+CurDir.get()
+->getDirectiveKind()))
+  ++I;
   }
 }
 
@@ -7633,6 +7642,7 @@ class MappableExprsHandler {
 isa(Next->getAssociatedExpression()) ||
 isa(Next->getAssociatedExpression()) ||
 isa(Next->getAssociatedExpression()) ||
+isa(Next->getAssociatedExpression()) ||
 isa(Next->getAssociatedExpression()) ||
 isa(Next->getAssociatedExpression())) &&
"Unexpected expression");

diff  --git a/clang/test/OpenMP/target_data_codegen.cpp 
b/clang/test/OpenMP/target_data_codegen.cpp
index 274b3e16b2f6..bf5874702ace 100644
--- a/clang/test/OpenMP/target_data_codegen.cpp
+++ b/clang/test/OpenMP/target_data_codegen.cpp
@@ -462,7 +462,7 @@ struct S2 {
 
 void test_close_modifier(int arg) {
   S2 *ps;
-  // CK5: private unnamed_addr constant [5 x i64] [i64 1059, i64 32, {{.*}}, 
i64 16, i64 1043]
+  // CK5: private unnamed_addr constant [6 x i64] [i64 1059, i64 32, i64 
562949953422339, i64 562949953421328, i64 16, i64 1043]
   #pragma omp target data map(close,tofrom: arg, ps->ps->ps->ps->s)
   {
 ++(arg);

diff  --git a/clang/test/OpenMP/target_data_use_device_ptr_codegen.cpp 
b/clang/test/OpenMP/target_data_use_device_ptr_codegen.cpp
index a3d8043b6b4e..fe6ea01b43c9 100644
--- a/clang/test/OpenMP/target_data_use_device_ptr_codegen.cpp
+++ b/clang/test/OpenMP/target_data_use_device_ptr_codegen.cpp
@@ -22,18 +22,18 @@
 double *g;
 
 // CK1: @g = global double*
-// CK1: [[MTYPE00:@.+]] = {{.*}}constant [1 x i64] [i64 99]
-// CK1: [[MTYPE01:@.+]] = {{.*}}constant [1 x i64] [i64 99]
-// CK1: [[MTYPE03:@.+]] = {{.*}}constant [1 x i64] [i64 99]
-// CK1: [[MTYPE04:@.+]] = {{.*}}constant [1 x i64] [i64 99]
-// CK1: [[MTYPE05:@.+]] = {{.*}}constant [1 x i64] [i64 99]
-// CK1: [[MTYPE06:@.+]] = {{.*}}constant [1 x i64] [i64 99]
-// CK1: [[MTYPE07:@.+]] = {{.*}}constant [1 x i64] [i64 99]
-// CK1: [[MTYPE08:@.+]] = {{.*}}constant [2 x i64] [{{i64 35, i64 99|i64 99, 
i64 35}}]
-// CK1: [[MTYPE09:@.+]] = {{.*}}constant [2 x i64] [i64 99, i64 99]
-// CK1: [[MTYPE10:@.+]] = {{.*}}constant [2 x i64] [i64 99, i64 99]
-// CK1: [[MTYPE11:@.+]] = {{.*}}constant [2 x i64] [i64 96, i64 35]
-// CK1: [[MTYPE12:@.+]] = {{.*}}constant [2 x i64] [i64 96, i64 35]
+// CK1: [[MTYPE00:@.+]] = {{.*}}constant [2 x i64] [i64 99, i64 19]
+// CK1: [[MTYPE01:@.+]] = {{.*}}constant [2 x i64] [i64 99, i64 19]
+// CK1: [[MTYPE03:@.+]] = {{.*}}constant [2 x i64] [i64 99, i64 19]
+// CK1: [[MTYPE04:@.+]] = {{.*}}constant [2 x i64] [i64 99, i64 19]
+// CK1: [[MTYPE05:@.+]] = {{.*}}constant [2 x i64] [i64 99, i64 19]
+// CK1: [[MTYPE06:@.+]] = {{.*}}constant [2 x i64] [i64 99, i64 19]
+// CK1: [[MTYPE07:@.+]] = {{.*}}constant [2 x 

[PATCH] D84182: [OPENMP]Fix PR46012: declare target pointer cannot be accessed in target region.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG13bfe4b226d2: [OPENMP]Fix PR46012: declare target pointer 
cannot be accessed in target region. (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84182

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_data_codegen.cpp
  clang/test/OpenMP/target_data_use_device_ptr_codegen.cpp
  clang/test/OpenMP/target_update_codegen.cpp

Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -310,23 +310,22 @@
 
 #ifdef CK5
 
-// CK5: [[SIZE00:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
-// CK5: [[MTYPE00:@.+]] = {{.+}}constant [1 x i64] [i64 33]
+// CK5: [[SIZE00:@.+]] = {{.+}}constant [2 x i[[sz:64|32]]] [i{{64|32}} {{8|4}}, i{{64|32}} 4]
+// CK5: [[MTYPE00:@.+]] = {{.+}}constant [2 x i64] [i64 33, i64 17]
 
 // CK5-LABEL: lvalue
 void lvalue(int *B, int l, int e) {
 
-  // CK5-DAG: call void @__tgt_target_data_update_mapper(i64 -1, i32 1, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[SIZE00]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[MTYPE00]]{{.+}}, i8** null)
+  // CK5-DAG: call void @__tgt_target_data_update_mapper(i64 -1, i32 2, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[2 x i{{.+}}]* [[SIZE00]], {{.+}}getelementptr {{.+}}[2 x i{{.+}}]* [[MTYPE00]]{{.+}}, i8** null)
   // CK5-DAG: [[GEPBP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
   // CK5-DAG: [[GEPP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
 
-  // CK5-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
-  // CK5-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
-  // CK5-DAG: [[BPC0:%.+]] = bitcast i8** [[BP0]] to i32**
+  // CK5-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 1
+  // CK5-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 1
+  // CK5-DAG: [[BPC0:%.+]] = bitcast i8** [[BP0]] to i32***
   // CK5-DAG: [[PC0:%.+]] = bitcast i8** [[P0]] to i32**
-  // CK5-DAG: store i32* [[B_VAL:%.+]], i32** [[BPC0]]
+  // CK5-DAG: store i32** [[B_ADDR:%.+]], i32*** [[BPC0]]
   // CK5-DAG: store i32* [[B_VAL_2:%.+]], i32** [[PC0]]
-  // CK5-DAG: [[B_VAL]] = load i32*, i32** [[B_ADDR:%.+]]
   // CK5-DAG: [[B_VAL_2]] = load i32*, i32** [[B_ADDR]]
   #pragma omp target update to(*B)
   *B += e;
@@ -352,27 +351,28 @@
 
 #ifdef CK6
 
-// CK6: [[SIZE00:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
-// CK6: [[MTYPE00:@.+]] = {{.+}}constant [1 x i64] [i64 33]
+// CK6: [[SIZE00:@.+]] = {{.+}}constant [2 x i[[sz:64|32]]] [i{{64|32}} {{8|4}}, i{{64|32}} 4]
+// CK6: [[MTYPE00:@.+]] = {{.+}}constant [2 x i64] [i64 33, i64 17]
 
 // CK6-LABEL: lvalue
 void lvalue(int *B, int l, int e) {
 
-  // CK6-DAG: call void @__tgt_target_data_update_mapper(i64 -1, i32 1, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[SIZE00]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[MTYPE00]]{{.+}}, i8** null)
+  // CK6-DAG: call void @__tgt_target_data_update_mapper(i64 -1, i32 2, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[2 x i{{.+}}]* [[SIZE00]], {{.+}}getelementptr {{.+}}[2 x i{{.+}}]* [[MTYPE00]]{{.+}}, i8** null)
   // CK6-DAG: [[GEPBP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
   // CK6-DAG: [[GEPP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
 
-  // CK6-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
-  // CK6-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
-  // CK6-DAG: [[BPC0:%.+]] = bitcast i8** [[BP0]] to i32**
+  // CK6-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 1
+  // CK6-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 1
+  // CK6-DAG: [[BPC0:%.+]] = bitcast i8** [[BP0]] to i32***
   // CK6-DAG: [[PC0:%.+]] = bitcast i8** [[P0]] to i32**
-  // CK6-DAG: store i32* [[ZERO:%.+]], i32** [[BPC0]]
+  // CK6-DAG: store i32** [[B_ADDR:%.+]], i32*** [[BPC0]]
   // CK6-DAG: store i32* [[ADD_PTR:%.+]], i32** [[PC0]]
   // CK6-64-DAG: [[ADD_PTR]] = getelementptr inbounds i32, i32* [[ONE:%.+]], i{{32|64}} [[IDX_EXT:%.+]]
   // CK6-32-DAG: [[ADD_PTR]] = getelementptr inbounds i32, i32* [[ONE:%.+]], i{{32|64}} [[L_VAL:%.+]]
   // CK6-64-DAG: [[IDX_EXT]] = sext i32 [[L_VAL:%.+]] to i64
   // CK6-DAG: [[L_VAL]] = load i32, i32* [[L_ADDR:%.+]]
   // CK6-DAG: store i32 {{.+}}, i32* [[L_ADDR]]
+  // CK6-DAG: [[ONE]] = load i32*, i32** [[B_ADDR]]
   #pragma omp target update to(*(B+l))
   *(B+l) += e;
   #pragma omp target update from(*(B+l))
@@ -397,23 +397,22 @@
 
 #ifdef CK7
 
-// CK7: [[SIZE00:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
-// CK7: 

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D83061#2165089 , @jdenny wrote:

> In D83061#2165063 , @ABataev wrote:
>
> > LG.
>
>
> Thanks for the review.
>
> As discussed in the review summary, please consider the following.  A present 
> map type modifier behavior that this patch does not attempt to implement is 
> TR8 sec. 2.22.7.1 "map Clause", p. 319, L14-16:
>
> > If a map clause with a present map-type-modifier is present in a map
> >  clause, then the effect of the clause is ordered before all other
> >  map clauses that do not have the present modifier.
>
> Compare to L10-11:
>
> > For a given construct, the effect of a map clause with the to, from,
> >  or tofrom map-type is ordered before the effect of a map clause with
> >  the alloc, release, or delete map-type.
>
> As far as I can tell, Clang does not implement L10-11. Is that correct?  If 
> not, then I think both passages should be implemented together later.  Any 
> objections?


Looks like you're right. Yes, go ahead and implement it.

> 
> 
>> Please, land it after the runtime part
> 
> I'll push them at the same time.  The runtime patch needs to be second 
> because its test suite depends on this patch.  But there are still some 
> details to resolve in the runtime patch as well.




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

https://reviews.llvm.org/D83061



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

[PATCH] D81003: [clang] SequenceChecker: Also visit default arguments and default initializers.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Friendly ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81003



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

[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/CGColorSpace.c:8-11
 void f() {
-  CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB(); // 
expected-warning{{leak}}
+  CGColorSpaceRef X = CGColorSpaceCreateDeviceRGB();
   CGColorSpaceRetain(X);
+} // expected-warning{{leak}}

This change doesn't look expected to me. It's not "we've found a report on a 
different path" (there's only one path here), it's "we're reporting the same 
report in a different location" (previously it was the uniqueing location, now 
it's the end-of-path location).

That said, RetainCountChecker is special because it has its own 
`PathSensitiveBugReport` sub-class with custom behavior. I think we should get 
regular checkers like MallocChecker right first. Are there more changes on 
tests on regular checkers? Or is the one in `malloc-plist.c` the only one?



Comment at: clang/test/Analysis/malloc-plist.c:137-139
 if (y)
-y++;
-}//expected-warning{{Potential leak}}
+  y++; //expected-warning{{Potential leak}}
+}

This sounds like an expected change: we're now displaying the same report on a 
different path. Except it's the longer path rather than the shorter path, so it 
still looks suspicious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83961



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

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D83061#2165063 , @ABataev wrote:

> LG.


Thanks for the review.

As discussed in the review summary, please consider the following.  A present 
map type modifier behavior that this patch does not attempt to implement is TR8 
sec. 2.22.7.1 "map Clause", p. 319, L14-16:

>   If a map clause with a present map-type-modifier is present in a map
>   clause, then the effect of the clause is ordered before all other
>   map clauses that do not have the present modifier.

Compare to L10-11:

> For a given construct, the effect of a map clause with the to, from,
>  or tofrom map-type is ordered before the effect of a map clause with
>  the alloc, release, or delete map-type.

As far as I can tell, Clang does not implement L10-11. Is that correct?  If 
not, then I think both passages should be implemented together later.  Any 
objections?

> Please, land it after the runtime part

I'll push them at the same time.  The runtime patch needs to be second because 
its test suite depends on this patch.  But there are still some details to 
resolve in the runtime patch as well.


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

https://reviews.llvm.org/D83061



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

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

rsmith added a comment.

In D84048#2164950 , @erichkeane wrote:

> Additionally, I sent out a CWG message on the reflector about this: 
> https://godbolt.org/z/bEr61T
>
> My implementation has this as ambiguous, but the wording makes it well-formed 
> I think.  I don't think that was the intent of CWG2303, so unless they 
> confirm that this was their intent (or that I'm wrong), I'll leave this patch 
> as is.


Let's wait to hear back. Either option seems plausible here. The wording as 
written behaves the same way as name shadowing in member name lookup, so it may 
be intentional.




Comment at: clang/test/CXX/drs/dr23xx.cpp:118
+#if __cplusplus >= 201103L
+namespace dr2303 {
+template 

This should include a comment that `make_cxx_dr_status` can parse, such as `// 
dr2303: 11` to indicate support in Clang 11 onwards.



Comment at: clang/www/cxx_dr_status.html:13636
 Partial ordering and recursive variadic inheritance
-Unknown
+Yes
   

This is a generated file; use `make_cxx_dr_status` in the same directory to 
regenerate it.


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

https://reviews.llvm.org/D84048



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

[PATCH] D84260: [OpenMP][AMDGCN] Support OpenMP offloading for AMDGCN architecture - Part 3

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam updated this revision to Diff 279610.
saiislam added a comment.

Added final to specialized classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84260

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/OpenMP/amdgcn_target_codegen.cpp

Index: clang/test/OpenMP/amdgcn_target_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/amdgcn_target_codegen.cpp
@@ -0,0 +1,45 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple amdgcn-amd-amdhsa -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+#define N 1000
+
+int test_amdgcn_target_tid_threads() {
+  // CHECK-LABEL: test_amdgcn_target_tid_threads
+  // CHECK-LABEL: entry:
+
+  int arr[N];
+
+// CHECK: %nvptx_tid = call i32 @llvm.amdgcn.workitem.id.x()
+// CHECK-DAG: %nvptx_num_threads = call i64 @__ockl_get_local_size(i32 0)
+// CHECK-DAG: [[VAR1:%[0-9]+]] = trunc i64 %nvptx_num_threads to i32
+// CHECK-DAG: %thread_limit = sub nuw i32 [[VAR1]], 64
+#pragma omp target
+  for (int i = 0; i < N; i++) {
+arr[i] = 1;
+  }
+
+  return arr[0];
+}
+
+int test_amdgcn_target_tid_threads_simd() {
+  // CHECK-LABEL: test_amdgcn_target_tid_threads_simd
+  // CHECK-LABEL: entry:
+
+  int arr[N];
+
+// CHECK: %nvptx_num_threads = call i64 @__ockl_get_local_size(i32 0)
+// CHECK-DAG: [[VAR2:%[0-9]+]] = trunc i64 %nvptx_num_threads to i32
+// CHECK-DAG: call void @__kmpc_spmd_kernel_init(i32 [[VAR2]], i16 0, i16 0)
+#pragma omp target simd
+  for (int i = 0; i < N; i++) {
+arr[i] = 1;
+  }
+  return arr[0];
+}
+
+#endif
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -19,6 +19,7 @@
 #include "CGObjCRuntime.h"
 #include "CGOpenCLRuntime.h"
 #include "CGOpenMPRuntime.h"
+#include "CGOpenMPRuntimeAMDGCN.h"
 #include "CGOpenMPRuntimeNVPTX.h"
 #include "CodeGenFunction.h"
 #include "CodeGenPGO.h"
@@ -215,6 +216,11 @@
"OpenMP NVPTX is only prepared to deal with device code.");
 OpenMPRuntime.reset(new CGOpenMPRuntimeNVPTX(*this));
 break;
+  case llvm::Triple::amdgcn:
+assert(getLangOpts().OpenMPIsDevice &&
+   "OpenMP AMDGCN is only prepared to deal with device code.");
+OpenMPRuntime.reset(new CGOpenMPRuntimeAMDGCN(*this));
+break;
   default:
 if (LangOpts.OpenMPSimd)
   OpenMPRuntime.reset(new CGOpenMPSIMDRuntime(*this));
Index: clang/lib/CodeGen/CMakeLists.txt
===
--- clang/lib/CodeGen/CMakeLists.txt
+++ clang/lib/CodeGen/CMakeLists.txt
@@ -62,6 +62,7 @@
   CGObjCRuntime.cpp
   CGOpenCLRuntime.cpp
   CGOpenMPRuntime.cpp
+  CGOpenMPRuntimeAMDGCN.cpp
   CGOpenMPRuntimeGPU.cpp
   CGOpenMPRuntimeNVPTX.cpp
   CGRecordLayoutBuilder.cpp
Index: clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -39,3 +39,21 @@
   (), llvm::Intrinsic::nvvm_read_ptx_sreg_warpsize),
   "nvptx_warp_size");
 }
+
+/// Get the id of the current thread on the GPU.
+llvm::Value *CGOpenMPRuntimeNVPTX::getGPUThreadID(CodeGenFunction ) {
+  CGBuilderTy  = CGF.Builder;
+  llvm::Function *F;
+  F = llvm::Intrinsic::getDeclaration(
+  (), llvm::Intrinsic::nvvm_read_ptx_sreg_tid_x);
+  return Bld.CreateCall(F, llvm::None, "nvptx_tid");
+}
+
+/// Get the maximum number of threads in a block of the GPU.
+llvm::Value *CGOpenMPRuntimeNVPTX::getGPUNumThreads(CodeGenFunction ) {
+  CGBuilderTy  = CGF.Builder;
+  llvm::Function *F;
+  F = llvm::Intrinsic::getDeclaration(
+  (), llvm::Intrinsic::nvvm_read_ptx_sreg_ntid_x);
+  return Bld.CreateCall(F, llvm::None, "nvptx_num_threads");
+}
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
@@ -7,7 +7,7 @@
 //===--===//
 //
 // This provides a generalized class for OpenMP runtime code generation
-// specialized by GPU target NVPTX.
+// specialized 

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Alexey Bataev via Phabricator via cfe-commits]

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

LG. Please, land it after the runtime part


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

https://reviews.llvm.org/D83061



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

[PATCH] D83652: Merge some of the PCH object support with modular codegen

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

@hans @llunak  - sounds like you're both fine with this? Either of you mind to 
give it a formal approval, if that's the case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83652



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

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:3067-3068
+  unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok));
+  if (Type < OMPC_MAP_MODIFIER_unknown)
+return OMPC_MAP_MODIFIER_unknown;
+  return static_cast(Type);

ABataev wrote:
> jdenny wrote:
> > ABataev wrote:
> > > jdenny wrote:
> > > > ABataev wrote:
> > > > > jdenny wrote:
> > > > > > ABataev wrote:
> > > > > > > Why do we need this?
> > > > > > When called with `OMPC_map`, `getOpenMPSimpleClauseType` can return 
> > > > > > either an `OpenMPMapModifierKind` or `OpenMPMapClauseKind` 
> > > > > > depending on `Tok`.  Thus, without this change, both 
> > > > > > `isMapModifier` and `isMapType` can return either of those despite 
> > > > > > the difference in their names and documentation.
> > > > > > 
> > > > > > I found that, when `Parser::parseMapTypeModifiers` ignores 
> > > > > > `present` as if it's not a modifier because OpenMP < 5.1, then 
> > > > > > `isMapType` later returns `OMPC_MAP_MODIFIER_present` and causes 
> > > > > > the following assert to fail in `Sema::ActOnOpenMPVarListClause`:
> > > > > > 
> > > > > > ```
> > > > > > assert(0 <= ExtraModifier && ExtraModifier <= OMPC_MAP_unknown &&
> > > > > >"Unexpected map modifier.");
> > > > > > ```
> > > > > > 
> > > > > > To me, the most obvious solution is to fix `isMapType` and 
> > > > > > `isMapModifier`.  Fortunately, looking in `OpenMPKinds.h`, the 
> > > > > > enumerator values in `OpenMPMapModifierKind` and 
> > > > > > `OpenMPMapClauseKind` are disjoint so we can tell which we have.
> > > > > Can we have something like:
> > > > > ```
> > > > > if (LangOpts.OpenMP <= 50 && Type == OMPC_MAP_MODIFIER_present)
> > > > >   return OMPC_MAP_MODIFIER_unknown;
> > > > > ```
> > > > > or extend `getOpenMPSimpleClauseType` function with the version 
> > > > > parameter and check there is modifier is allowed and return `unknown` 
> > > > > if it is not compatible with provided version?
> > > > As far as I can tell, my changes general fix this bug in `isMapType` 
> > > > and `isMapModifier`.  It makes them behave as documented.  The 
> > > > suggestions you're making only fix them for the case of `present`.  Why 
> > > > is that better?
> > > It is too general. I think it may mask some issues in future. That's why 
> > > I would suggest to tweak it for `present` only. Or, even better, extend 
> > > `getOpenMPSimpleClauseType` function to handle this modifiers more 
> > > correctly for each particular version rather than fix it here.
> > > Or, even better, extend getOpenMPSimpleClauseType function to handle this 
> > > modifiers more correctly for each particular version rather than fix it 
> > > here.
> > 
> > One way to implement what I think you mean is to add an additional 
> > parameter to `getOpenMPSimpleClauseType` to request either the clause's 
> > associated type or that type's modifiers.  Unless I missed a clause, this 
> > parameter would affect only map, defaultmap, and schedule clauses.  For 
> > other clauses, this parameter would be ignored.  Is that what you have in 
> > mind?
> I would also add a check for version compatibility if the modifier is 
> supported for the given version. But anyway, I would like to take a look at 
> what you have in mind
I think I misread your previous comment.  I believe I've implemented your 
suggestion now.


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

https://reviews.llvm.org/D83061



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 279609.
erichkeane added a comment.

Revert change to cxx_dr_status and update the test to have the right tag.


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

https://reviews.llvm.org/D84048

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/drs/dr23xx.cpp


Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -113,3 +113,26 @@
   extern template const int d;
 #endif
 }
+
+#if __cplusplus >= 201103L
+namespace dr2303 { // dr2303: 12
+template 
+struct A;
+template <>
+struct A<> {};
+template 
+struct A : A {};
+struct B : A {};
+
+template 
+void f(const A &);
+template 
+void f2(const A *);
+
+void g() {
+  f(B{}); // This is no longer ambiguous.
+  B b;
+  f2();
+}
+} //namespace dr2303
+#endif
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -1792,7 +1792,10 @@
   //   transformed A can be a derived class of the deduced A. Likewise if
   //   P is a pointer to a class of the form simple-template-id, the
   //   transformed A can be a pointer to a derived class pointed to by the
-  //   deduced A.
+  //   deduced A. However, if there is a class C that is a (direct or
+  //   indirect) base class of D and derived (directly or indirectly) from 
a
+  //   class B and that would be a valid deduced A, the deduced A cannot be
+  //   B or pointer to B, respectively.
   //
   //   These alternatives are considered only if type deduction would
   //   otherwise fail. If they yield more than one possible deduced A, the
@@ -1812,6 +1815,7 @@
   while (!ToVisit.empty()) {
 // Retrieve the next class in the inheritance hierarchy.
 const RecordType *NextT = ToVisit.pop_back_val();
+bool SkipBases = false;
 
 // If we have already seen this type, skip it.
 if (!Visited.insert(NextT).second)
@@ -1840,17 +1844,28 @@
 Info.Param = BaseInfo.Param;
 Info.FirstArg = BaseInfo.FirstArg;
 Info.SecondArg = BaseInfo.SecondArg;
+
+// In order to implement CWG2303 (added the following to p4b3):
+//   However, if there is a class C that is a (direct or indirect)
+//   base class of D and derived (directly or indirectly) from a
+//   class B and that would be a valid deduced A, the deduced A
+//   cannot be B or pointer to B, respectively.
+// We shouldn't visit the bases of a successful match ('C'), as 
they
+// could only be 'B' here.
+SkipBases = true;
   }
 
   Deduced = DeducedOrig;
 }
 
 // Visit base classes
-CXXRecordDecl *Next = cast(NextT->getDecl());
-for (const auto  : Next->bases()) {
-  assert(Base.getType()->isRecordType() &&
- "Base class that isn't a record?");
-  ToVisit.push_back(Base.getType()->getAs());
+if (!SkipBases) {
+  CXXRecordDecl *Next = cast(NextT->getDecl());
+  for (const auto  : Next->bases()) {
+assert(Base.getType()->isRecordType() &&
+   "Base class that isn't a record?");
+ToVisit.push_back(Base.getType()->getAs());
+  }
 }
   }
 


Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -113,3 +113,26 @@
   extern template const int d;
 #endif
 }
+
+#if __cplusplus >= 201103L
+namespace dr2303 { // dr2303: 12
+template 
+struct A;
+template <>
+struct A<> {};
+template 
+struct A : A {};
+struct B : A {};
+
+template 
+void f(const A &);
+template 
+void f2(const A *);
+
+void g() {
+  f(B{}); // This is no longer ambiguous.
+  B b;
+  f2();
+}
+} //namespace dr2303
+#endif
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -1792,7 +1792,10 @@
   //   transformed A can be a derived class of the deduced A. Likewise if
   //   P is a pointer to a class of the form simple-template-id, the
   //   transformed A can be a pointer to a derived class pointed to by the
-  //   deduced A.
+  //   deduced A. However, if there is a class C that is a (direct or
+  //   indirect) base class of D and derived (directly or indirectly) from a
+  //   class B and that would be a valid deduced A, the deduced A cannot be
+  //   B or pointer to B, respectively.
   //
   //   These alternatives are considered only if type deduction would
   //   otherwise fail. If 

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

> Ah, doh, I see a few now. Is that number the compiler version number?

Yup


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

https://reviews.llvm.org/D84048



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D84048#2165026 , @erichkeane wrote:

> In D84048#2165023 , @riccibruno 
> wrote:
>
> > > Also make sure to update the the cxx_dr_status.html document.
> >
> > It is auto-generated. You need to tag the test case: `dr2303: 12`.
> >
> > Edit: And put it at the right place in the file (not at the end).
>
>
> Do you have an example of this?  I've not seen that anywhere before.


The test-case just above `namespace dr2387 { // dr2387: 9`.


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

https://reviews.llvm.org/D84048



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D84048#2165026 , @erichkeane wrote:

> In D84048#2165023 , @riccibruno 
> wrote:
>
> > > Also make sure to update the the cxx_dr_status.html document.
> >
> > It is auto-generated. You need to tag the test case: `dr2303: 12`.
> >
> > Edit: And put it at the right place in the file (not at the end).
>
>
> Do you have an example of this?  I've not seen that anywhere before.


Ah, doh, I see a few now. Is that number the compiler version number?


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

https://reviews.llvm.org/D84048



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D84048#2165023 , @riccibruno wrote:

> > Also make sure to update the the cxx_dr_status.html document.
>
> It is auto-generated. You need to tag the test case: `dr2303: 12`.
>
> Edit: And put it at the right place in the file (not at the end).


Do you have an example of this?  I've not seen that anywhere before.


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

https://reviews.llvm.org/D84048



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

> Also make sure to update the the cxx_dr_status.html document.

It is auto-generated. You need to tag the test case: `dr2303: 12`.


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

https://reviews.llvm.org/D84048



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 279605.
zequanwu added a comment.

Change `@LINE` to `# @LINE`.


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

https://reviews.llvm.org/D83592

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h
  clang/lib/Lex/Preprocessor.cpp
  clang/test/CoverageMapping/break.c
  clang/test/CoverageMapping/builtinmacro.c
  clang/test/CoverageMapping/classtemplate.cpp
  clang/test/CoverageMapping/comment-in-macro.c
  clang/test/CoverageMapping/continue.c
  clang/test/CoverageMapping/coroutine.cpp
  clang/test/CoverageMapping/deferred-region.cpp
  clang/test/CoverageMapping/if.cpp
  clang/test/CoverageMapping/includehell.cpp
  clang/test/CoverageMapping/label.cpp
  clang/test/CoverageMapping/logical.cpp
  clang/test/CoverageMapping/loops.cpp
  clang/test/CoverageMapping/macro-expressions.cpp
  clang/test/CoverageMapping/macroparams2.c
  clang/test/CoverageMapping/macros.c
  clang/test/CoverageMapping/macroscopes.cpp
  clang/test/CoverageMapping/moremacros.c
  clang/test/CoverageMapping/objc.m
  clang/test/CoverageMapping/pr32679.cpp
  clang/test/CoverageMapping/preprocessor.c
  clang/test/CoverageMapping/return.c
  clang/test/CoverageMapping/switch.cpp
  clang/test/CoverageMapping/switchmacro.c
  clang/test/CoverageMapping/test.c
  clang/test/CoverageMapping/trycatch.cpp
  clang/test/CoverageMapping/unreachable-macro.c
  clang/test/CoverageMapping/while.c
  clang/test/lit.cfg.py
  compiler-rt/test/profile/Inputs/instrprof-comdat.h
  compiler-rt/test/profile/coverage_comments.cpp

Index: compiler-rt/test/profile/coverage_comments.cpp
===
--- /dev/null
+++ compiler-rt/test/profile/coverage_comments.cpp
@@ -0,0 +1,71 @@
+// RUN: %clangxx_profgen -fcoverage-mapping -Wno-comment -o %t %s
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw
+// RUN: llvm-cov show %t -instr-profile %t.profdata -path-equivalence=/tmp,%S 2>&1 | FileCheck %s
+
+int main() {   // CHECK:  [[# @LINE]]| 1|int main() {
+/* comment */ int x = 0;   // CHECK:  [[# @LINE]]| 1|  /* comment */ int x = 0;
+int y = 0; /* comment */   // CHECK:  [[# @LINE]]| 1|  int y = 0; /* comment */
+int z = 0; // comment  // CHECK:  [[# @LINE]]| 1|  int z = 0; // comment
+// comment // CHECK:  [[# @LINE]]|  |  // comment
+   // CHECK:  [[# @LINE]]|  |
+x = 0; /*  // CHECK:  [[# @LINE]]| 1|  x = 0; /*
+comment// CHECK:  [[# @LINE]]|  |comment
+*/ // CHECK:  [[# @LINE]]|  |*/
+   // CHECK:  [[# @LINE]]|  |
+/* // CHECK:  [[# @LINE]]|  |/*
+comment// CHECK:  [[# @LINE]]|  |comment
+*/ x = 0;  // CHECK:  [[# @LINE]]| 1|*/ x = 0;
+   // CHECK:  [[# @LINE]]|  |
+/* comment */  // CHECK:  [[# @LINE]]|  |/* comment */
+// comment // CHECK:  [[# @LINE]]|  |// comment
+/* comment */  // CHECK:  [[# @LINE]]|  |/* comment */
+z =// CHECK:  [[# @LINE]]| 1|z =
+x // comment   // CHECK:  [[# @LINE]]| 1|x // comment
+// comment // CHECK:  [[# @LINE]]|  |// comment
++ /*   // CHECK:  [[# @LINE]]| 1|+ /*
+comment// CHECK:  [[# @LINE]]|  |comment
+*/ // CHECK:  [[# @LINE]]|  |*/
+/* // CHECK:  [[# @LINE]]|  |/*
+comment// CHECK:  [[# @LINE]]|  |comment
+*/y;   // CHECK:  [[# @LINE]]| 1|*/y;
+   // CHECK:  [[# @LINE]]|  |
+// Comments inside directives. // CHECK:  [[# @LINE]]|  |// Comments inside directives.
+#if 0 //comment// CHECK:  [[# @LINE]]|  |#if 0 //comment
+/* comment */ x = 0;   // CHECK:  [[# @LINE]]|  |/* comment */ x = 0;
+y = 0; /* comment */   // CHECK:  [[# @LINE]]|  |y = 0; /* comment */
+z = 0; // comment  // CHECK:  [[# @LINE]]|  |z = 0; // comment
+// comment // CHECK:  [[# @LINE]]|  |// comment
+   // CHECK:  [[# @LINE]]|  |
+x = 0; /*  // CHECK:  [[# @LINE]]|  |x = 0; /*
+comment// CHECK:  [[# @LINE]]|  |  

Re: [clang-tools-extra] 9791416 - Silence a "logical operation on address of string constant" via CMake instead.

[David Blaikie via cfe-commits]

Cool - thanks for the context!

On Tue, Jul 21, 2020 at 5:44 AM Aaron Ballman 
wrote:

> On Mon, Jul 20, 2020 at 5:44 PM David Blaikie  wrote:
> >
> > Should the warning be disabled for LLVM overall, rather than only for
> > this subproject? (& be good tohave some details in the commit at least
> > of why this warning is being disabled - how it is noisy/unhelpful/etc)
>
> Thank you for pointing out the lack of details in the commit message,
> sorry about that! I didn't think it should be disabled for LLVM
> overall because the issue was highly local to that particular file.
> Effectively, there's a #define/#include/#undef pattern in that file
> where some of the macro expansions would expand out to "string
> literal" == nullptr, which MSVC would complain about. Replacing the
> macro expansion with different code caused other diagnostics, so this
> was an expedient solution. I think if we found a second place where we
> need to disable the diagnostic, it might make sense to disable it
> globally at that point, but I wouldn't be strongly opposed to
> silencing it globally if someone felt strongly.
>
> ~Aaron
>
> >
> > On Sun, Jul 19, 2020 at 8:20 AM Aaron Ballman via cfe-commits
> >  wrote:
> > >
> > >
> > > Author: Aaron Ballman
> > > Date: 2020-07-19T11:19:48-04:00
> > > New Revision: 97914164f8454e745219566d58479b5762cccd51
> > >
> > > URL:
> https://github.com/llvm/llvm-project/commit/97914164f8454e745219566d58479b5762cccd51
> > > DIFF:
> https://github.com/llvm/llvm-project/commit/97914164f8454e745219566d58479b5762cccd51.diff
> > >
> > > LOG: Silence a "logical operation on address of string constant" via
> CMake instead.
> > >
> > > Added:
> > >
> > >
> > > Modified:
> > > clang-tools-extra/clangd/CMakeLists.txt
> > >
> > > Removed:
> > >
> > >
> > >
> > >
> 
> > > diff  --git a/clang-tools-extra/clangd/CMakeLists.txt
> b/clang-tools-extra/clangd/CMakeLists.txt
> > > index b3002b1d5698..8db6656e5291 100644
> > > --- a/clang-tools-extra/clangd/CMakeLists.txt
> > > +++ b/clang-tools-extra/clangd/CMakeLists.txt
> > > @@ -28,6 +28,10 @@ set(LLVM_LINK_COMPONENTS
> > >Option
> > >)
> > >
> > > +if(MSVC AND NOT CLANG_CL)
> > > + set_source_files_properties(CompileCommands.cpp PROPERTIES
> COMPILE_FLAGS -wd4130) # disables C4130: logical operation on address of
> string constant
> > > +endif()
> > > +
> > >  add_clang_library(clangDaemon
> > >AST.cpp
> > >ClangdLSPServer.cpp
> > >
> > >
> > >
> > > ___
> > > cfe-commits mailing list
> > > cfe-commits@lists.llvm.org
> > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84260: [OpenMP][AMDGCN] Support OpenMP offloading for AMDGCN architecture - Part 3

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.h:25
 
-class CGOpenMPRuntimeNVPTX : public CGOpenMPRuntimeGPU {
+class CGOpenMPRuntimeAMDGCN : public CGOpenMPRuntimeGPU {
 

`final`



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h:25
 
 class CGOpenMPRuntimeNVPTX : public CGOpenMPRuntimeGPU {
 

`final`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84260



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

[PATCH] D83551: [PATCH 2/4][Sema][AArch64] Add semantics for arm_sve_vector_bits attribute

[Aaron Ballman via Phabricator via cfe-commits]

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

The attribute bits LGTM, thanks!




Comment at: clang/lib/AST/ASTContext.cpp:1887
+
+unsigned getSvePredWidth(const Type *T) { return getSveVectorWidth(T) / 8; }
+

c-rhodes wrote:
> aaron.ballman wrote:
> > Should this be dividing by the number of bits in a char for the target as 
> > opposed to hard-coding to 8?
> > Should this be dividing by the number of bits in a char for the target as 
> > opposed to hard-coding to 8?
> 
> Predicate registers in SVE hold one bit per byte of a vector register so each 
> predicate is 1/8th the size of a vector which are defined in bits, it has to 
> be 8 and I know `getCharWidth` returns 8 for the target this is implemented 
> for but I dont know what it would mean for any other target or if we care 
> about that?
Ah, so this is a case where we expect that value to always be 8 where this 
feature is supported anyway -- good to know. I still think the current code is 
a cleaner approach to hard-coding 8 bits, so thank you for making the change 
even though it doesn't enable much.


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

https://reviews.llvm.org/D83551



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

[PATCH] D84260: [OpenMP][AMDGCN] Support OpenMP offloading for AMDGCN architecture - Part 3

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam created this revision.
saiislam added reviewers: ABataev, jdoerfert, JonChesterfield, grokos.
Herald added subscribers: cfe-commits, aaron.ballman, sstefan1, guansong, 
yaxunl, mgorny, jvesely, jholewinski.
Herald added a project: clang.

Provides AMDGCN and NVPTX specific specialization of getGPUWarpSize,
getGPUThreadID, and getGPUNumThreads methods. Adds tests for AMDGCN
codegen for these methods in generic and simd modes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84260

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/OpenMP/amdgcn_target_codegen.cpp

Index: clang/test/OpenMP/amdgcn_target_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/amdgcn_target_codegen.cpp
@@ -0,0 +1,45 @@
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple amdgcn-amd-amdhsa -fopenmp-targets=amdgcn-amd-amdhsa -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+#define N 1000
+
+int test_amdgcn_target_tid_threads() {
+  // CHECK-LABEL: test_amdgcn_target_tid_threads
+  // CHECK-LABEL: entry:
+
+  int arr[N];
+
+// CHECK: %nvptx_tid = call i32 @llvm.amdgcn.workitem.id.x()
+// CHECK-DAG: %nvptx_num_threads = call i64 @__ockl_get_local_size(i32 0)
+// CHECK-DAG: [[VAR1:%[0-9]+]] = trunc i64 %nvptx_num_threads to i32
+// CHECK-DAG: %thread_limit = sub nuw i32 [[VAR1]], 64
+#pragma omp target
+  for (int i = 0; i < N; i++) {
+arr[i] = 1;
+  }
+
+  return arr[0];
+}
+
+int test_amdgcn_target_tid_threads_simd() {
+  // CHECK-LABEL: test_amdgcn_target_tid_threads_simd
+  // CHECK-LABEL: entry:
+
+  int arr[N];
+
+// CHECK: %nvptx_num_threads = call i64 @__ockl_get_local_size(i32 0)
+// CHECK-DAG: [[VAR2:%[0-9]+]] = trunc i64 %nvptx_num_threads to i32
+// CHECK-DAG: call void @__kmpc_spmd_kernel_init(i32 [[VAR2]], i16 0, i16 0)
+#pragma omp target simd
+  for (int i = 0; i < N; i++) {
+arr[i] = 1;
+  }
+  return arr[0];
+}
+
+#endif
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -19,6 +19,7 @@
 #include "CGObjCRuntime.h"
 #include "CGOpenCLRuntime.h"
 #include "CGOpenMPRuntime.h"
+#include "CGOpenMPRuntimeAMDGCN.h"
 #include "CGOpenMPRuntimeNVPTX.h"
 #include "CodeGenFunction.h"
 #include "CodeGenPGO.h"
@@ -215,6 +216,11 @@
"OpenMP NVPTX is only prepared to deal with device code.");
 OpenMPRuntime.reset(new CGOpenMPRuntimeNVPTX(*this));
 break;
+  case llvm::Triple::amdgcn:
+assert(getLangOpts().OpenMPIsDevice &&
+   "OpenMP AMDGCN is only prepared to deal with device code.");
+OpenMPRuntime.reset(new CGOpenMPRuntimeAMDGCN(*this));
+break;
   default:
 if (LangOpts.OpenMPSimd)
   OpenMPRuntime.reset(new CGOpenMPSIMDRuntime(*this));
Index: clang/lib/CodeGen/CMakeLists.txt
===
--- clang/lib/CodeGen/CMakeLists.txt
+++ clang/lib/CodeGen/CMakeLists.txt
@@ -62,6 +62,7 @@
   CGObjCRuntime.cpp
   CGOpenCLRuntime.cpp
   CGOpenMPRuntime.cpp
+  CGOpenMPRuntimeAMDGCN.cpp
   CGOpenMPRuntimeGPU.cpp
   CGOpenMPRuntimeNVPTX.cpp
   CGRecordLayoutBuilder.cpp
Index: clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -39,3 +39,21 @@
   (), llvm::Intrinsic::nvvm_read_ptx_sreg_warpsize),
   "nvptx_warp_size");
 }
+
+/// Get the id of the current thread on the GPU.
+llvm::Value *CGOpenMPRuntimeNVPTX::getGPUThreadID(CodeGenFunction ) {
+  CGBuilderTy  = CGF.Builder;
+  llvm::Function *F;
+  F = llvm::Intrinsic::getDeclaration(
+  (), llvm::Intrinsic::nvvm_read_ptx_sreg_tid_x);
+  return Bld.CreateCall(F, llvm::None, "nvptx_tid");
+}
+
+/// Get the maximum number of threads in a block of the GPU.
+llvm::Value *CGOpenMPRuntimeNVPTX::getGPUNumThreads(CodeGenFunction ) {
+  CGBuilderTy  = CGF.Builder;
+  llvm::Function *F;
+  F = llvm::Intrinsic::getDeclaration(
+  (), llvm::Intrinsic::nvvm_read_ptx_sreg_ntid_x);
+  return Bld.CreateCall(F, llvm::None, "nvptx_num_threads");
+}
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
+++ 

[PATCH] D84225: [CFE] Add nomerge function attribute to inline assembly.

[Zequan Wu via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84225



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

New tests should use `[[#@LINE]]` instead of `[[@LINE]]`

https://llvm.org/docs/CommandGuide/FileCheck.html

> To support legacy uses of @LINE as a special string variable, FileCheck also 
> accepts the following uses of @LINE with string substitution block syntax: 
> [[@LINE]], [[@LINE+]] and [[@LINE-]] without any spaces 
> inside the brackets and where offset is an integer.


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

https://reviews.llvm.org/D83592



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 279598.
erichkeane added a comment.

Also make sure to update the the cxx_dr_status.html document.

Additionally, I sent out a CWG message on the reflector about this: 
https://godbolt.org/z/bEr61T

My implementation has this as ambiguous, but the wording makes it well-formed I 
think.  I don't think that was the intent of CWG2303, so unless they confirm  
that this was their intent (or that I'm wrong), I'll leave this patch as is.


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

https://reviews.llvm.org/D84048

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/drs/dr23xx.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -13633,7 +13633,7 @@
 https://wg21.link/cwg2303;>2303
 DRWP
 Partial ordering and recursive variadic inheritance
-Unknown
+Yes
   
   
 https://wg21.link/cwg2304;>2304
Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -113,3 +113,26 @@
   extern template const int d;
 #endif
 }
+
+#if __cplusplus >= 201103L
+namespace dr2303 {
+template 
+struct A;
+template <>
+struct A<> {};
+template 
+struct A : A {};
+struct B : A {};
+
+template 
+void f(const A &);
+template 
+void f2(const A *);
+
+void g() {
+  f(B{}); // This is no longer ambiguous.
+  B b;
+  f2();
+}
+} //namespace dr2303
+#endif
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -1792,7 +1792,10 @@
   //   transformed A can be a derived class of the deduced A. Likewise if
   //   P is a pointer to a class of the form simple-template-id, the
   //   transformed A can be a pointer to a derived class pointed to by the
-  //   deduced A.
+  //   deduced A. However, if there is a class C that is a (direct or
+  //   indirect) base class of D and derived (directly or indirectly) from a
+  //   class B and that would be a valid deduced A, the deduced A cannot be
+  //   B or pointer to B, respectively.
   //
   //   These alternatives are considered only if type deduction would
   //   otherwise fail. If they yield more than one possible deduced A, the
@@ -1812,6 +1815,7 @@
   while (!ToVisit.empty()) {
 // Retrieve the next class in the inheritance hierarchy.
 const RecordType *NextT = ToVisit.pop_back_val();
+bool SkipBases = false;
 
 // If we have already seen this type, skip it.
 if (!Visited.insert(NextT).second)
@@ -1840,17 +1844,28 @@
 Info.Param = BaseInfo.Param;
 Info.FirstArg = BaseInfo.FirstArg;
 Info.SecondArg = BaseInfo.SecondArg;
+
+// In order to implement CWG2303 (added the following to p4b3):
+//   However, if there is a class C that is a (direct or indirect)
+//   base class of D and derived (directly or indirectly) from a
+//   class B and that would be a valid deduced A, the deduced A
+//   cannot be B or pointer to B, respectively.
+// We shouldn't visit the bases of a successful match ('C'), as they
+// could only be 'B' here.
+SkipBases = true;
   }
 
   Deduced = DeducedOrig;
 }
 
 // Visit base classes
-CXXRecordDecl *Next = cast(NextT->getDecl());
-for (const auto  : Next->bases()) {
-  assert(Base.getType()->isRecordType() &&
- "Base class that isn't a record?");
-  ToVisit.push_back(Base.getType()->getAs());
+if (!SkipBases) {
+  CXXRecordDecl *Next = cast(NextT->getDecl());
+  for (const auto  : Next->bases()) {
+assert(Base.getType()->isRecordType() &&
+   "Base class that isn't a record?");
+ToVisit.push_back(Base.getType()->getAs());
+  }
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

LGTM, though I have a possible code simplification if you think it's an 
improvement.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp:78
+
+  if (const auto *DeclRef = Result.Nodes.getNodeAs("expr")) {
+const Decl *D = DeclRef->getDecl();

Can you get away with:
```
if (const auto *E = Result.Nodes.getNodeAs("expr")) {
  const Decl *D = isa ? cast(E)->getDecl() : 
cast(E)->getMemberDecl();
  const auto M = ignoringParenImpCasts(anyOf(declRefExpr(to(equalsNode(D))), 
memberExpr(hasDeclaration(equalsNode(D);
  checkImpl(Result, E, If, M, *this);
}
```
(Totally untested, but it hopefully demonstrates the point.)


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

https://reviews.llvm.org/D83188



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

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D83261#2162904 , @Meinersbur wrote:

> In D83261#2162561 , @ABataev wrote:
>
> > 1. OMPChildren class uses standard TrailingObjects harness instead of 
> > manual calculation.
>
>
> I was going to argue that OMPExecutableDirective could derive from 
> TrailingObjects as well, but it requires a template parameter for its final 
> subclass. Good point!
>
> > 2. Child Stmt* based nodes are not related to the AsssociatedStmt* anymore 
> > and can exist independently.
>
> It moved into the OMPChildren class. Not sure whether it is better.


What I mean, that previously child nodes could not exist without 
AssociatedStmt, i.e. you have to have AssociatedStmt to have other children. It 
is solved (though, of course, can be implemented in a different way with the 
current design).

> 
> 
>>> OMPChildren also handles clauses for OMPExecutableDirectives but not for 
>>> declarative directives. Should handling of of clauses also be extracted 
>>> into into its own class? That would make (de-)serialization easier for 
>>> those as well.
>> 
>> This class is only for executable directives.
> 
> Nearly all directives have clauses. At the moment they all implement their 
> own clause list. I don't see why clause management should be centralized for 
> executable statements only.

Sure, we can make `OMPChildren` common for declarative and executable 
directives. Do you want me to do it?

> 
> 
>>> There is no effect on D76342  (except a 
>>> requiring a rebase), since the OMPTileDirective has children and thus does 
>>> not profit from the functionality of `OMPChildren` becoming optional. Since 
>>> I don't see a relation to D76342 , more , 
>>> I am indifferent to whether this should be merged.
>> 
>> There should be an additional patch, which, I hope, should simplify things 
>> for loop-based directives.
> 
> OK. What does this patch do? Are you going to upload it as well?

At first, need to deal with this one, at least come to an agreement with the 
design.

> 
> 
>>> Trailing objects is a technique to ensure that all substmts are consecutive 
>>> in memory (so `StmtIterator` can iterator over them). For 
>>> OMPExeuctableDirectives, only the associated statement is returned by the 
>>> `StmtIterator`, i.e. all the children could be made regular class members 
>>> without the complication of computing the offset. I'd prefer that change 
>>> over OMPChildren.
>> 
>> There are also used_children, which are used by the clang analyzer for, at 
>> least, use of uninitialized variables diagnostics.
> 
> OK, I see.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83261



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 279595.
zequanwu marked 2 inline comments as done.
zequanwu retitled this revision from "[Parser] Add comment to skipped regions" 
to "[Coverage] Add comment to skipped regions".
zequanwu edited the summary of this revision.
zequanwu added a comment.

- Address comments
- Update summary.


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

https://reviews.llvm.org/D83592

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h
  clang/lib/Lex/Preprocessor.cpp
  clang/test/CoverageMapping/break.c
  clang/test/CoverageMapping/builtinmacro.c
  clang/test/CoverageMapping/classtemplate.cpp
  clang/test/CoverageMapping/comment-in-macro.c
  clang/test/CoverageMapping/continue.c
  clang/test/CoverageMapping/coroutine.cpp
  clang/test/CoverageMapping/deferred-region.cpp
  clang/test/CoverageMapping/if.cpp
  clang/test/CoverageMapping/includehell.cpp
  clang/test/CoverageMapping/label.cpp
  clang/test/CoverageMapping/logical.cpp
  clang/test/CoverageMapping/loops.cpp
  clang/test/CoverageMapping/macro-expressions.cpp
  clang/test/CoverageMapping/macroparams2.c
  clang/test/CoverageMapping/macros.c
  clang/test/CoverageMapping/macroscopes.cpp
  clang/test/CoverageMapping/moremacros.c
  clang/test/CoverageMapping/objc.m
  clang/test/CoverageMapping/pr32679.cpp
  clang/test/CoverageMapping/preprocessor.c
  clang/test/CoverageMapping/return.c
  clang/test/CoverageMapping/switch.cpp
  clang/test/CoverageMapping/switchmacro.c
  clang/test/CoverageMapping/test.c
  clang/test/CoverageMapping/trycatch.cpp
  clang/test/CoverageMapping/unreachable-macro.c
  clang/test/CoverageMapping/while.c
  clang/test/lit.cfg.py
  compiler-rt/test/profile/Inputs/instrprof-comdat.h
  compiler-rt/test/profile/coverage_comments.cpp

Index: compiler-rt/test/profile/coverage_comments.cpp
===
--- /dev/null
+++ compiler-rt/test/profile/coverage_comments.cpp
@@ -0,0 +1,71 @@
+// RUN: %clangxx_profgen -fcoverage-mapping -Wno-comment -o %t %s
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw
+// RUN: llvm-cov show %t -instr-profile %t.profdata -path-equivalence=/tmp,%S 2>&1 | FileCheck %s
+
+int main() {   // CHECK:  [[@LINE]]| 1|int main() {
+/* comment */ int x = 0;   // CHECK:  [[@LINE]]| 1|  /* comment */ int x = 0;
+int y = 0; /* comment */   // CHECK:  [[@LINE]]| 1|  int y = 0; /* comment */
+int z = 0; // comment  // CHECK:  [[@LINE]]| 1|  int z = 0; // comment
+// comment // CHECK:  [[@LINE]]|  |  // comment
+   // CHECK:  [[@LINE]]|  |
+x = 0; /*  // CHECK:  [[@LINE]]| 1|  x = 0; /*
+comment// CHECK:  [[@LINE]]|  |comment
+*/ // CHECK:  [[@LINE]]|  |*/
+   // CHECK:  [[@LINE]]|  |
+/* // CHECK:  [[@LINE]]|  |/*
+comment// CHECK:  [[@LINE]]|  |comment
+*/ x = 0;  // CHECK:  [[@LINE]]| 1|*/ x = 0;
+   // CHECK:  [[@LINE]]|  |
+/* comment */  // CHECK:  [[@LINE]]|  |/* comment */
+// comment // CHECK:  [[@LINE]]|  |// comment
+/* comment */  // CHECK:  [[@LINE]]|  |/* comment */
+z =// CHECK:  [[@LINE]]| 1|z =
+x // comment   // CHECK:  [[@LINE]]| 1|x // comment
+// comment // CHECK:  [[@LINE]]|  |// comment
++ /*   // CHECK:  [[@LINE]]| 1|+ /*
+comment// CHECK:  [[@LINE]]|  |comment
+*/ // CHECK:  [[@LINE]]|  |*/
+/* // CHECK:  [[@LINE]]|  |/*
+comment// CHECK:  [[@LINE]]|  |comment
+*/y;   // CHECK:  [[@LINE]]| 1|*/y;
+   // CHECK:  [[@LINE]]|  |
+// Comments inside directives. // CHECK:  [[@LINE]]|  |// Comments inside directives.
+#if 0 //comment// CHECK:  [[@LINE]]|  |#if 0 //comment
+/* comment */ x = 0;   // CHECK:  [[@LINE]]|  |/* comment */ x = 0;
+y = 0; /* comment */   // CHECK:  [[@LINE]]|  |y = 0; /* comment */
+z = 0; // comment  // CHECK:  [[@LINE]]|  |z = 0; // comment
+// comment // CHECK:  [[@LINE]]|  |// comment
+   // CHECK:  

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

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Also, what about compatibility with declare mapper? Can you add tests for it?




Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7809-7813
+MapValuesArrayTy CurOffsets{llvm::ConstantInt::get(CGF.CGM.Int64Ty, 0)};
+MapValuesArrayTy CurCounts{llvm::ConstantInt::get(CGF.CGM.Int64Ty, 1)};
+MapValuesArrayTy CurStrides;
+SmallVector DimSizes{
+llvm::ConstantInt::get(CGF.CGM.Int64Ty, 1)};

DO not use braced initializer list 
https://llvm.org/docs/CodingStandards.html#id26



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7839-7847
+if (CAT) {
+  ElementType = CAT->getElementType().getTypePtr();
+} else if (VAT) {
+  ElementType = VAT->getElementType().getTypePtr();
+} else {
+  assert( == &*Components.begin() &&
+ "Only expect pointer (non CAT or VAT) when this is the "

No need for braces here per coding standard



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7876
+
+// Skip the dummy dimension since we have already have its information.
+auto DI = DimSizes.begin() + 1;

have already have



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7892
+  const Expr *AssocExpr = Component.getAssociatedExpression();
+  AssocExpr->dump();
+  const auto *OASE = dyn_cast(AssocExpr);

debug code



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7893
+  AssocExpr->dump();
+  const auto *OASE = dyn_cast(AssocExpr);
+  const auto *AE = dyn_cast(AssocExpr);

Move this after the first `if` statement



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7894-7896
+  const auto *AE = dyn_cast(AssocExpr);
+
+  if (AE) {

Better to make it something like:
```
if (const auto *AE = dyn_cast(AssocExpr))
```



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7944-7945
+  : llvm::ConstantInt::get(CGF.Int64Ty, /*V=*/1);
+  Count = CGF.Builder.CreateUDiv(CGF.Builder.CreateNUWSub(*DI, Offset),
+ Stride);
+}

If no stride at all, why need to divide?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7967
+  DimProd = CGF.Builder.CreateNUWMul(DimProd, *(DI - 1));
+  CurStrides.push_back(CGF.Builder.CreateNUWMul(DimProd, Stride));
+  if (DI != DimSizes.end())

Same, if no stride at all, no need to mul.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:16913-16919
+if (OASE) {
+  if (!OASE->getLength())
+SemaRef.Diag(ELoc, diag::err_array_section_does_not_specify_length)
+<< ERange;
+  else
+break;
+}

Better to transform it to something like:
```
if (!OASE || OASE->getLength())
  break;
SemaRef.Diag(...)
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84192



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

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman removed a reviewer: aaron.ballman.
aaron.ballman added a comment.

While this looks reasonable to me, I don't feel comfortable signing off on it 
because Templates Are Complicated (switching myself to a subscriber instead).


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

https://reviews.llvm.org/D84048



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