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

2020-09-19 Thread LuoYuanke via Phabricator via cfe-commits 
LuoYuanke created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, mgorny, 
qcolombet, MatzeB.
Herald added projects: clang, LLVM.
LuoYuanke requested review of this revision.
Herald added a subscriber: jdoerfert.

Change-Id: I935e1080916ffcb72af54c2c83faa8b2e97d5cb0


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87981

Files:
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/lib/Headers/amxintrin.h
  clang/test/CodeGen/AMX/amx_api.c
  llvm/include/llvm/CodeGen/LiveIntervalUnion.h
  llvm/include/llvm/CodeGen/LiveRegMatrix.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/CodeGen/TileShapeInfo.h
  llvm/include/llvm/CodeGen/VirtRegMap.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/CodeGen/InlineSpiller.cpp
  llvm/lib/CodeGen/LiveIntervalUnion.cpp
  llvm/lib/CodeGen/LiveRegMatrix.cpp
  llvm/lib/CodeGen/VirtRegMap.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86ExpandPseudo.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrAMX.td
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86MachineFunctionInfo.h
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86RegisterInfo.h
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Target/X86/X86TileConfig.cpp
  llvm/test/CodeGen/X86/AMX/amx-config.ll
  llvm/test/CodeGen/X86/AMX/amx-spill.ll
  llvm/test/CodeGen/X86/AMX/amx-type.ll
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -246,7 +246,8 @@
   IIT_SUBDIVIDE4_ARG = 45,
   IIT_VEC_OF_BITCASTS_TO_INT = 46,
   IIT_V128 = 47,
-  IIT_BF16 = 48
+  IIT_BF16 = 48,
+  IIT_V256 = 49
 };
 
 static void EncodeFixedValueType(MVT::SimpleValueType VT,
@@ -384,6 +385,7 @@
 case 32: Sig.push_back(IIT_V32); break;
 case 64: Sig.push_back(IIT_V64); break;
 case 128: Sig.push_back(IIT_V128); break;
+case 256: Sig.push_back(IIT_V256); break;
 case 512: Sig.push_back(IIT_V512); break;
 case 1024: Sig.push_back(IIT_V1024); break;
 }
Index: llvm/test/CodeGen/X86/AMX/amx-type.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/AMX/amx-type.ll
@@ -0,0 +1,143 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -lower-amx-type %s -S | FileCheck %s
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.__tile_str = type { i16, i16, <256 x i32> }
+
+@buf = dso_local global [1024 x i8] zeroinitializer, align 16
+@buf2 = dso_local global [1024 x i8] zeroinitializer, align 16
+
+define dso_local void @test_load(i8* %in, i8* %out) local_unnamed_addr #2 {
+; CHECK-LABEL: @test_load(
+; CHECK-NEXT:[[TMP1:%.*]] = bitcast i8* [[IN:%.*]] to <256 x i32>*
+; CHECK-NEXT:[[TMP2:%.*]] = bitcast i8* [[OUT:%.*]] to <256 x i32>*
+; CHECK-NEXT:[[TMP3:%.*]] = bitcast <256 x i32>* [[TMP1]] to <128 x i32>*
+; CHECK-NEXT:[[TMP4:%.*]] = load <128 x i32>, <128 x i32>* [[TMP3]], align 64
+; CHECK-NEXT:[[TMP5:%.*]] = getelementptr <128 x i32>, <128 x i32>* [[TMP3]], i32 1
+; CHECK-NEXT:[[TMP6:%.*]] = load <128 x i32>, <128 x i32>* [[TMP5]], align 64
+; CHECK-NEXT:[[TMP7:%.*]] = bitcast <256 x i32>* [[TMP2]] to <128 x i32>*
+; CHECK-NEXT:store <128 x i32> [[TMP4]], <128 x i32>* [[TMP7]], align 64
+; CHECK-NEXT:[[TMP8:%.*]] = getelementptr <128 x i32>, <128 x i32>* [[TMP7]], i32 1
+; CHECK-NEXT:store <128 x i32> [[TMP6]], <128 x i32>* [[TMP8]], align 64
+; CHECK-NEXT:ret void
+;
+  %1 = bitcast i8* %in to <256 x i32>*
+  %2 = bitcast i8* %out to <256 x i32>*
+  %3 = load <256 x i32>, <256 x i32>* %1, align 64, !tbaa !8
+  store <256 x i32> %3, <256 x i32>* %2, align 64, !tbaa !8
+  ret void
+}
+
+define dso_local void @__tile_loadd(%struct.__tile_str* nocapture %0, i8* %1, i64 %2) local_unnamed_addr #0 {
+; CHECK-LABEL: @__tile_loadd(
+; CHECK-NEXT:[[TMP4:%.*]] = getelementptr inbounds [[STRUCT___TILE_STR:%.*]], %struct.__tile_str* [[TMP0:%.*]], i64 0, i32 0
+; CHECK-NEXT:[[TMP5:%.*]] = load i16, i16* [[TMP4]], align 64, [[TBAA2:!tbaa !.*]]
+; CHECK-NEXT:[[TMP6:%.*]] = getelementptr inbounds [[STRUCT___TILE_STR]], %struct.__tile_str* [[TMP0]], i64 0, i32 1
+; CHECK-NEXT:[[TMP7:%.*]] = load i16, i16* [[TMP6]], align 2, [[TBAA7:!tbaa !.*]]
+; CHECK-NEXT:[[TMP8:%.*]] = shl i64 [[TMP2:%.*]], 32
+; CHECK-NEXT:[[TMP9:%.*]] = ashr exact i64 [[TMP8]], 32
+; CHECK-NEXT:[[TMP10:%.*]] = tail call <256 x i32>

[PATCH] D87953: [xray] Function coverage groups

2020-09-19 Thread Ian Levesque via Phabricator via cfe-commits 
ianlevesque added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:817
+CurFn->getName().bytes_end());
+  auto Group = crc32(FuncName) % FuncGroups;
+  if (Group != CGM.getCodeGenOpts().XRaySelectedFunctionGroup &&

dberris wrote:
> I'm a little concerned that this is randomly determined. Could we think about 
> using a function attribute instead that opts functions into groups by name? 
> Or is the intent to have "sampling" in the coverage information instead of 
> full coverage?
The intention is to have something deterministic across builds but yeah 
sampled-ish across the entire codebase.  By iterating over the groups with 
different builds we are hoping to cover every function eventually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87953

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

[PATCH] D87953: [xray] Function coverage groups

2020-09-19 Thread Dean Michael Berris via Phabricator via cfe-commits 
dberris added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:817
+CurFn->getName().bytes_end());
+  auto Group = crc32(FuncName) % FuncGroups;
+  if (Group != CGM.getCodeGenOpts().XRaySelectedFunctionGroup &&

I'm a little concerned that this is randomly determined. Could we think about 
using a function attribute instead that opts functions into groups by name? Or 
is the intent to have "sampling" in the coverage information instead of full 
coverage?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87953

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

[PATCH] D87979: [clangd] Link libclangdSupport into clangd-index-server

2020-09-19 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
mgorny.
Herald added a project: clang.
nridge requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Fixes https://github.com/clangd/clangd/issues/534


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87979

Files:
  clang-tools-extra/clangd/index/remote/server/CMakeLists.txt


Index: clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
@@ -11,6 +11,7 @@
 target_link_libraries(clangd-index-server
   PRIVATE
   clangDaemon
+  clangdSupport
 
   RemoteIndexProtos
   clangdRemoteMarshalling


Index: clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
===
--- clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
+++ clang-tools-extra/clangd/index/remote/server/CMakeLists.txt
@@ -11,6 +11,7 @@
 target_link_libraries(clangd-index-server
   PRIVATE
   clangDaemon
+  clangdSupport
 
   RemoteIndexProtos
   clangdRemoteMarshalling
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: Summary: [Builtin] Add __builtin_zero_non_value_bits.

2020-09-19 Thread JF Bastien via Phabricator via cfe-commits 
jfb added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1682
+
+  size_t NumFeilds = std::distance(R->field_begin(), R->field_end());
+  auto CurrentField = R->field_begin();

Typo in "fields".



Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp:160
+
+int main() {
+  testAllForType<32, 16, char>(11, 22, 33, 44);

Usually CodeGen tests will use lit to check the emitted IR matches 
expectations. I think that's what you want to do here.

Remember to test `volatile` qualified pointers, as well as address spaces too.



Comment at: clang/test/SemaCXX/builtin-zero-non-value-bits.cpp:11
+  __builtin_zero_non_value_bits(e); // This should not error.
+}

You should also check incomplete types, vector, variable width integers, 
`const` qualified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87974

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

[clang] 9087209 - [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread Fangrui Song via cfe-commits 

Author: Fangrui Song
Date: 2020-09-19T15:49:44-07:00
New Revision: 9087209314caafed4b232d4a66287f2d16054ad3

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

LOG: [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning 
for -fuse-ld=/abs/path

The warning is currently not under a -W option, so it cannot be suppressed.
This is annoying for the widespread build system Bazel when specifying the path 
to gold
https://github.com/bazelbuild/bazel/commit/cdd0c3cdba270115940e8ca5ec8104cbcd694671

I have notified them about using --ld-path= forwards
https://github.com/bazelbuild/bazel/pull/8580#issuecomment-694321543
but we have to give some transitional period.

Reviewed By: dblaikie

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/lib/Driver/ToolChain.cpp
clang/test/Driver/fuse-ld.c
clang/test/Misc/warning-flags.c

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index f66d6ae70583..3bf1bb19b7ae 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -463,8 +463,9 @@ def warn_drv_msvc_not_found : Warning<
   "try running Clang from a developer command prompt">,
   InGroup>;
 
-def warn_drv_use_ld_non_word : Warning<
-  "'-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead">;
+def warn_drv_fuse_ld_path : Warning<
+  "'-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead">,
+  InGroup, DefaultIgnore;
 
 def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
   "option '-ffine-grained-bitfield-accesses' cannot be enabled together with a 
sanitizer; flag ignored">,

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index a9bd52b8afcd..d826e26bea53 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -869,6 +869,8 @@ def PointerToEnumCast : DiagGroup<"pointer-to-enum-cast",
 def PointerToIntCast : DiagGroup<"pointer-to-int-cast",
  [PointerToEnumCast, VoidPointerToIntCast]>;
 
+def FUseLdPath : DiagGroup<"fuse-ld-path">;
+
 def Move : DiagGroup<"move", [
 PessimizingMove,
 RedundantMove,
@@ -887,7 +889,8 @@ def Extra : DiagGroup<"extra", [
 UnusedParameter,
 NullPointerArithmetic,
 EmptyInitStatement,
-StringConcatation
+StringConcatation,
+FUseLdPath,
   ]>;
 
 def Most : DiagGroup<"most", [

diff  --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index a6c83689235f..0831791c8d06 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -581,7 +581,7 @@ std::string ToolChain::GetLinkerPath() const {
   // to a relative path is surprising. This is more complex due to priorities
   // among -B, COMPILER_PATH and PATH. --ld-path= should be used instead.
   if (UseLinker.find('/') != StringRef::npos)
-getDriver().Diag(diag::warn_drv_use_ld_non_word);
+getDriver().Diag(diag::warn_drv_fuse_ld_path);
 
   if (llvm::sys::path::is_absolute(UseLinker)) {
 // If we're passed what looks like an absolute path, don't attempt to

diff  --git a/clang/test/Driver/fuse-ld.c b/clang/test/Driver/fuse-ld.c
index 678913dd84c3..338d73e530e5 100644
--- a/clang/test/Driver/fuse-ld.c
+++ b/clang/test/Driver/fuse-ld.c
@@ -1,10 +1,18 @@
-// RUN: %clang %s -### \
-// RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
-// RUN: -target x86_64-unknown-linux \
-// RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+/// The absolute path warning is enabled by -Wfuse-ld-path and -Wextra.
+// RUN: %clang %s -### -target x86_64-unknown-linux -Wfuse-ld-path \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
 // CHECK-ABSOLUTE-LD: warning: '-fuse-ld=' taking a path is deprecated. Use 
'--ld-path=' instead
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 
+// RUN: %clang %s -### -target x86_64-unknown-linux -Wextra \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+
+// RUN: %clang %s -### -target x86_64-unknown-linux \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
+// CHECK-NO-WARN-NOT: warning:
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \

diff  --git a/clang/test/Misc/warning-flags.c b/clang/test/Misc/warning-flags.c
index 168f2a3551ba..e4f9069b88c8 100644
--- a/clang/test/Misc/warning-flags.c
+++

[PATCH] D87837: [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread Fangrui Song via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9087209314ca: [Driver] Add disabled-by-default -Wuse-ld-path 
for the deprecation warning for… (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87837

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/fuse-ld.c
  clang/test/Misc/warning-flags.c


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (69):
+CHECK: Warnings without flags (68):
 
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
@@ -47,7 +47,6 @@
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
 CHECK-NEXT:   warn_drv_clang_unsupported
 CHECK-NEXT:   warn_drv_pch_not_first_include
-CHECK-NEXT:   warn_drv_use_ld_non_word
 CHECK-NEXT:   warn_dup_category_def
 CHECK-NEXT:   warn_enum_value_overflow
 CHECK-NEXT:   warn_expected_qualified_after_typename
Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -1,10 +1,18 @@
-// RUN: %clang %s -### \
-// RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
-// RUN: -target x86_64-unknown-linux \
-// RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+/// The absolute path warning is enabled by -Wfuse-ld-path and -Wextra.
+// RUN: %clang %s -### -target x86_64-unknown-linux -Wfuse-ld-path \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
 // CHECK-ABSOLUTE-LD: warning: '-fuse-ld=' taking a path is deprecated. Use 
'--ld-path=' instead
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 
+// RUN: %clang %s -### -target x86_64-unknown-linux -Wextra \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+
+// RUN: %clang %s -### -target x86_64-unknown-linux \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
+// CHECK-NO-WARN-NOT: warning:
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -581,7 +581,7 @@
   // to a relative path is surprising. This is more complex due to priorities
   // among -B, COMPILER_PATH and PATH. --ld-path= should be used instead.
   if (UseLinker.find('/') != StringRef::npos)
-getDriver().Diag(diag::warn_drv_use_ld_non_word);
+getDriver().Diag(diag::warn_drv_fuse_ld_path);
 
   if (llvm::sys::path::is_absolute(UseLinker)) {
 // If we're passed what looks like an absolute path, don't attempt to
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -869,6 +869,8 @@
 def PointerToIntCast : DiagGroup<"pointer-to-int-cast",
  [PointerToEnumCast, VoidPointerToIntCast]>;
 
+def FUseLdPath : DiagGroup<"fuse-ld-path">;
+
 def Move : DiagGroup<"move", [
 PessimizingMove,
 RedundantMove,
@@ -887,7 +889,8 @@
 UnusedParameter,
 NullPointerArithmetic,
 EmptyInitStatement,
-StringConcatation
+StringConcatation,
+FUseLdPath,
   ]>;
 
 def Most : DiagGroup<"most", [
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -463,8 +463,9 @@
   "try running Clang from a developer command prompt">,
   InGroup>;
 
-def warn_drv_use_ld_non_word : Warning<
-  "'-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead">;
+def warn_drv_fuse_ld_path : Warning<
+  "'-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead">,
+  InGroup, DefaultIgnore;
 
 def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
   "option '-ffine-grained-bitfield-accesses' cannot be enabled together with a 
sanitizer; flag ignored">,


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (69):
+CHECK:

[PATCH] D87837: [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

In D87837#2283784 , @chandlerc wrote:

> Good to confirm with Dave of course, but this looks pretty great to me, and 
> thanks so much!

Works for me too!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87837

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

[PATCH] D87837: [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread Chandler Carruth via Phabricator via cfe-commits 
chandlerc added a comment.

Good to confirm with Dave of course, but this looks pretty great to me, and 
thanks so much!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87837

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

[PATCH] D87837: [Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay updated this revision to Diff 292989.
MaskRay retitled this revision from "[Driver] Add disabled-by-default 
-Wuse-ld-non-word for the deprecation warning for -fuse-ld=/abs/path" to 
"[Driver] Add disabled-by-default -Wuse-ld-path for the deprecation warning for 
-fuse-ld=/abs/path".
MaskRay edited the summary of this revision.
MaskRay added a comment.

Adopt dblaikie's suggestion: -Wfuse-ld-path


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87837

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/fuse-ld.c
  clang/test/Misc/warning-flags.c


Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (69):
+CHECK: Warnings without flags (68):
 
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
@@ -47,7 +47,6 @@
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
 CHECK-NEXT:   warn_drv_clang_unsupported
 CHECK-NEXT:   warn_drv_pch_not_first_include
-CHECK-NEXT:   warn_drv_use_ld_non_word
 CHECK-NEXT:   warn_dup_category_def
 CHECK-NEXT:   warn_enum_value_overflow
 CHECK-NEXT:   warn_expected_qualified_after_typename
Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -1,10 +1,18 @@
-// RUN: %clang %s -### \
-// RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
-// RUN: -target x86_64-unknown-linux \
-// RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+/// The absolute path warning is enabled by -Wfuse-ld-path and -Wextra.
+// RUN: %clang %s -### -target x86_64-unknown-linux -Wfuse-ld-path \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
 // CHECK-ABSOLUTE-LD: warning: '-fuse-ld=' taking a path is deprecated. Use 
'--ld-path=' instead
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 
+// RUN: %clang %s -### -target x86_64-unknown-linux -Wextra \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+
+// RUN: %clang %s -### -target x86_64-unknown-linux \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
+// CHECK-NO-WARN-NOT: warning:
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -581,7 +581,7 @@
   // to a relative path is surprising. This is more complex due to priorities
   // among -B, COMPILER_PATH and PATH. --ld-path= should be used instead.
   if (UseLinker.find('/') != StringRef::npos)
-getDriver().Diag(diag::warn_drv_use_ld_non_word);
+getDriver().Diag(diag::warn_drv_fuse_ld_path);
 
   if (llvm::sys::path::is_absolute(UseLinker)) {
 // If we're passed what looks like an absolute path, don't attempt to
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -869,6 +869,8 @@
 def PointerToIntCast : DiagGroup<"pointer-to-int-cast",
  [PointerToEnumCast, VoidPointerToIntCast]>;
 
+def FUseLdPath : DiagGroup<"fuse-ld-path">;
+
 def Move : DiagGroup<"move", [
 PessimizingMove,
 RedundantMove,
@@ -887,7 +889,8 @@
 UnusedParameter,
 NullPointerArithmetic,
 EmptyInitStatement,
-StringConcatation
+StringConcatation,
+FUseLdPath,
   ]>;
 
 def Most : DiagGroup<"most", [
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -463,8 +463,9 @@
   "try running Clang from a developer command prompt">,
   InGroup>;
 
-def warn_drv_use_ld_non_word : Warning<
-  "'-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead">;
+def warn_drv_fuse_ld_path : Warning<
+  "'-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead">,
+  InGroup, DefaultIgnore;
 
 def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
   "option '-ffine-grained-bitfield-accesses' cannot be enabled together with a 
sanitizer; flag ignored">,


Index: clang/test/Misc/warning-flags.c
===
---

[PATCH] D87974: Summary: [Builtin] Add __builtin_zero_non_value_bits.

2020-09-19 Thread Zoe Carver via Phabricator via cfe-commits 
zoecarver created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
zoecarver requested review of this revision.

Adds `__builtin_zero_non_value_bits` to zero all padding bits of a
struct.

Currently does not support unions or bitfields.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87974

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
  clang/test/SemaCXX/builtin-zero-non-value-bits.cpp

Index: clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct Foo { };
+
+void test(int a, Foo b, void* c, int *d, Foo *e) {
+  __builtin_zero_non_value_bits(a); // expected-error {{passing 'int' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int' vs structure pointer)}}
+  __builtin_zero_non_value_bits(b); // expected-error {{passing 'Foo' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('Foo' vs structure pointer)}}
+  __builtin_zero_non_value_bits(c); // expected-error {{passing 'void *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('void *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(d); // expected-error {{passing 'int *' to parameter of incompatible type structure pointer: type mismatch at 1st parameter ('int *' vs structure pointer)}}
+  __builtin_zero_non_value_bits(e); // This should not error.
+}
Index: clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp
@@ -0,0 +1,171 @@
+// RUN: mkdir -p %t
+// RUN: %clang++ %s -o %t/run
+// RUN: %t/run
+
+#include 
+#include 
+#include 
+
+template
+struct alignas(A1) BasicWithPadding {
+  T x;
+  alignas(A2) T y;
+};
+
+template
+struct alignas(A1) ArrWithPadding {
+  T x[N];
+  alignas(A2) char c;
+  T y[N];
+};
+
+template
+struct alignas(A1) PtrWithPadding {
+  T *x;
+  alignas(A2) T *y;
+};
+
+template
+struct alignas(A1) ThreeWithPadding {
+  T x;
+  alignas(A2) T y;
+  alignas(A3) T z;
+};
+
+template
+struct Normal {
+  T a;
+  T b;
+};
+
+template
+struct X {
+  T x;
+};
+
+template
+struct Z {
+  T z;
+};
+
+template
+struct YZ : public Z {
+  alignas(A) T y;
+};
+
+template
+struct alignas(A1) HasBase : public X, public YZ {
+  T a;
+  alignas(A2) T b;
+};
+
+template
+void testAllForType(T a, T b, T c, T d) {
+  using B = BasicWithPadding;
+  B basic1;
+  memset(, 0, sizeof(B));
+  basic1.x = a;
+  basic1.y = b;
+  B basic2;
+  memset(, 42, sizeof(B));
+  basic2.x = a;
+  basic2.y = b;
+  assert(memcmp(, , sizeof(B)) != 0);
+  __builtin_zero_non_value_bits();
+  assert(memcmp(, , sizeof(B)) == 0);
+
+  using A = ArrWithPadding;
+  A arr1;
+  memset(, 0, sizeof(A));
+  arr1.x[0] = a;
+  arr1.x[1] = b;
+  arr1.y[0] = c;
+  arr1.y[1] = d;
+  A arr2;
+  memset(, 42, sizeof(A));
+  arr2.x[0] = a;
+  arr2.x[1] = b;
+  arr2.y[0] = c;
+  arr2.y[1] = d;
+  arr2.c = 0;
+  assert(memcmp(, , sizeof(A)) != 0);
+  __builtin_zero_non_value_bits();
+  assert(memcmp(, , sizeof(A)) == 0);
+
+  using P = PtrWithPadding;
+  P ptr1;
+  memset(, 0, sizeof(P));
+  ptr1.x = 
+  ptr1.y = 
+  P ptr2;
+  memset(, 42, sizeof(P));
+  ptr2.x = 
+  ptr2.y = 
+  assert(memcmp(, , sizeof(P)) != 0);
+  __builtin_zero_non_value_bits();
+  assert(memcmp(, , sizeof(P)) == 0);
+
+  using Three = ThreeWithPadding;
+  Three three1;
+  memset(, 0, sizeof(Three));
+  three1.x = a;
+  three1.y = b;
+  three1.z = c;
+  Three three2;
+  memset(, 42, sizeof(Three));
+  three2.x = a;
+  three2.y = b;
+  three2.z = c;
+  __builtin_zero_non_value_bits();
+  assert(memcmp(, , sizeof(Three)) == 0);
+
+  using N = Normal;
+  N normal1;
+  memset(, 0, sizeof(N));
+  normal1.a = a;
+  normal1.b = b;
+  N normal2;
+  memset(, 42, sizeof(N));
+  normal2.a = a;
+  normal2.b = b;
+  __builtin_zero_non_value_bits();
+  assert(memcmp(, , sizeof(N)) == 0);
+
+  using H = HasBase;
+  H base1;
+  memset(, 0, sizeof(H));
+  base1.a = a;
+  base1.b = b;
+  base1.x = c;
+  base1.y = d;
+  base1.z = a;
+  H base2;
+  memset(, 42, sizeof(H));
+  base2.a = a;
+  base2.b = b;
+  base2.x = c;
+  base2.y = d;
+  base2.z = a;
+  assert(memcmp(, , sizeof(H)) != 0);
+  __builtin_zero_non_value_bits();
+  unsigned i = 0;
+  assert(memcmp(, , sizeof(H)) == 0);
+}
+
+struct Foo {
+  int x;
+  int y;
+};
+
+int main() {
+  testAllForType<32, 16, char>(11, 22, 33, 44);
+  testAllForType<64, 32, char>(4, 5, 6, 7);
+  testAllForType<32, 16, int>(0, 1, 2, 3);
+  testAllForType<64, 32, int>(4, 5, 6, 7);
+  testAllForType<32, 16, double>(0, 1, 2, 3);
+  testAllForType<64, 32, double>(4, 5, 6, 7);
+

[PATCH] D87837: [Driver] Add disabled-by-default -Wuse-ld-non-word for the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie added a comment.

Rather than "non-word" maybe "path" would be more legible (that's the 
terminology the message text uses, and seems shorter/clearer in the warning 
name/constant name/etc), ie: -Wuse-ld-path

(how do other warnings about flags do naming regarding the 'f' (or 'g', etc) 
prefix - maybe "-Wfuse-ld-path" would be a more clear?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87837

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

[PATCH] D87972: [OldPM] Pass manager: run SROA after (simple) loop unrolling

2020-09-19 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri updated this revision to Diff 292984.
lebedev.ri added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixing a few clang tests and updating one more llvm test to check this also.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87972

Files:
  clang/test/CodeGenCXX/union-tbaa2.cpp
  clang/test/Misc/loop-opt-setup.c
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/Other/unroll-sroa.ll
  llvm/test/Transforms/PhaseOrdering/X86/SROA-after-loop-unrolling.ll

Index: llvm/test/Transforms/PhaseOrdering/X86/SROA-after-loop-unrolling.ll
===
--- llvm/test/Transforms/PhaseOrdering/X86/SROA-after-loop-unrolling.ll
+++ llvm/test/Transforms/PhaseOrdering/X86/SROA-after-loop-unrolling.ll
@@ -22,53 +22,21 @@
 %"struct.std::array" = type { [6 x i32] }
 
 define dso_local void @_Z3fooi(i32 %cnt) {
-; OLDPM-LABEL: @_Z3fooi(
-; OLDPM-NEXT:  entry:
-; OLDPM-NEXT:[[ARR:%.*]] = alloca %"struct.std::array", align 16
-; OLDPM-NEXT:[[TMP0:%.*]] = bitcast %"struct.std::array"* [[ARR]] to i8*
-; OLDPM-NEXT:call void @llvm.lifetime.start.p0i8(i64 24, i8* nonnull [[TMP0]])
-; OLDPM-NEXT:[[ARRAYDECAY_I_I_I:%.*]] = getelementptr inbounds %"struct.std::array", %"struct.std::array"* [[ARR]], i64 0, i32 0, i64 0
-; OLDPM-NEXT:[[INCDEC_PTR:%.*]] = getelementptr inbounds %"struct.std::array", %"struct.std::array"* [[ARR]], i64 0, i32 0, i64 1
-; OLDPM-NEXT:[[INCDEC_PTR_1:%.*]] = getelementptr inbounds %"struct.std::array", %"struct.std::array"* [[ARR]], i64 0, i32 0, i64 2
-; OLDPM-NEXT:[[INCDEC_PTR_2:%.*]] = getelementptr inbounds %"struct.std::array", %"struct.std::array"* [[ARR]], i64 0, i32 0, i64 3
-; OLDPM-NEXT:[[TMP1:%.*]] = insertelement <4 x i32> undef, i32 [[CNT:%.*]], i32 0
-; OLDPM-NEXT:[[TMP2:%.*]] = shufflevector <4 x i32> [[TMP1]], <4 x i32> undef, <4 x i32> zeroinitializer
-; OLDPM-NEXT:[[TMP3:%.*]] = add nsw <4 x i32> [[TMP2]], 
-; OLDPM-NEXT:[[TMP4:%.*]] = bitcast %"struct.std::array"* [[ARR]] to <4 x i32>*
-; OLDPM-NEXT:store <4 x i32> [[TMP3]], <4 x i32>* [[TMP4]], align 16
-; OLDPM-NEXT:[[INCDEC_PTR_3:%.*]] = getelementptr inbounds %"struct.std::array", %"struct.std::array"* [[ARR]], i64 0, i32 0, i64 4
-; OLDPM-NEXT:[[INC_4:%.*]] = add nsw i32 [[CNT]], 5
-; OLDPM-NEXT:store i32 [[INC_4]], i32* [[INCDEC_PTR_3]], align 16
-; OLDPM-NEXT:[[INC_5:%.*]] = add nsw i32 [[CNT]], 6
-; OLDPM-NEXT:[[TMP5:%.*]] = load i32, i32* [[ARRAYDECAY_I_I_I]], align 16
-; OLDPM-NEXT:call void @_Z3usei(i32 [[TMP5]])
-; OLDPM-NEXT:[[TMP6:%.*]] = load i32, i32* [[INCDEC_PTR]], align 4
-; OLDPM-NEXT:call void @_Z3usei(i32 [[TMP6]])
-; OLDPM-NEXT:[[TMP7:%.*]] = load i32, i32* [[INCDEC_PTR_1]], align 8
-; OLDPM-NEXT:call void @_Z3usei(i32 [[TMP7]])
-; OLDPM-NEXT:[[TMP8:%.*]] = load i32, i32* [[INCDEC_PTR_2]], align 4
-; OLDPM-NEXT:call void @_Z3usei(i32 [[TMP8]])
-; OLDPM-NEXT:[[TMP9:%.*]] = load i32, i32* [[INCDEC_PTR_3]], align 16
-; OLDPM-NEXT:call void @_Z3usei(i32 [[TMP9]])
-; OLDPM-NEXT:call void @_Z3usei(i32 [[INC_5]])
-; OLDPM-NEXT:call void @llvm.lifetime.end.p0i8(i64 24, i8* nonnull [[TMP0]])
-; OLDPM-NEXT:ret void
-;
-; NEWPM-LABEL: @_Z3fooi(
-; NEWPM-NEXT:  entry:
-; NEWPM-NEXT:[[INC:%.*]] = add nsw i32 [[CNT:%.*]], 1
-; NEWPM-NEXT:[[INC_1:%.*]] = add nsw i32 [[CNT]], 2
-; NEWPM-NEXT:[[INC_2:%.*]] = add nsw i32 [[CNT]], 3
-; NEWPM-NEXT:[[INC_3:%.*]] = add nsw i32 [[CNT]], 4
-; NEWPM-NEXT:[[INC_4:%.*]] = add nsw i32 [[CNT]], 5
-; NEWPM-NEXT:[[INC_5:%.*]] = add nsw i32 [[CNT]], 6
-; NEWPM-NEXT:call void @_Z3usei(i32 [[INC]])
-; NEWPM-NEXT:call void @_Z3usei(i32 [[INC_1]])
-; NEWPM-NEXT:call void @_Z3usei(i32 [[INC_2]])
-; NEWPM-NEXT:call void @_Z3usei(i32 [[INC_3]])
-; NEWPM-NEXT:call void @_Z3usei(i32 [[INC_4]])
-; NEWPM-NEXT:call void @_Z3usei(i32 [[INC_5]])
-; NEWPM-NEXT:ret void
+; CHECK-LABEL: @_Z3fooi(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[INC:%.*]] = add nsw i32 [[CNT:%.*]], 1
+; CHECK-NEXT:[[INC_1:%.*]] = add nsw i32 [[CNT]], 2
+; CHECK-NEXT:[[INC_2:%.*]] = add nsw i32 [[CNT]], 3
+; CHECK-NEXT:[[INC_3:%.*]] = add nsw i32 [[CNT]], 4
+; CHECK-NEXT:[[INC_4:%.*]] = add nsw i32 [[CNT]], 5
+; CHECK-NEXT:[[INC_5:%.*]] = add nsw i32 [[CNT]], 6
+; CHECK-NEXT:call void @_Z3usei(i32 [[INC]])
+; CHECK-NEXT:call void @_Z3usei(i32 [[INC_1]])
+; CHECK-NEXT:call void @_Z3usei(i32 [[INC_2]])
+; CHECK-NEXT:call void @_Z3usei(i32 [[INC_3]])
+; CHECK-NEXT:call void @_Z3usei(i32 [[INC_4]])
+; CHECK-NEXT:call void @_Z3usei(i32

[PATCH] D78075: [WIP][Clang][OpenMP] Added support for nowait target in CodeGen

2020-09-19 Thread Shilei Tian via Phabricator via cfe-commits 
tianshilei1992 updated this revision to Diff 292979.
tianshilei1992 added a comment.

Fixed the case `target_parallel_for_codegen.cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78075

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  clang/test/OpenMP/target_codegen.cpp
  clang/test/OpenMP/target_parallel_codegen.cpp
  clang/test/OpenMP/target_parallel_for_codegen.cpp

Index: clang/test/OpenMP/target_parallel_for_codegen.cpp
===
--- clang/test/OpenMP/target_parallel_for_codegen.cpp
+++ clang/test/OpenMP/target_parallel_for_codegen.cpp
@@ -74,7 +74,11 @@
 #ifndef HEADER
 #define HEADER
 
-// CHECK-DAG: %struct.ident_t = type { i32, i32, i32, i32, i8* }
+// CHECK-DAG: [[IDENT_T:%.+]] = type { i32, i32, i32, i32, i8* }
+// CHECK-DAG: [[ANON_T:%.+]] = type { i16, i32, i32 }
+// CHECK-DAG: [[KMP_TASK_T_WITH_PRIVATES:%.+]] = type { [[KMP_TASK_T:%.+]], [[KMP_PRIVATES_T:%.+]] }
+// CHECK-DAG: [[KMP_TASK_T]] = type { i8*, i32 (i32, i8*)*, i32, {{%[^,]+}}, {{%[^,]+}} }
+// CHECK-DAG: [[KMP_PRIVATES_T]] = type { [3 x i64], [3 x i8*], [3 x i8*], [3 x i8*], i16 }
 // CHECK-DAG: [[STR:@.+]] = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00"
 // CHECK-DAG: [[DEF_LOC:@.+]] = private unnamed_addr constant %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[STR]], i32 0, i32 0) }
 
@@ -158,34 +162,41 @@
 a += 1;
   }
 
-  // CHECK-DAG:   [[RET:%.+]] = call i32 @__tgt_target_teams_nowait_mapper(i64 -1, i8* @{{[^,]+}}, i32 3, i8** [[BP:%[^,]+]], i8** [[P:%[^,]+]], i64* getelementptr inbounds ([3 x i64], [3 x i64]* [[SIZET2]], i32 0, i32 0), i64* getelementptr inbounds ([3 x i64], [3 x i64]* [[MAPT2]], i32 0, i32 0), i8** null, i32 1, i32 0)
-  // CHECK-DAG:   [[BP]] = getelementptr inbounds [3 x i8*], [3 x i8*]* [[BPR:%[^,]+]], i32 0, i32 0
-  // CHECK-DAG:   [[P]] = getelementptr inbounds [3 x i8*], [3 x i8*]* [[PR:%[^,]+]], i32 0, i32 0
-  // CHECK-DAG:   [[BPADDR0:%.+]] = getelementptr inbounds [3 x i8*], [3 x i8*]* [[BPR]], i32 0, i32 0
-  // CHECK-DAG:   [[PADDR0:%.+]] = getelementptr inbounds [3 x i8*], [3 x i8*]* [[PR]], i32 0, i32 0
-  // CHECK-DAG:   [[CBPADDR0:%.+]] = bitcast i8** [[BPADDR0]] to i[[SZ]]*
-  // CHECK-DAG:   [[CPADDR0:%.+]] = bitcast i8** [[PADDR0]] to i[[SZ]]*
-  // CHECK-DAG:   store i[[SZ]] [[VAL0:%.+]], i[[SZ]]* [[CBPADDR0]],
-  // CHECK-DAG:   store i[[SZ]] [[VAL0]], i[[SZ]]* [[CPADDR0]],
-  // CHECK-DAG:   [[BPADDR1:%.+]] = getelementptr inbounds [3 x i8*], [3 x i8*]* [[BPR]], i32 0, i32 1
-  // CHECK-DAG:   [[PADDR1:%.+]] = getelementptr inbounds [3 x i8*], [3 x i8*]* [[PR]], i32 0, i32 1
-  // CHECK-DAG:   [[CBPADDR1:%.+]] = bitcast i8** [[BPADDR1]] to i[[SZ]]*
-  // CHECK-DAG:   [[CPADDR1:%.+]] = bitcast i8** [[PADDR1]] to i[[SZ]]*
-  // CHECK-DAG:   store i[[SZ]] [[VAL1:%.+]], i[[SZ]]* [[CBPADDR1]],
-  // CHECK-DAG:   store i[[SZ]] [[VAL1]], i[[SZ]]* [[CPADDR1]],
-  // CHECK-DAG:   [[BPADDR2:%.+]] = getelementptr inbounds [3 x i8*], [3 x i8*]* [[BPR]], i32 0, i32 1
-  // CHECK-DAG:   [[PADDR2:%.+]] = getelementptr inbounds [3 x i8*], [3 x i8*]* [[PR]], i32 0, i32 1
-  // CHECK-DAG:   [[CBPADDR2:%.+]] = bitcast i8** [[BPADDR2]] to i[[SZ]]*
-  // CHECK-DAG:   [[CPADDR2:%.+]] = bitcast i8** [[PADDR2]] to i[[SZ]]*
-  // CHECK-DAG:   store i[[SZ]] [[VAL2:%.+]], i[[SZ]]* [[CBPADDR2]],
-  // CHECK-DAG:   store i[[SZ]] [[VAL2]], i[[SZ]]* [[CPADDR2]],
-
-  // CHECK-NEXT:  [[ERROR:%.+]] = icmp ne i32 [[RET]], 0
-  // CHECK-NEXT:  br i1 [[ERROR]], label %[[FAIL:[^,]+]], label %[[END:[^,]+]]
-  // CHECK:   [[FAIL]]
-  // CHECK:   call void [[HVT2:@.+]](i[[SZ]] {{[^,]+}}, i[[SZ]] {{[^)]+}})
-  // CHECK-NEXT:  br label %[[END]]
-  // CHECK:   [[END]]
+  // CHECK-DAG:   call i32 @__kmpc_omp_task([[IDENT_T:%[^,]+]]* {{@[^,]+}}, i32 {{%[^,]+}}, i8* [[TASK:%[0-9]+]])
+  // CHECK-DAG:   [[TASK]] = call i8* @__kmpc_omp_target_task_alloc([[IDENT_T]]* {{@[^,]+}}, i32 {{%[^,]+}}, i32 1, i32 84, i32 12, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T_WITH_PRIVATES]]*)* [[OMP_TASK_ENTRY:@.+]] to i32 (i32, i8*)*), i64 -1)
+  // CHECK-DAG:   [[TASK_WITH_PRIVATES_CAST:%.+]] = bitcast i8* [[TASK]] to [[KMP_TASK_T_WITH_PRIVATES]]*
+  // CHECK-DAG:   [[KMP_PRIVATES:%.+]] = getelementptr inbounds [[KMP_TASK_T_WITH_PRIVATES]], [[KMP_TASK_T_WITH_PRIVATES]]* [[TASK_WITH_PRIVATES_CAST]], i32 0, i32 1
+  // CHECK-DAG:   [[FPSIZEGEP:%.+]] = getelementptr inbounds [[KMP_PRIVATES_T]], [[KMP_PRIVATES_T]]* [[KMP_PRIVATES]], i32 0, i32 0
+  // CHECK-DAG:   [[FPSIZECAST:%.+]] = bitcast [3 x i64]* [[FPSIZEGEP]] to i8*
+  // CHECK-DAG:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[FPSIZECAST]], i8* align 4 bitcast ([3 x i64]* [[SIZET2]] to i8*), i32 24, i1 false)
+  // CHECK-DAG:   [[FPBPRGEP:%.+]] = getelementptr inbounds [[KMP_PRIVATES_T]],

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2020-09-19 Thread Douglas Chen via Phabricator via cfe-commits 
dougpuob added a comment.

Replied comments by @aaron.ballman




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:216
+{"long","l"},
+{"long long",   "ll"},
+{"unsigned long",   "ul"},

aaron.ballman wrote:
> `unsigned long long` -> `ull`
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:225
+{"WORD","w"},
+{"DWORD",   "dw"}};
+  // clang-format on

aaron.ballman wrote:
> `ULONG` -> `ul`
> `HANDLE` -> `h`
> 
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:236
+  PrefixStr = "fn"; // Function Pointer
+} else if (QT->isPointerType()) {
+  // clang-format off

aaron.ballman wrote:
> I'm not certain how valid it is to look at just the type and decide that it's 
> a null-terminated string. For instance, the following is not an uncommon 
> pattern: `void something(const char *buffer, size_t length);` and it would be 
> a bit strange to change that into `szBuffer` as that would give an indication 
> that the buffer is null terminated. You could look at surrounding code for 
> additional information though, like other parameters in a function 
> declaration. As an aside, this sort of heuristic search may also allow you to 
> change `length` into `cbLength` instead of `nLength` for conventions like the 
> Microsoft one.
> 
> However, for local variables, I don't see how you could even come up with a 
> heuristic like you could with parameters, so I'm not certain what the right 
> answer is here.
OK (size_t nLength --> cbLength)

About the `void something(const char *buffer, size_t length)` pattern. `char` 
is a special type which can express signed char and unsigned char.  One can 
express C string(ASCII), another expresses general data buffer(in hex). I think 
you are mentioning when it is a general data buffer. I agree with you if it 
changed the name from `buffer` to `szBuffer` for general data buffer is 
strange. I prefer to use `uint8_t` or `unsigned char` instead.

How about adding a new config option for it? like, `  - { key: 
readability-identifier-naming.HungarainNotation.DontChangeCharPointer   , 
value: 1 }` Users can make decision for their projects. For consistency with 
HN, the default will still be changed to `szBuffer` in your case.

If I add the option, does it make sense to you from your side?



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:309
+
+  if (PtrCount > 0) {
+for (size_t Idx = 0; Idx < PtrCount; Idx++) {

aaron.ballman wrote:
> No need for this `if` statement, the `for` loop won't run anyway if `PtrCount 
> == 0`.
OK! redundant code.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:319
+std::string
+IdentifierNamingCheck::getDeclTypeName(const clang::NamedDecl *Decl) const {
+  const ValueDecl *ValDecl = dyn_cast(Decl);

aaron.ballman wrote:
> `ND` instead of `Decl`.
> 
> The function name doesn't really help me to understand why you'd call this as 
> opposed to getting the type information as a string from the `NamedDecl` 
> itself. I'm a bit worried about maintaining this code as the language evolves 
> -- Clang will get new keywords, and someone will have to remember to come 
> update this code. Could you get away with using 
> `Decl->getType()->getAsString()` and working with that rather than going back 
> to the original source code and trying to parse manually?
OK, I should do it. (ND instead of Decl.)

Use `Decl->getType()->getAsString()` is a good way. But HN is a strongly-typed 
notation, if users haven't specific the include directories, the checking 
result may look messy (it will be changed to unexpected type). So I parse a 
string with `SourceLocation`, just for user-friendly.

I understand your consideration, maintaining-friendly is also important. I can 
implement it with `Decl->getType()->getAsString()`, if my explanation can't 
convince you still. It is also fine to me. Or you think it just need a better 
appropriate function name?



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:320
+IdentifierNamingCheck::getDeclTypeName(const clang::NamedDecl *Decl) const {
+  const ValueDecl *ValDecl = dyn_cast(Decl);
+  if (!ValDecl) {

aaron.ballman wrote:
> `const auto *` since the type is spelled out in the initialization.
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:554
+  case IdentifierNamingCheck::CT_HungarianNotation: {
+const NamedDecl *ND = dyn_cast(InputDecl);
+const std::string TypePrefix =

aaron.ballman wrote:
> `const auto *` because the type is spelled out in the initialization.
OK.

[PATCH] D87837: [Driver] Add disabled-by-default -Wuse-ld-non-word for the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay updated this revision to Diff 292973.
MaskRay retitled this revision from "[Driver] Remove the deprecation warning 
for -fuse-ld=/abs/path" to "[Driver] Add disabled-by-default -Wuse-ld-non-word 
for the deprecation warning for -fuse-ld=/abs/path".
MaskRay edited the summary of this revision.
MaskRay added a comment.

Add a warning flag


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87837

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/test/Driver/fuse-ld.c


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -1,10 +1,18 @@
-// RUN: %clang %s -### \
-// RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
-// RUN: -target x86_64-unknown-linux \
-// RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+/// The absolute path warning is enabled by -Wuse-ld-non-word and -Wextra.
+// RUN: %clang %s -### -target x86_64-unknown-linux -Wuse-ld-non-word \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
 // CHECK-ABSOLUTE-LD: warning: '-fuse-ld=' taking a path is deprecated. Use 
'--ld-path=' instead
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 
+// RUN: %clang %s -### -target x86_64-unknown-linux -Wextra \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+
+// RUN: %clang %s -### -target x86_64-unknown-linux \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
+// CHECK-NO-WARN-NOT: warning:
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -869,6 +869,8 @@
 def PointerToIntCast : DiagGroup<"pointer-to-int-cast",
  [PointerToEnumCast, VoidPointerToIntCast]>;
 
+def UseLdNonWord : DiagGroup<"use-ld-non-word">;
+
 def Move : DiagGroup<"move", [
 PessimizingMove,
 RedundantMove,
@@ -887,7 +889,8 @@
 UnusedParameter,
 NullPointerArithmetic,
 EmptyInitStatement,
-StringConcatation
+StringConcatation,
+UseLdNonWord,
   ]>;
 
 def Most : DiagGroup<"most", [
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -464,7 +464,8 @@
   InGroup>;
 
 def warn_drv_use_ld_non_word : Warning<
-  "'-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead">;
+  "'-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead">,
+  InGroup, DefaultIgnore;
 
 def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
   "option '-ffine-grained-bitfield-accesses' cannot be enabled together with a 
sanitizer; flag ignored">,


Index: clang/test/Driver/fuse-ld.c
===
--- clang/test/Driver/fuse-ld.c
+++ clang/test/Driver/fuse-ld.c
@@ -1,10 +1,18 @@
-// RUN: %clang %s -### \
-// RUN: -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 \
-// RUN: -target x86_64-unknown-linux \
-// RUN:   | FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+/// The absolute path warning is enabled by -Wuse-ld-non-word and -Wextra.
+// RUN: %clang %s -### -target x86_64-unknown-linux -Wuse-ld-non-word \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
 // CHECK-ABSOLUTE-LD: warning: '-fuse-ld=' taking a path is deprecated. Use '--ld-path=' instead
 // CHECK-ABSOLUTE-LD: /usr/local/bin/or1k-linux-ld
 
+// RUN: %clang %s -### -target x86_64-unknown-linux -Wextra \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-ABSOLUTE-LD
+
+// RUN: %clang %s -### -target x86_64-unknown-linux \
+// RUN:   -fuse-ld=/usr/local/bin/or1k-linux-ld 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-NO-WARN
+// CHECK-NO-WARN-NOT: warning:
 
 // RUN: %clang %s -### \
 // RUN: -target x86_64-unknown-freebsd 2>&1 \
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -869,6 +869,8 @@
 def PointerToIntCast : DiagGroup<"pointer-to-int-cast",
  [PointerToEnumCast, VoidPointerToIntCast]>;
 
+def UseLdNonWord : DiagGroup<"use-ld-non-word">;
+
 def Move : DiagGroup<"move", [
 PessimizingMove,
 RedundantMove,
@@ -887,7

[PATCH] D83296: [clang-format] Add a MacroExpander.

2020-09-19 Thread Manuel Klimek via Phabricator via cfe-commits 
klimek updated this revision to Diff 292970.
klimek marked 7 inline comments as done.
klimek added a comment.

Worked in review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83296

Files:
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/FormatToken.h
  clang/lib/Format/MacroExpander.cpp
  clang/lib/Format/Macros.h
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/MacroExpanderTest.cpp
  clang/unittests/Format/TestLexer.h

Index: clang/unittests/Format/TestLexer.h
===
--- /dev/null
+++ clang/unittests/Format/TestLexer.h
@@ -0,0 +1,88 @@
+//===--- TestLexer.h - Format C++ code --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+///
+/// \file
+/// This file contains a TestLexer to create FormatTokens from strings.
+///
+//===--===//
+
+#ifndef CLANG_UNITTESTS_FORMAT_TEST_LEXER_H
+#define CLANG_UNITTESTS_FORMAT_TEST_LEXER_H
+
+#include "../../lib/Format/FormatTokenLexer.h"
+
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/SourceManager.h"
+
+#include 
+#include 
+
+namespace clang {
+namespace format {
+
+typedef llvm::SmallVector TokenList;
+
+inline std::ostream <<(std::ostream , const FormatToken ) {
+  Stream << "(" << Tok.Tok.getName() << ", \"" << Tok.TokenText.str() << "\")";
+  return Stream;
+}
+inline std::ostream <<(std::ostream , const TokenList ) {
+  Stream << "{";
+  for (size_t I = 0, E = Tokens.size(); I != E; ++I) {
+Stream << (I > 0 ? ", " : "") << *Tokens[I];
+  }
+  Stream << "}";
+  return Stream;
+}
+
+inline TokenList uneof(const TokenList ) {
+  assert(!Tokens.empty() && Tokens.back()->is(tok::eof));
+  return TokenList(Tokens.begin(), std::prev(Tokens.end()));
+}
+
+inline std::string text(llvm::ArrayRef Tokens) {
+  return std::accumulate(Tokens.begin(), Tokens.end(), std::string(),
+ [](const std::string , FormatToken *Tok) {
+   return (R + Tok->TokenText).str();
+ });
+}
+
+class TestLexer {
+public:
+  TestLexer() : SourceMgr("test.cpp", "") {}
+
+  TokenList lex(llvm::StringRef Code) {
+Buffers.push_back(
+llvm::MemoryBuffer::getMemBufferCopy(Code, ""));
+clang::FileID FID = SourceMgr.get().createFileID(SourceManager::Unowned,
+ Buffers.back().get());
+FormatTokenLexer Lex(SourceMgr.get(), FID, 0, Style, Encoding, Allocator,
+ IdentTable);
+auto Result = Lex.lex();
+return TokenList(Result.begin(), Result.end());
+  }
+
+  FormatToken *id(llvm::StringRef Code) {
+auto Result = uneof(lex(Code));
+assert(Result.size() == 1U && "Code must expand to 1 token.");
+return Result[0];
+  }
+
+  FormatStyle Style = getLLVMStyle();
+  encoding::Encoding Encoding = encoding::Encoding_UTF8;
+  std::vector> Buffers;
+  clang::SourceManagerForFile SourceMgr;
+  llvm::SpecificBumpPtrAllocator Allocator;
+  IdentifierTable IdentTable;
+};
+
+} // namespace format
+} // namespace clang
+
+#endif // LLVM_CLANG_UNITTESTS_FORMAT_TEST_LEXER_H
Index: clang/unittests/Format/MacroExpanderTest.cpp
===
--- /dev/null
+++ clang/unittests/Format/MacroExpanderTest.cpp
@@ -0,0 +1,187 @@
+#include "../../lib/Format/Macros.h"
+#include "TestLexer.h"
+#include "clang/Basic/FileManager.h"
+
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace format {
+
+namespace {
+
+class MacroExpanderTest : public ::testing::Test {
+public:
+  std::unique_ptr
+  create(const std::vector ) {
+return std::make_unique(MacroDefinitions,
+   Lex.SourceMgr.get(), Lex.Style,
+   Lex.Allocator, Lex.IdentTable);
+  }
+
+  std::string expand(MacroExpander , llvm::StringRef Name,
+ const std::vector  = {}) {
+EXPECT_TRUE(Macros.defined(Name));
+return text(Macros.expand(Lex.id(Name), lexArgs(Args)));
+  }
+
+  llvm::SmallVector
+  lexArgs(const std::vector ) {
+llvm::SmallVector Result;
+for (const auto  : Args) {
+  Result.push_back(uneof(Lex.lex(Arg)));
+}
+return Result;
+  }
+
+  struct MacroAttributes {
+clang::tok::TokenKind Kind;
+MacroRole Role;
+unsigned Start;
+unsigned End;
+llvm::SmallVector ExpandedFrom;
+  };
+
+  void expectAttributes(const TokenList ,
+const std::vector ,
+const std::string , unsigned Line) {
+EXPECT_EQ(Tokens.size(), Attributes.size())

[PATCH] D83296: [clang-format] Add a MacroExpander.

2020-09-19 Thread Manuel Klimek via Phabricator via cfe-commits 
klimek added inline comments.



Comment at: clang/lib/Format/FormatToken.h:177
+/// EndOfExpansion:   0  0 0   21 0 1
+struct MacroContext {
+  MacroContext(MacroRole Role) : Role(Role) {}

sammccall wrote:
> "context" is often pretty vague - "MacroSource" isn't a brilliant name but at 
> least seems to hint at the direction (that the FormatToken is the expanded 
> token and the MacroSource gives information about what it was expanded from)
> 
> I don't feel strongly about this though, up to you.
MacroSource sounds like it is about the macro source (i.e. the tokens written 
for the macro).
I'd be happy to rename to MacroExpansion or MacroExpansionInfo or somesuch if 
you think that helps?



Comment at: clang/lib/Format/MacroExpander.cpp:81
+  nextToken();
+}
+if (Current->isNot(tok::r_paren))

sammccall wrote:
> this accepts `FOO(A,B,)=...` as equivalent to `FOO(A,B)=...`. Not sure if 
> worth fixing.
We're generally accepting too much; I'd either want to restrict it fully, or 
basically be somewhat minimum/forgiving. Given that we can't get errors back to 
the user, I was aiming for the latter.



Comment at: clang/lib/Format/MacroExpander.cpp:88
+
+  bool parseExpansion() {
+if (!Current->isOneOf(tok::equal, tok::eof))

sammccall wrote:
> (nit: I'd probably find this easier to follow as `if (equal) else if (eof) 
> else` with parseTail inlined, but up to you)
I basically like having the implementation match the BNR.
That said, not feeling strongly about it. You're saying you'd duplicate the 
Def.Body.push_back in the if (eof)?

  if (Current->is(tok::equal) {
nextToken();
// inline parseTail
  } else if (Current->is(tok::eof) {
Def.Body.push_back(Current);
  } else {
return false;
  }

Generally, I personally find it easier to read the early exit.



Comment at: clang/lib/Format/MacroExpander.cpp:142
+  auto Definition = Parser.parse();
+  Definitions[Definition.Name] = Definition;
+}

sammccall wrote:
> nit: this is a copy for what seems like no reason - move `Parser.parse()` 
> inline to this line?
Reason is that we need the name.



Comment at: clang/lib/Format/MacroExpander.cpp:186
+Tok->MacroCtx = MacroContext(MR_ExpandedArg);
+  pushToken(Tok);
+}

klimek wrote:
> sammccall wrote:
> > klimek wrote:
> > > sammccall wrote:
> > > > you're pushing here without copying. This means the original tokens 
> > > > from the ArgsList are mutated. Maybe we own them, but this seems at 
> > > > least wrong for multiple expansion of the same arg.
> > > > 
> > > > e.g.
> > > > ```
> > > > #define M(X,Y) X Y X
> > > > M(1,2)
> > > > ```
> > > > 
> > > > Will expand to:
> > > > - 1, ExpandedArg, ExpandedFrom = [M, M] // should just be one M
> > > > - 2, ExpandedArg, ExpandedFrom = [M]
> > > > - 1, ExpandedArg, ExpandedFrom = [M, M] // this is the same token 
> > > > pointer as the first one
> > > > 
> > > > Maybe it would be better if pushToken performed the copy, and returned 
> > > > a mutable pointer to the copy. (If you can make the input const, that 
> > > > would prevent this type of bug)
> > > Ugh. I'll need to take a deeper look, but generally, the problem is we 
> > > don't want to copy - we're mutating the data of the token while 
> > > formatting the expanded token stream, and then re-use that info when 
> > > formatting the original stream.
> > > We could copy, add a reference to the original token, and then have a 
> > > step that adapts in the end, and perhaps that's cleaner overall anyway, 
> > > but will be quite a change.
> > > The alternative is that I'll look into how to specifically handle 
> > > double-expansion (or ... forbid it).
> > > (or ... forbid it).
> > 
> > I'm starting to think this is the best option.
> > 
> > The downsides seem pretty acceptable to me:
> >  - it's another wart to document: on the other hand it simplifies the 
> > conceptual model, I think it helps users understand the deeper behavior
> >  - some macros require simplification rather than supplying the actual 
> > definition: already crossed this bridge by not supporting macros in macro 
> > bodies, variadics, pasting...
> >  - loses information: one expansion is enough to establish which part of 
> > the grammar the arguments form in realistic cases. (Even in pathological 
> > cases, preserving the conflicting info only helps you if you have a plan to 
> > resolve the conflicts)
> >  - it's another wart to document: 
> > 
> > Are there any others?
> > 
> My main concern is that it's probably the most surprising feature to not 
> support.
Forbade multi-expansion.



Comment at: clang/lib/Format/Macros.h:82
+///
+/// Furthermore, expansion and definition are independent - a macro defined
+/// as "A()=a" and "A=a" can both be expanded  as "A()" and "A".

[PATCH] D87561: [Sema] List conversion validate character array

2020-09-19 Thread Mark de Wever via Phabricator via cfe-commits 
Mordante added inline comments.



Comment at: clang/test/CXX/drs/dr14xx.cpp:411-414
+  void f(const char[4]);
+  void f(const wchar_t[4]);
+  void f(const char16_t[4]);
+  void f(const char32_t[4]);

rsmith wrote:
> These should presumably be references to arrays, rather than arrays, or the 
> parameter type is as if you wrote (for example) `void f(const char *)`, which 
> shouldn't get the special treatment here.
> 
> [over.ics.list]p4 mentions this in its footnote:
> 
> "Otherwise, if the parameter type is a character array [Footnote: Since there 
> are no parameters of array type, this will only occur as the referenced type 
> of a reference parameter.] and the initializer list has a single element that 
> is an appropriately-typed string-literal (9.4.3), the implicit conversion 
> sequence is the identity conversion."
Ah I missed that footnote. I reread the standard and can you confirm some cases?
```
namespace A { 
  void f(const char(&)[4]);
  void g() { f({"abc"}); }
}
namespace B { 
  void f(const char(&&)[4]);
  void g() { f({"abc"}); }
} 
```
both should work and with P0388 the array without bounds will also work.

I ask since this is my interpretation of the standard, but it seems there's a 
difference between implementations and `void f(const char(&&)[4]);` fails for 
"abc" but works for "ab".
It seems ICC and consider "abc" an lvalue in this case and not when using "ab".

Here's a gotbolt link with the examples https://godbolt.org/z/r1TKfx


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87561

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

[clang] 2819cea - Revert "[HIP] Fix -gsplit-dwarf option"

2020-09-19 Thread Yaxun Liu via cfe-commits 

Author: Yaxun (Sam) Liu
Date: 2020-09-19T10:15:27-04:00
New Revision: 2819cea2ef8aab9d7ef8ba29feec9cb58cc942f6

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

LOG: Revert "[HIP] Fix -gsplit-dwarf option"

This reverts commit e50465ecefc964e5700df26fc7e02a673eed085a
due to regression in lldb tests.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/CommonArgs.h
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/MinGW.cpp

Removed: 
clang/test/Driver/hip-gsplit-dwarf-options.hip



diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 1b7476d8ffd2..0c03a90b8566 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4810,7 +4810,7 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 (isa(JA) || isa(JA) ||
  isa(JA));
   if (SplitDWARF) {
-const char *SplitDWARFOut = SplitDebugName(JA, Args, Input, Output);
+const char *SplitDWARFOut = SplitDebugName(Args, Input, Output);
 CmdArgs.push_back("-split-dwarf-file");
 CmdArgs.push_back(SplitDWARFOut);
 if (DwarfFission == DwarfFissionKind::Split) {
@@ -7047,7 +7047,7 @@ void ClangAs::ConstructJob(Compilation , const 
JobAction ,
   if (getDebugFissionKind(D, Args, A) == DwarfFissionKind::Split &&
   T.isOSBinFormatELF()) {
 CmdArgs.push_back("-split-dwarf-output");
-CmdArgs.push_back(SplitDebugName(JA, Args, Input, Output));
+CmdArgs.push_back(SplitDebugName(Args, Input, Output));
   }
 
   assert(Input.isFilename() && "Invalid input.");

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 151d5893961e..5dc5d834136e 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -902,23 +902,15 @@ bool tools::areOptimizationsEnabled(const ArgList ) {
   return false;
 }
 
-const char *tools::SplitDebugName(const JobAction , const ArgList ,
-  const InputInfo ,
+const char *tools::SplitDebugName(const ArgList , const InputInfo ,
   const InputInfo ) {
-  // Adds '_' and GPU arch to the stem of .dwo file for HIP, which is
-  // expected by gdb.
-  auto AddPostfix = [JA](auto ) {
-if (JA.getOffloadingDeviceKind() == Action::OFK_HIP)
-  F += (Twine("_") + JA.getOffloadingArch()).str();
-  };
   if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
 if (StringRef(A->getValue()) == "single")
   return Args.MakeArgString(Output.getFilename());
 
   Arg *FinalOutput = Args.getLastArg(options::OPT_o);
   if (FinalOutput && Args.hasArg(options::OPT_c)) {
-SmallString<128> T(llvm::sys::path::stem(FinalOutput->getValue()));
-AddPostfix(T);
+SmallString<128> T(FinalOutput->getValue());
 llvm::sys::path::replace_extension(T, "dwo");
 return Args.MakeArgString(T);
   } else {
@@ -926,7 +918,6 @@ const char *tools::SplitDebugName(const JobAction , 
const ArgList ,
 SmallString<128> T(
 Args.getLastArgValue(options::OPT_fdebug_compilation_dir));
 SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
-AddPostfix(F);
 llvm::sys::path::replace_extension(F, "dwo");
 T += F;
 return Args.MakeArgString(F);

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.h 
b/clang/lib/Driver/ToolChains/CommonArgs.h
index 4947c33b6224..0028ea0ca337 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.h
+++ b/clang/lib/Driver/ToolChains/CommonArgs.h
@@ -49,7 +49,7 @@ void AddRunTimeLibs(const ToolChain , const Driver ,
 llvm::opt::ArgStringList ,
 const llvm::opt::ArgList );
 
-const char *SplitDebugName(const JobAction , const llvm::opt::ArgList ,
+const char *SplitDebugName(const llvm::opt::ArgList ,
const InputInfo , const InputInfo );
 
 void SplitDebugInfo(const ToolChain , Compilation , const Tool ,

diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 68a75db0b92a..7f7a3956781a 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -939,7 +939,7 @@ void tools::gnutools::Assembler::ConstructJob(Compilation 
,
   if (Args.hasArg(options::OPT_gsplit_dwarf) &&
   getToolChain().getTriple().isOSLinux())
 SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
-   SplitDebugName(JA, Args, Inputs[0], Output));
+   SplitDebugName(Args, Inputs[0], Output));
 }
 
 namespace {

diff  --git a/clang/lib/Driver/ToolChains/MinGW.cpp

[clang] e50465e - [HIP] Fix -gsplit-dwarf option

2020-09-19 Thread Yaxun Liu via cfe-commits 

Author: Yaxun (Sam) Liu
Date: 2020-09-19T10:06:51-04:00
New Revision: e50465ecefc964e5700df26fc7e02a673eed085a

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

LOG: [HIP] Fix -gsplit-dwarf option

when -gsplit option is used with clang driver, clang driver will create
a filename with .dwo option based on the input file name and pass
it to clang -cc1. This file is used for storing the debug info. Since
HIP generate separate object files for different GPU arch's,
this file should be different for different GPU arch. This patch
adds _ and GPU arch to the stem of the dwo file.

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

Added: 
clang/test/Driver/hip-gsplit-dwarf-options.hip

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Driver/ToolChains/CommonArgs.h
clang/lib/Driver/ToolChains/Gnu.cpp
clang/lib/Driver/ToolChains/MinGW.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 0c03a90b8566..1b7476d8ffd2 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4810,7 +4810,7 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
 (isa(JA) || isa(JA) ||
  isa(JA));
   if (SplitDWARF) {
-const char *SplitDWARFOut = SplitDebugName(Args, Input, Output);
+const char *SplitDWARFOut = SplitDebugName(JA, Args, Input, Output);
 CmdArgs.push_back("-split-dwarf-file");
 CmdArgs.push_back(SplitDWARFOut);
 if (DwarfFission == DwarfFissionKind::Split) {
@@ -7047,7 +7047,7 @@ void ClangAs::ConstructJob(Compilation , const 
JobAction ,
   if (getDebugFissionKind(D, Args, A) == DwarfFissionKind::Split &&
   T.isOSBinFormatELF()) {
 CmdArgs.push_back("-split-dwarf-output");
-CmdArgs.push_back(SplitDebugName(Args, Input, Output));
+CmdArgs.push_back(SplitDebugName(JA, Args, Input, Output));
   }
 
   assert(Input.isFilename() && "Invalid input.");

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 5dc5d834136e..151d5893961e 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -902,15 +902,23 @@ bool tools::areOptimizationsEnabled(const ArgList ) {
   return false;
 }
 
-const char *tools::SplitDebugName(const ArgList , const InputInfo ,
+const char *tools::SplitDebugName(const JobAction , const ArgList ,
+  const InputInfo ,
   const InputInfo ) {
+  // Adds '_' and GPU arch to the stem of .dwo file for HIP, which is
+  // expected by gdb.
+  auto AddPostfix = [JA](auto ) {
+if (JA.getOffloadingDeviceKind() == Action::OFK_HIP)
+  F += (Twine("_") + JA.getOffloadingArch()).str();
+  };
   if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
 if (StringRef(A->getValue()) == "single")
   return Args.MakeArgString(Output.getFilename());
 
   Arg *FinalOutput = Args.getLastArg(options::OPT_o);
   if (FinalOutput && Args.hasArg(options::OPT_c)) {
-SmallString<128> T(FinalOutput->getValue());
+SmallString<128> T(llvm::sys::path::stem(FinalOutput->getValue()));
+AddPostfix(T);
 llvm::sys::path::replace_extension(T, "dwo");
 return Args.MakeArgString(T);
   } else {
@@ -918,6 +926,7 @@ const char *tools::SplitDebugName(const ArgList , 
const InputInfo ,
 SmallString<128> T(
 Args.getLastArgValue(options::OPT_fdebug_compilation_dir));
 SmallString<128> F(llvm::sys::path::stem(Input.getBaseInput()));
+AddPostfix(F);
 llvm::sys::path::replace_extension(F, "dwo");
 T += F;
 return Args.MakeArgString(F);

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.h 
b/clang/lib/Driver/ToolChains/CommonArgs.h
index 0028ea0ca337..4947c33b6224 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.h
+++ b/clang/lib/Driver/ToolChains/CommonArgs.h
@@ -49,7 +49,7 @@ void AddRunTimeLibs(const ToolChain , const Driver ,
 llvm::opt::ArgStringList ,
 const llvm::opt::ArgList );
 
-const char *SplitDebugName(const llvm::opt::ArgList ,
+const char *SplitDebugName(const JobAction , const llvm::opt::ArgList ,
const InputInfo , const InputInfo );
 
 void SplitDebugInfo(const ToolChain , Compilation , const Tool ,

diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 7f7a3956781a..68a75db0b92a 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -939,7 +939,7 @@ void tools::gnutools::Assembler::ConstructJob(Compilation 
,
   if (Args.hasArg(options::OPT_gsplit_dwarf) &&

[PATCH] D87791: [CUDA][HIP] Fix -gsplit-dwarf option

2020-09-19 Thread Yaxun Liu via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rGe50465ecefc9: [HIP] Fix -gsplit-dwarf option (authored by 
yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D87791?vs=292326=292957#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87791

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/hip-gsplit-dwarf-options.hip

Index: clang/test/Driver/hip-gsplit-dwarf-options.hip
===
--- /dev/null
+++ clang/test/Driver/hip-gsplit-dwarf-options.hip
@@ -0,0 +1,25 @@
+// REQUIRES: zlib, clang-driver, amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -c \
+// RUN:   --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -c \
+// RUN:   -fgpu-rdc --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu \
+// RUN:   -fgpu-rdc --offload-arch=gfx906:xnack+ %s -nogpulib -nogpuinc \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   -ggdb -gsplit-dwarf 2>&1 | FileCheck %s
+
+// CHECK-DAG: {{".*clang.*".* "-target-cpu" "gfx906".* "-split-dwarf-output" "hip-gsplit-dwarf-options_gfx906:xnack\+.dwo"}}
+// CHECK-DAG: {{".*clang.*".* "-target-cpu" "gfx900".* "-split-dwarf-output" "hip-gsplit-dwarf-options_gfx900.dwo"}}
+// CHECK-DAG: {{".*clang.*".* "-target-cpu" "x86-64".* "-split-dwarf-output" "hip-gsplit-dwarf-options.dwo"}}
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -55,7 +55,7 @@
 
   if (Args.hasArg(options::OPT_gsplit_dwarf))
 SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
-   SplitDebugName(Args, Inputs[0], Output));
+   SplitDebugName(JA, Args, Inputs[0], Output));
 }
 
 void tools::MinGW::Linker::AddLibGCC(const ArgList ,
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -939,7 +939,7 @@
   if (Args.hasArg(options::OPT_gsplit_dwarf) &&
   getToolChain().getTriple().isOSLinux())
 SplitDebugInfo(getToolChain(), C, *this, JA, Args, Output,
-   SplitDebugName(Args, Inputs[0], Output));
+   SplitDebugName(JA, Args, Inputs[0], Output));
 }
 
 namespace {
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -49,7 +49,7 @@
 llvm::opt::ArgStringList ,
 const llvm::opt::ArgList );
 
-const char *SplitDebugName(const llvm::opt::ArgList ,
+const char *SplitDebugName(const JobAction , const llvm::opt::ArgList ,
const InputInfo , const InputInfo );
 
 void SplitDebugInfo(const ToolChain , Compilation , const Tool ,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -902,15 +902,23 @@
   return false;
 }
 
-const char *tools::SplitDebugName(const ArgList , const InputInfo ,
+const char *tools::SplitDebugName(const JobAction , const ArgList ,
+  const InputInfo ,
   const InputInfo ) {
+  // Adds '_' and GPU arch to the stem of .dwo file for HIP, which is
+  // expected by gdb.
+  auto AddPostfix = [JA](auto ) {
+if (JA.getOffloadingDeviceKind() == Action::OFK_HIP)
+  F += (Twine("_") + JA.getOffloadingArch()).str();
+  };
   if (Arg *A = Args.getLastArg(options::OPT_gsplit_dwarf_EQ))
 if (StringRef(A->getValue()) == "single")
   return Args.MakeArgString(Output.getFilename());
 
   Arg *FinalOutput = Args.getLastArg(options::OPT_o);
   if (FinalOutput && Args.hasArg(options::OPT_c)) {
-SmallString<128> T(FinalOutput->getValue());
+SmallString<128> T(llvm::sys::path::stem(FinalOutput->getValue()));
+AddPostfix(T);

[PATCH] D87942: [Analyzer] GNU named variadic macros in Plister

2020-09-19 Thread Chris Hamilton via Phabricator via cfe-commits 
chrish_ericsson_atx added inline comments.



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:536
+  do {   \
+  } while (0)
+

Seems the do {} while (0) idiom is a problem, because the plist output gets 
formatted differently on windows than it does on Linux.   (Extra space).  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87942

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

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

2020-09-19 Thread Chuyang Chen via Phabricator via cfe-commits 
nomanous created this revision.
nomanous added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
nomanous requested review of this revision.

The multi-character character constants should be implementation-defined 
according to the C standard but actually were made to be extension, which would 
cause errors when using clang with option "-pedantic-errors". This patch  fixes 
it and it is for bug #46797 on the Bugzilla system.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87962

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/test/CXX/lex/lex.literal/lex.ccon/p1.cpp
  clang/test/FixIt/format.m
  clang/test/Lexer/multi-character-character-constant.c
  clang/test/Preprocessor/expr_multichar.c

Index: clang/test/Preprocessor/expr_multichar.c
===
--- clang/test/Preprocessor/expr_multichar.c
+++ clang/test/Preprocessor/expr_multichar.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 < %s -E -verify -triple i686-pc-linux-gnu
-// expected-no-diagnostics
+// Expect the warning of the four-character character constant after changing it from extension to implementation-defined
 
-#if (('1234' >> 24) != '1')
+#if (('1234' >> 24) != '1') // expected-warning {{multi-character character constant}}
 #error Bad multichar constant calculation!
 #endif
Index: clang/test/Lexer/multi-character-character-constant.c
===
--- /dev/null
+++ clang/test/Lexer/multi-character-character-constant.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -pedantic-errors %s
+
+int x = 'ab'; // expected-warning {{multi-character character constant}}
+int y = 'abcd'; // expected-warning {{multi-character character constant}}
+
+int main() {
+   return 0;
+}
+
Index: clang/test/FixIt/format.m
===
--- clang/test/FixIt/format.m
+++ clang/test/FixIt/format.m
@@ -161,14 +161,17 @@
   // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
 
 
-  NSLog(@"%s", 'abcd'); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+  NSLog(@"%s", 'abcd'); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}} \
+  // expected-warning {{multi-character character constant}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:13}:"%d"
 
-  NSLog(@"%lf", 'abcd'); // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:14}:"%d"
+  NSLog(@"%lf", 'abcd'); // expected-warning{{format specifies type 'double' but the argument has type 'int'}} \
+  // expected-warning {{multi-character character constant}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:14}:"%d"
 
-  NSLog(@"%@", 'abcd'); // expected-warning{{format specifies type 'id' but the argument has type 'int'}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+  NSLog(@"%@", 'abcd'); // expected-warning{{format specifies type 'id' but the argument has type 'int'}} \
+  // expected-warning {{multi-character character constant}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:13}:"%d"
 }
 
 void multichar_constants_false_negative() {
@@ -177,8 +180,9 @@
   // type-checker expects %c to correspond to an integer argument, because
   // many C library functions like fgetc() actually return an int (using -1
   // as a sentinel).
-  NSLog(@"%c", 'abcd'); // missing-warning{{format specifies type 'char' but the argument has type 'int'}}
-  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+  NSLog(@"%c", 'abcd'); // missing-warning{{format specifies type 'char' but the argument has type 'int'}} \
+  // expected-warning {{multi-character character constant}}
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:13}:"%d"
 }
 
 
Index: clang/test/CXX/lex/lex.literal/lex.ccon/p1.cpp
===
--- clang/test/CXX/lex/lex.literal/lex.ccon/p1.cpp
+++ clang/test/CXX/lex/lex.literal/lex.ccon/p1.cpp
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
-// expected-no-diagnostics
+// Expect the warning of the four-character character constant after changing it from extension to implementation-defined
 
 // Check types of char literals
 extern char a;
 extern __typeof('a') a;
 extern int b;
-extern __typeof('asdf') b;
+extern __typeof('asdf') b; // expected-warning {{multi-character character constant}}
 extern wchar_t c;
 extern __typeof(L'a') c;
 #if __cplusplus >= 201103L
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++

[clang-tools-extra] 985deba - Revert "Temporarily Revert "[clangd] Add Random Forest runtime for code completion.""

2020-09-19 Thread Utkarsh Saxena via cfe-commits 

Author: Utkarsh Saxena
Date: 2020-09-19T10:54:04+02:00
New Revision: 985deba9319be464673c1002767f8a3ec597480d

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

LOG: Revert "Temporarily Revert "[clangd] Add Random Forest runtime for code 
completion.""

We intend to replace heuristics based code completion ranking with a Decision 
Forest Model.

This patch introduces a format for representing the model and an inference 
runtime that is code-generated at build time.
- Forest.json contains all the trees as an array of trees.
- Features.json describes the features to be used.
- Codegen file takes the above two files and generates CompletionModel 
containing Feature struct and corresponding Evaluate function.
   The Evaluate function maps a feature to a real number describing the 
relevance of this candidate.
- The codegen is part of build system and these files are generated at build 
time.
- Proposes a way to test the generated runtime using a test model.
  - Replicates the model structure in unittests.
  - unittest tests both the test model (for correct tree traversal) and the 
real model (for sanity).

This reverts commit 549e55b3d5634870aa9d42135f51ad46a6a0e347.

Added: 
clang-tools-extra/clangd/quality/CompletionModel.cmake
clang-tools-extra/clangd/quality/CompletionModelCodegen.py
clang-tools-extra/clangd/quality/README.md
clang-tools-extra/clangd/quality/model/features.json
clang-tools-extra/clangd/quality/model/forest.json
clang-tools-extra/clangd/unittests/DecisionForestTests.cpp

clang-tools-extra/clangd/unittests/decision_forest_model/CategoricalFeature.h
clang-tools-extra/clangd/unittests/decision_forest_model/features.json
clang-tools-extra/clangd/unittests/decision_forest_model/forest.json

Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index 3a1a034ed17b..9d2ab5be222a 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -28,6 +28,9 @@ set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Option
   )
+  
+include(${CMAKE_CURRENT_SOURCE_DIR}/quality/CompletionModel.cmake)
+gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/quality/model CompletionModel 
clang::clangd::Example)
 
 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
@@ -77,6 +80,7 @@ add_clang_library(clangDaemon
   TUScheduler.cpp
   URI.cpp
   XRefs.cpp
+  ${CMAKE_CURRENT_BINARY_DIR}/CompletionModel.cpp
 
   index/Background.cpp
   index/BackgroundIndexLoader.cpp
@@ -117,6 +121,11 @@ add_clang_library(clangDaemon
   omp_gen
   )
 
+# Include generated CompletionModel headers.
+target_include_directories(clangDaemon PUBLIC
+  $
+)
+
 clang_target_link_libraries(clangDaemon
   PRIVATE
   clangAST

diff  --git a/clang-tools-extra/clangd/quality/CompletionModel.cmake 
b/clang-tools-extra/clangd/quality/CompletionModel.cmake
new file mode 100644
index ..60c6d2aa8433
--- /dev/null
+++ b/clang-tools-extra/clangd/quality/CompletionModel.cmake
@@ -0,0 +1,37 @@
+# Run the Completion Model Codegenerator on the model present in the 
+# ${model} directory.
+# Produces a pair of files called ${filename}.h and  ${filename}.cpp in the 
+# ${CMAKE_CURRENT_BINARY_DIR}. The generated header
+# will define a C++ class called ${cpp_class} - which may be a
+# namespace-qualified class name.
+function(gen_decision_forest model filename cpp_class)
+  set(model_compiler 
${CMAKE_SOURCE_DIR}/../clang-tools-extra/clangd/quality/CompletionModelCodegen.py)
+  
+  set(output_dir ${CMAKE_CURRENT_BINARY_DIR})
+  set(header_file ${output_dir}/${filename}.h)
+  set(cpp_file ${output_dir}/${filename}.cpp)
+
+  add_custom_command(OUTPUT ${header_file} ${cpp_file}
+COMMAND "${Python3_EXECUTABLE}" ${model_compiler}
+  --model ${model}
+  --output_dir ${output_dir}
+  --filename ${filename}
+  --cpp_class ${cpp_class}
+COMMENT "Generating code completion model runtime..."
+DEPENDS ${model_compiler} ${model}/forest.json ${model}/features.json
+VERBATIM )
+
+  set_source_files_properties(${header_file} PROPERTIES
+GENERATED 1)
+  set_source_files_properties(${cpp_file} PROPERTIES
+GENERATED 1)
+
+  # Disable unused label warning for generated files.
+  if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
+set_source_files_properties(${cpp_file} PROPERTIES
+  COMPILE_FLAGS /wd4102)
+  else()
+set_source_files_properties(${cpp_file} PROPERTIES
+  COMPILE_FLAGS

[clang] f64903f - Add -Wno-error=unknown flag to clang-format.

2020-09-19 Thread Joachim Meyer via cfe-commits 

Author: Joachim Meyer
Date: 2020-09-19T10:17:57+02:00
New Revision: f64903fd81764f1fde7aeb00eea5e1d488458f63

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

LOG: Add -Wno-error=unknown flag to clang-format.

Currently newer clang-format options cannot be included in .clang-format files, 
if not all users can be forced to use an updated version.
This patch tries to solve this by adding an option to clang-format, enabling to 
ignore unknown (newer) options.

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

Added: 


Modified: 
clang/docs/ClangFormat.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/tools/clang-format/ClangFormat.cpp
clang/unittests/Format/FormatTest.cpp
llvm/include/llvm/Support/YAMLParser.h
llvm/include/llvm/Support/YAMLTraits.h
llvm/lib/Support/YAMLParser.cpp
llvm/lib/Support/YAMLTraits.cpp
llvm/unittests/ObjectYAML/YAMLTest.cpp

Removed: 




diff  --git a/clang/docs/ClangFormat.rst b/clang/docs/ClangFormat.rst
index b09f48b0027b..d396667ce10c 100644
--- a/clang/docs/ClangFormat.rst
+++ b/clang/docs/ClangFormat.rst
@@ -31,6 +31,12 @@ to format C/C++/Java/JavaScript/Objective-C/Protobuf/C# code.
   Clang-format options:
 
 --Werror   - If set, changes formatting warnings to errors
+--Wno-error=unknown- If set, unknown format options are only 
warned about.
+ This can be used to enable formatting, even 
if the
+ configuration contains unknown (newer) 
options.
+ Use with caution, as this might lead to 
dramatically
+ 
diff ering format depending on an option being
+ supported or not.
 --assume-filename= - Override filename used to determine the 
language.
  When reading from stdin, clang-format assumes 
this
  filename to determine the language.

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 2840a6846018..40d5184f4d0d 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2523,7 +2523,8 @@ struct FormatStyle {
 private:
   FormatStyleSet StyleSet;
 
-  friend std::error_code parseConfiguration(StringRef Text, FormatStyle 
*Style);
+  friend std::error_code parseConfiguration(StringRef Text, FormatStyle *Style,
+bool AllowUnknownOptions);
 };
 
 /// Returns a format style complying with the LLVM coding standards:
@@ -2578,7 +2579,11 @@ bool getPredefinedStyle(StringRef Name, 
FormatStyle::LanguageKind Language,
 ///
 /// When ``BasedOnStyle`` is not present, options not present in the YAML
 /// document, are retained in \p Style.
-std::error_code parseConfiguration(StringRef Text, FormatStyle *Style);
+///
+/// If AllowUnknownOptions is true, no errors are emitted if unknown
+/// format options are occured.
+std::error_code parseConfiguration(StringRef Text, FormatStyle *Style,
+   bool AllowUnknownOptions = false);
 
 /// Gets configuration in a YAML string.
 std::string configurationAsText(const FormatStyle );
@@ -2715,6 +2720,9 @@ extern const char *DefaultFallbackStyle;
 /// language if the filename isn't sufficient.
 /// \param[in] FS The underlying file system, in which the file resides. By
 /// default, the file system is the real file system.
+/// \param[in] AllowUnknownOptions If true, unknown format options only
+/// emit a warning. If false, errors are emitted on unknown format
+/// options.
 ///
 /// \returns FormatStyle as specified by ``StyleName``. If ``StyleName`` is
 /// "file" and no file is found, returns ``FallbackStyle``. If no style could 
be
@@ -2722,7 +2730,8 @@ extern const char *DefaultFallbackStyle;
 llvm::Expected getStyle(StringRef StyleName, StringRef FileName,
  StringRef FallbackStyle,
  StringRef Code = "",
- llvm::vfs::FileSystem *FS = nullptr);
+ llvm::vfs::FileSystem *FS = nullptr,
+ bool AllowUnknownOptions = false);
 
 // Guesses the language from the ``FileName`` and ``Code`` to be formatted.
 // Defaults to FormatStyle::LK_Cpp.

diff  --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 03188b46099d..8e4f65ace3f0 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1301,7 +1301,8 @@ bool getPredefinedStyle(StringRef Name, 
FormatStyle::LanguageKind Language,
   return true;
 }

[PATCH] D86137: Add -Wno-error=unknown flag to clang-format.

2020-09-19 Thread Joachim Meyer via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf64903fd8176: Add -Wno-error=unknown flag to clang-format. 
(authored by fodinabor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86137

Files:
  clang/docs/ClangFormat.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp
  llvm/include/llvm/Support/YAMLParser.h
  llvm/include/llvm/Support/YAMLTraits.h
  llvm/lib/Support/YAMLParser.cpp
  llvm/lib/Support/YAMLTraits.cpp
  llvm/unittests/ObjectYAML/YAMLTest.cpp

Index: llvm/unittests/ObjectYAML/YAMLTest.cpp
===
--- llvm/unittests/ObjectYAML/YAMLTest.cpp
+++ llvm/unittests/ObjectYAML/YAMLTest.cpp
@@ -35,3 +35,21 @@
   YOut << BH;
   EXPECT_NE(OS.str().find("''"), StringRef::npos);
 }
+
+TEST(ObjectYAML, UnknownOption) {
+  StringRef InputYAML = "InvalidKey: InvalidValue\n"
+"Binary: \n";
+  BinaryHolder BH;
+  yaml::Input Input(InputYAML);
+  // test 1: default in trying to parse invalid key is an error case.
+  Input >> BH;
+  EXPECT_EQ(Input.error().value(), 22);
+
+  // test 2: only warn about invalid key if actively set.
+  yaml::Input Input2(InputYAML);
+  BinaryHolder BH2;
+  Input2.setAllowUnknownKeys(true);
+  Input2 >> BH2;
+  EXPECT_EQ(BH2.Binary, yaml::BinaryRef(""));
+  EXPECT_EQ(Input2.error().value(), 0);
+}
Index: llvm/lib/Support/YAMLTraits.cpp
===
--- llvm/lib/Support/YAMLTraits.cpp
+++ llvm/lib/Support/YAMLTraits.cpp
@@ -48,6 +48,10 @@
   Ctxt = Context;
 }
 
+void IO::setAllowUnknownKeys(bool Allow) {
+  llvm_unreachable("Only supported for Input");
+}
+
 //===--===//
 //  Input
 //===--===//
@@ -197,8 +201,12 @@
 return;
   for (const auto  : MN->Mapping) {
 if (!is_contained(MN->ValidKeys, NN.first())) {
-  setError(NN.second.get(), Twine("unknown key '") + NN.first() + "'");
-  break;
+  HNode *ReportNode = NN.second.get();
+  if (!AllowUnknownKeys) {
+setError(ReportNode, Twine("unknown key '") + NN.first() + "'");
+break;
+  } else
+reportWarning(ReportNode, Twine("unknown key '") + NN.first() + "'");
 }
   }
 }
@@ -370,6 +378,11 @@
   EC = make_error_code(errc::invalid_argument);
 }
 
+void Input::reportWarning(HNode *hnode, const Twine ) {
+  assert(hnode && "HNode must not be NULL");
+  Strm->printError(hnode->_node, message, SourceMgr::DK_Warning);
+}
+
 std::unique_ptr Input::createHNodes(Node *N) {
   SmallString<128> StringStorage;
   if (ScalarNode *SN = dyn_cast(N)) {
@@ -428,6 +441,8 @@
   setError(CurrentNode, Message);
 }
 
+void Input::setAllowUnknownKeys(bool Allow) { AllowUnknownKeys = Allow; }
+
 bool Input::canElideEmptySequence() {
   return false;
 }
Index: llvm/lib/Support/YAMLParser.cpp
===
--- llvm/lib/Support/YAMLParser.cpp
+++ llvm/lib/Support/YAMLParser.cpp
@@ -1775,12 +1775,9 @@
 
 bool Stream::failed() { return scanner->failed(); }
 
-void Stream::printError(Node *N, const Twine ) {
+void Stream::printError(Node *N, const Twine , SourceMgr::DiagKind Kind) {
   SMRange Range = N ? N->getSourceRange() : SMRange();
-  scanner->printError( Range.Start
- , SourceMgr::DK_Error
- , Msg
- , Range);
+  scanner->printError(Range.Start, Kind, Msg, Range);
 }
 
 document_iterator Stream::begin() {
Index: llvm/include/llvm/Support/YAMLTraits.h
===
--- llvm/include/llvm/Support/YAMLTraits.h
+++ llvm/include/llvm/Support/YAMLTraits.h
@@ -789,6 +789,7 @@
   virtual NodeKind getNodeKind() = 0;
 
   virtual void setError(const Twine &) = 0;
+  virtual void setAllowUnknownKeys(bool Allow);
 
   template 
   void enumCase(T , const char* Str, const T ConstVal) {
@@ -1495,6 +1496,9 @@
   void setError(HNode *hnode, const Twine );
   void setError(Node *node, const Twine );
 
+  void reportWarning(HNode *hnode, const Twine );
+  void reportWarning(Node *hnode, const Twine );
+
 public:
   // These are only used by operator>>. They could be private
   // if those templated things could be made friends.
@@ -1504,6 +1508,8 @@
   /// Returns the current node that's being parsed by the YAML Parser.
   const Node *getCurrentNode() const;
 
+  void setAllowUnknownKeys(bool Allow) override;
+
 private:
   SourceMgr   SrcMgr; // must be before Strm
   std::unique_ptr Strm;
@@ -1514,6 +1520,7 @@
   std::vector   BitValuesUsed;
   HNode

[PATCH] D87837: [Driver] Remove the deprecation warning for -fuse-ld=/abs/path

2020-09-19 Thread Chandler Carruth via Phabricator via cfe-commits 
chandlerc added a comment.

FWIW, I would like to see *something* like this (both in the release and on 
trunk) but not sure this is actually the patch we want... Clang typically uses 
`FIXME` comments, and doesn't leave commented out code...

I feel like this should be an off-by-default warning, with an actual warning 
flag maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87837

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