[PATCH] D89177: [cmake] Add support for multiple distributions

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Reading up on the Ninja multi-config generator a bit, you can define your own 
build types, so you should be able to create e.g. one build type for LTO and 
another for PIC. You'd still need some external logic to define that you use 
the LTO mode for executables and the PIC mode for libraries, but the multiple 
distribution support should work nicely in conjunction with that.

Unfortunately, I don't think that'll work for my RTTI case, since LLVM's flag 
additions for enabling or disabling (based on `LLVM_ENABLE_RTTI`) will override 
any flags you specify for your build type. (Maybe there's a reasonable way to 
tweak the build system to support that case though.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89177

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

[PATCH] D89177: [cmake] Add support for multiple distributions

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D89177#2326832 , @smeenai wrote:

> In D89177#2325566 , @phosek wrote:
>
>> We've already considered introducing a similar mechanism so thank you for 
>> working on this! There's one issue that I haven't figured out how to resolve 
>> and I'd be interested in your thoughts: building executables and libraries 
>> in the most optimal may require incompatible flags. For example, when 
>> building a shared library you have to use `-fPIC` but for executables that's 
>> suboptimal; similarly, when building executables you may want to use LTO but 
>> if you also want to create a distribution that contains static libraries, 
>> that's a problem because those will contain bitcode. So far our solution has 
>> been to simply do multiple builds with different flags (in GN this can be 
>> done in a single build by leveraging the "toolchain" concept that also 
>> allows sharing as much work as possible, but I haven't figured out how to do 
>> anything like that in CMake).
>
> Good question. We need something similar as well, where some clients of the 
> libraries want them built with RTTI (because their programs rely on RTTI and 
> you can get link errors when linking them against non-RTTI LLVM libraries), 
> but we don't wanna build the executables with RTTI because of the size 
> increases.

We recently ran into this as well where we need RTTI for LLVM libraries but 
don't want to enable it for the toolchain.

> I don't know of any way to do this in CMake other than just configuring 
> multiple times with the different flags, like you said. Maybe we could 
> implement some logic for that in the build system, but I'm not sure if it's 
> worth it. (I'm wondering if the Ninja multi-config generator added in CMake 
> 3.17 could be useful for this, although I haven't played around with that at 
> all yet.) I do recognize it limits the usefulness of this approach quite a 
> bit (since if you're doing separate configurations you can just do different 
> `LLVM_DISTRIBUTION_COMPONENT` settings), but I'm hoping it can still be 
> valuable for simpler scenarios.

One idea I had would be to eliminate the use of global variables like 
`CMAKE_C_FLAGS` and `CMAKE_CXX_FLAGS` and always set these via 
`target_compile_options` (which is something we should do in any case to move 
towards more modern CMake); since we always use functions like 
`add_llvm_component_library` to create targets in LLVM, we could then have 
these create multiple targets that would differ in flags like `-fPIC` or 
`-frtti` and set up dependencies among these appropriately. It means you'd have 
to build files multiple times (I don't think there's any way around that) but 
you'd save time on things like tablegen.

> Out of curiosity, with GN, how would you specify that you want binaries to be 
> built with one toolchain (e.g. LTO and non-PIC) and libraries with another 
> (e.g. non-LTO and PIC)?

In GN there's no global concept of tools like `CC` or `LD`, instead these are 
scoped inside a `toolchain` which in GN is a first class concept, and 
dependencies can be parametrized by toolchains (for example you can have `deps 
= [ "//path/to/dependency(//path/to/toolchain)" ]`. These toolchains can be 
also parametrized and templated. A common use case is to have a variety of 
toolchains that have the same tools but differ in flags like `-fPIC`. In Ninja 
this is represented as subninjas where each child Ninja file defines its own 
set of rules.

You can see an example of this even in LLVM GN build where we define a set of 
toolchains in 
https://github.com/llvm/llvm-project/blob/1687a8d83b702332c4aae4f2a95d27c16688418d/llvm/utils/gn/build/toolchain/BUILD.gn#L179
 and then use them to built runtimes in 
https://github.com/llvm/llvm-project/blob/1687a8d83b702332c4aae4f2a95d27c16688418d/llvm/utils/gn/secondary/compiler-rt/BUILD.gn
 which is an (more efficient) equivalent of the runtimes build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89177

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

[PATCH] D89300: [clang-rename] Fix rename on variable templates.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: kbobyrev.
Herald added a project: clang.
hokein requested review of this revision.

This patch adds support for renaming variable templates.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89300

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
  clang/test/clang-rename/VariableTemplate.cpp

Index: clang/test/clang-rename/VariableTemplate.cpp
===
--- /dev/null
+++ clang/test/clang-rename/VariableTemplate.cpp
@@ -0,0 +1,32 @@
+template 
+bool Foo = true;  // CHECK: bool Bar = true;
+
+// explicit template specialization
+template <>
+bool Foo = false; // CHECK: bool Bar = false;
+
+// partial template specialization
+template 
+bool Foo = false; // bool Bar = false;
+
+void k() {
+  // ref to the explicit template specialization
+  Foo;   // CHECK: Bar;
+  // ref to the primary template.
+  Foo;   // CHECK: Bar;
+}
+
+
+// Test 1.
+// RUN: clang-rename -offset=34 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 2.
+// RUN: clang-rename -offset=128 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 3.
+// RUN: clang-rename -offset=248 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 4.
+// RUN: clang-rename -offset=357 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 5.
+// RUN: clang-rename -offset=431 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+
+// To find offsets after modifying the file, use:
+//   grep -Ubo 'Foo.*' 
Index: clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRFindingAction.cpp
@@ -87,6 +87,16 @@
 handleFunctionTemplateDecl(FTD);
 } else if (const auto *FD = dyn_cast(FoundDecl)) {
   handleFunctionTemplateDecl(FD);
+} else if (const auto *VTD = dyn_cast(FoundDecl)) {
+  handleVarTemplateDecl(VTD);
+} else if (const auto *VD =
+   dyn_cast(FoundDecl)) {
+  // FIXME: figure out why FoundDecl can be a VarTemplateSpecializationDecl.
+  handleVarTemplateDecl(VD->getSpecializedTemplate());
+} else if (const auto *VD = dyn_cast(FoundDecl)) {
+  USRSet.insert(getUSRForDecl(VD));
+  if (const auto *VTD = VD->getDescribedVarTemplate())
+handleVarTemplateDecl(VTD);
 } else {
   USRSet.insert(getUSRForDecl(FoundDecl));
 }
@@ -132,6 +142,19 @@
   USRSet.insert(getUSRForDecl(S));
   }
 
+  void handleVarTemplateDecl(const VarTemplateDecl *VTD) {
+USRSet.insert(getUSRForDecl(VTD));
+USRSet.insert(getUSRForDecl(VTD->getTemplatedDecl()));
+llvm::for_each(VTD->specializations(), [&](const auto *Spec) {
+  USRSet.insert(getUSRForDecl(Spec));
+});
+SmallVector PartialSpecs;
+VTD->getPartialSpecializations(PartialSpecs);
+llvm::for_each(PartialSpecs, [&](const auto *Spec) {
+  USRSet.insert(getUSRForDecl(Spec));
+});
+  }
+
   void addUSRsOfCtorDtors(const CXXRecordDecl *RD) {
 const auto* RecordDecl = RD->getDefinition();
 
Index: clang/lib/AST/DeclTemplate.cpp
===
--- clang/lib/AST/DeclTemplate.cpp
+++ clang/lib/AST/DeclTemplate.cpp
@@ -1142,7 +1142,7 @@
 }
 
 llvm::FoldingSetVector &
-VarTemplateDecl::getPartialSpecializations() {
+VarTemplateDecl::getPartialSpecializations() const {
   LoadLazySpecializations();
   return getCommonPtr()->PartialSpecializations;
 }
@@ -1198,7 +1198,7 @@
 }
 
 void VarTemplateDecl::getPartialSpecializations(
-SmallVectorImpl &PS) {
+SmallVectorImpl &PS) const {
   llvm::FoldingSetVector &PartialSpecs =
   getPartialSpecializations();
   PS.clear();
Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -3095,7 +3095,7 @@
   /// Retrieve the set of partial specializations of this class
   /// template.
   llvm::FoldingSetVector &
-  getPartialSpecializations();
+  getPartialSpecializations() const;
 
   VarTemplateDecl(ASTContext &C, DeclContext *DC, SourceLocation L,
   DeclarationName Name, TemplateParameterList *Params,
@@ -3191,7 +3191,7 @@
 
   /// Retrieve the partial specializations as an ordered list.
   void getPartialSpecializations(
-  SmallVectorImpl &PS);
+  SmallVectorImpl &PS) const;
 
   /// Find a variable template partial specialization which was
   /// instantiated
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89301: [X86] Add user-level interrupt instructions

[Wang Tianqing via Phabricator via cfe-commits]

tianqing created this revision.
Herald added subscribers: llvm-commits, cfe-commits, dang, hiraditya, mgorny.
Herald added projects: clang, LLVM.
tianqing requested review of this revision.

For more details about these instructions, please refer to the latest
ISE document:
https://software.intel.com/en-us/download/intel-architecture-instruction-set-extensions-programming-reference.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89301

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/cpuid.h
  clang/lib/Headers/uintrintrin.h
  clang/lib/Headers/x86gprintrin.h
  clang/test/CodeGen/X86/x86-uintr-builtins.c
  clang/test/Driver/x86-target-features.c
  clang/test/Preprocessor/x86_target_features.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/test/CodeGen/X86/uintr-intrinsics.ll
  llvm/test/MC/Disassembler/X86/x86-64.txt
  llvm/test/MC/X86/x86-64.s

Index: llvm/test/MC/X86/x86-64.s
===
--- llvm/test/MC/X86/x86-64.s
+++ llvm/test/MC/X86/x86-64.s
@@ -2018,3 +2018,35 @@
 // CHECK: hreset
 // CHECK: encoding: [0xf3,0x0f,0x3a,0xf0,0xc0,0x01]
 hreset $1
+
+// CHECK: uiret
+// CHECK: encoding: [0xf3,0x0f,0x01,0xec]
+uiret
+
+// CHECK: clui
+// CHECK: encoding: [0xf3,0x0f,0x01,0xee]
+clui
+
+// CHECK: stui
+// CHECK: encoding: [0xf3,0x0f,0x01,0xef]
+stui
+
+// CHECK: testui
+// CHECK: encoding: [0xf3,0x0f,0x01,0xed]
+testui
+
+// CHECK: senduipi %rax
+// CHECK: encoding: [0xf3,0x0f,0xc7,0xf0]
+senduipi %rax
+
+// CHECK: senduipi %rdx
+// CHECK: encoding: [0xf3,0x0f,0xc7,0xf2]
+senduipi %rdx
+
+// CHECK: senduipi %r8
+// CHECK: encoding: [0xf3,0x41,0x0f,0xc7,0xf0]
+senduipi %r8
+
+// CHECK: senduipi %r13
+// CHECK: encoding: [0xf3,0x41,0x0f,0xc7,0xf5]
+senduipi %r13
Index: llvm/test/MC/Disassembler/X86/x86-64.txt
===
--- llvm/test/MC/Disassembler/X86/x86-64.txt
+++ llvm/test/MC/Disassembler/X86/x86-64.txt
@@ -715,3 +715,27 @@
 
 # CHECK: hreset $1
 0xf3 0x0f 0x3a 0xf0 0xc0 0x01
+
+# CHECK: uiret
+0xf3,0x0f,0x01,0xec
+
+# CHECK: clui
+0xf3,0x0f,0x01,0xee
+
+# CHECK: stui
+0xf3,0x0f,0x01,0xef
+
+# CHECK: testui
+0xf3,0x0f,0x01,0xed
+
+# CHECK: senduipi %rax
+0xf3,0x0f,0xc7,0xf0
+
+# CHECK: senduipi %rdx
+0xf3,0x0f,0xc7,0xf2
+
+# CHECK: senduipi %r8
+0xf3,0x41,0x0f,0xc7,0xf0
+
+# CHECK: senduipi %r13
+0xf3,0x41,0x0f,0xc7,0xf5
Index: llvm/test/CodeGen/X86/uintr-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/uintr-intrinsics.ll
@@ -0,0 +1,34 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+uintr | FileCheck %s --check-prefix=X64
+; RUN: llc < %s -mtriple=x86_64-linux-gnux32 -mattr=+uintr | FileCheck %s --check-prefix=X32
+
+define i8 @test_uintr(i64 %arg) {
+; X64-LABEL: test_uintr:
+; X64:   # %bb.0: # %entry
+; X64-NEXT:clui
+; X64-NEXT:stui
+; X64-NEXT:senduipi %rdi
+; X64-NEXT:testui
+; X64-NEXT:setb %al
+; X64-NEXT:retq
+
+; X32-LABEL: test_uintr:
+; X32:   # %bb.0: # %entry
+; X32-NEXT:clui
+; X32-NEXT:stui
+; X32-NEXT:senduipi %rdi
+; X32-NEXT:testui
+; X32-NEXT:setb %al
+; X32-NEXT:retq
+entry:
+  call void @llvm.x86.clui()
+  call void @llvm.x86.stui()
+  call void @llvm.x86.senduipi(i64 %arg)
+  %0 = call i8 @llvm.x86.testui()
+  ret i8 %0
+}
+
+declare void @llvm.x86.clui()
+declare void @llvm.x86.stui()
+declare i8 @llvm.x86.testui()
+declare void @llvm.x86.senduipi(i64 %arg)
Index: llvm/lib/Target/X86/X86Subtarget.h
===
--- llvm/lib/Target/X86/X86Subtarget.h
+++ llvm/lib/Target/X86/X86Subtarget.h
@@ -415,6 +415,9 @@
   bool HasAMXBF16 = false;
   bool HasAMXINT8 = false;
 
+  /// Processor supports User Level Interrupt instructions
+  bool HasUINTR = false;
+
   /// Processor has a single uop BEXTR implementation.
   bool HasFastBEXTR = false;
 
@@ -742,6 +745,7 @@
   bool hasHRESET() const { return HasHRESET; }
   bool hasSERIALIZE() const { return HasSERIALIZE; }
   bool hasTSXLDTRK() const { return HasTSXLDTRK; }
+  bool hasUINTR() const { return HasUINTR; }
   bool useRetpolineIndirectCalls() const { return UseRetpolineIndirectCalls; }
   bool useRetpolineIndirectBranches() const {
 return UseRetpolineIndirectBranches;
Index: llvm/lib/Target/X86/X86I

[clang] 6c23cbc - [X86] Convert integer _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2020-10-13T09:28:39+01:00
New Revision: 6c23cbc5603cf0011f8d57b0354954aeca695daf

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

LOG: [X86] Convert integer _mm_reduce_* intrinsics to emit llvm.reduction 
intrinsics (PR47506)

Emit the equivalent integer reduction intrinsics in IR instead of expanding to 
shuffle+arithmetic sequences.

The fadd/fmul reductions might be trickier as they assume a similar bisection 
reduction while the generic intrinsics assume a sequential reduction (intel 
docs are ambiguous on the correct approach) - I'm not sure if we want to always 
tag them with reassoc? Anyway, that issue can wait until a separate fp patch 
along with the fmin/fmax reductions.

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsX86.def
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Headers/avx512fintrin.h
clang/test/CodeGen/X86/avx512-reduceIntrin.c
clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsX86.def 
b/clang/include/clang/Basic/BuiltinsX86.def
index 8f9cfe4b6dc5..6bfb9b2cf8a5 100644
--- a/clang/include/clang/Basic/BuiltinsX86.def
+++ b/clang/include/clang/Basic/BuiltinsX86.def
@@ -1871,6 +1871,24 @@ TARGET_BUILTIN(__builtin_ia32_selectpd_512, 
"V8dUcV8dV8d", "ncV:512:", "avx512f"
 TARGET_BUILTIN(__builtin_ia32_selectss_128, "V4fUcV4fV4f", "ncV:128:", 
"avx512f")
 TARGET_BUILTIN(__builtin_ia32_selectsd_128, "V2dUcV2dV2d", "ncV:128:", 
"avx512f")
 
+// generic reduction intrinsics
+TARGET_BUILTIN(__builtin_ia32_reduce_add_d512, "iV16i", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_add_q512, "OiV8Oi", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_and_d512, "iV16i", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_and_q512, "OiV8Oi", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_mul_d512, "iV16i", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_mul_q512, "OiV8Oi", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_or_d512, "iV16i", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_or_q512, "OiV8Oi", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_smax_d512, "iV16i", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_smax_q512, "OiV8Oi", "ncV:512:", 
"avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_smin_d512, "iV16i", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_smin_q512, "OiV8Oi", "ncV:512:", 
"avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_umax_d512, "iV16i", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_umax_q512, "OiV8Oi", "ncV:512:", 
"avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_umin_d512, "iV16i", "ncV:512:", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_reduce_umin_q512, "OiV8Oi", "ncV:512:", 
"avx512f")
+
 // MONITORX/MWAITX
 TARGET_BUILTIN(__builtin_ia32_monitorx, "vvC*UiUi", "n", "mwaitx")
 TARGET_BUILTIN(__builtin_ia32_mwaitx, "vUiUiUi", "n", "mwaitx")

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index e8c64bf0f351..45deb4164553 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -13416,6 +13416,56 @@ Value *CodeGenFunction::EmitX86BuiltinExpr(unsigned 
BuiltinID,
 // Ops 0 and 1 are swapped.
 return EmitX86FunnelShift(*this, Ops[1], Ops[0], Ops[2], true);
 
+  // Reductions
+  case X86::BI__builtin_ia32_reduce_add_d512:
+  case X86::BI__builtin_ia32_reduce_add_q512: {
+Function *F =
+CGM.getIntrinsic(Intrinsic::vector_reduce_add, Ops[0]->getType());
+return Builder.CreateCall(F, {Ops[0]});
+  }
+  case X86::BI__builtin_ia32_reduce_and_d512:
+  case X86::BI__builtin_ia32_reduce_and_q512: {
+Function *F =
+CGM.getIntrinsic(Intrinsic::vector_reduce_and, Ops[0]->getType());
+return Builder.CreateCall(F, {Ops[0]});
+  }
+  case X86::BI__builtin_ia32_reduce_mul_d512:
+  case X86::BI__builtin_ia32_reduce_mul_q512: {
+Function *F =
+CGM.getIntrinsic(Intrinsic::vector_reduce_mul, Ops[0]->getType());
+return Builder.CreateCall(F, {Ops[0]});
+  }
+  case X86::BI__builtin_ia32_reduce_or_d512:
+  case X86::BI__builtin_ia32_reduce_or_q512: {
+Function *F =
+CGM.getIntrinsic(Intrinsic::vector_reduce_or, Ops[0]->getType());
+return Builder.CreateCall(F, {Ops[0]});
+  }
+  case X86::BI__builtin_ia32_reduce_smax_d512:
+  case X86::BI__builtin_ia32_reduce_smax_q512: {
+Function *F =
+CGM.getIntrinsic(Intrinsic::vector_reduce_smax, Ops[0]->getType());
+return Builder.CreateCall(F, {Ops[0]});
+  }
+  case X86::BI__builtin_ia32_reduce_smin_d512:
+  case X86::BI__builtin_ia32_reduce_smin_q512: {
+Function *F =
+  

[PATCH] D87604: [X86] Convert integer _mm_reduce_* intrinsics to emit llvm.reduction intrinsics (PR47506)

[Simon Pilgrim 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 rG6c23cbc5603c: [X86] Convert integer _mm_reduce_* intrinsics 
to emit llvm.reduction intrinsics… (authored by RKSimon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87604

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/avx512fintrin.h
  clang/test/CodeGen/X86/avx512-reduceIntrin.c
  clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c

Index: clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c
===
--- clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c
+++ clang/test/CodeGen/X86/avx512-reduceMinMaxIntrin.c
@@ -4,25 +4,13 @@
 
 long long test_mm512_reduce_max_epi64(__m512i __W){
 // CHECK-LABEL: @test_mm512_reduce_max_epi64(
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.smax.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.smax.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.smax.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:extractelement <8 x i64> %{{.*}}, i32 0
+// CHECK:call i64 @llvm.vector.reduce.smax.v8i64(<8 x i64> %{{.*}})
   return _mm512_reduce_max_epi64(__W);
 }
 
 unsigned long long test_mm512_reduce_max_epu64(__m512i __W){
 // CHECK-LABEL: @test_mm512_reduce_max_epu64(
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.umax.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.umax.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.umax.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:extractelement <8 x i64> %{{.*}}, i32 0
+// CHECK:call i64 @llvm.vector.reduce.umax.v8i64(<8 x i64> %{{.*}})
   return _mm512_reduce_max_epu64(__W);
 }
 
@@ -42,25 +30,13 @@
 
 long long test_mm512_reduce_min_epi64(__m512i __W){
 // CHECK-LABEL: @test_mm512_reduce_min_epi64(
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.smin.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.smin.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.smin.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:extractelement <8 x i64> %{{.*}}, i32 0
+// CHECK:call i64 @llvm.vector.reduce.smin.v8i64(<8 x i64> %{{.*}})
   return _mm512_reduce_min_epi64(__W);
 }
 
 unsigned long long test_mm512_reduce_min_epu64(__m512i __W){
 // CHECK-LABEL: @test_mm512_reduce_min_epu64(
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.umin.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.umin.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.umin.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:extractelement <8 x i64> %{{.*}}, i32 0
+// CHECK:call i64 @llvm.vector.reduce.umin.v8i64(<8 x i64> %{{.*}})
   return _mm512_reduce_min_epu64(__W);
 }
 
@@ -82,13 +58,7 @@
 // CHECK-LABEL: @test_mm512_mask_reduce_max_epi64(
 // CHECK:bitcast i8 %{{.*}} to <8 x i1>
 // CHECK:select <8 x i1> %{{.*}}, <8 x i64> %{{.*}}, <8 x i64> %{{.*}}
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.smax.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.smax.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:shufflevector <8 x i64> %{{.*}}, <8 x i64> %{{.*}}, <8 x i32> 
-// CHECK:call <8 x i64> @llvm.smax.v8i64(<8 x i64> %{{.*}}, <8 x i64> %{{.*}})
-// CHECK:extractelement <8 x i64> %{{.*}}, i32 0
+// CHECK:call i64 @llvm.vector.reduce.smax.v8i64(<8 x i64> %{{.*}})
   return _mm512_mask_reduce_max_epi64(__M, __W); 
 }
 
@@ -96,13 +66,7 @@
 // CHECK-LABEL: @test_mm512_mask_reduce_max_epu64(
 // CHECK:bitcast i8 %{{.*}} to <8 x i1>
 // CHECK:select <8 x i1> %{{.*}}, <8 x i64> %{{.*}}, <8 x i64> %{{.*}}
-// 

[PATCH] D89277: [clangd] Add $/dumpMemoryTree LSP extension

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D89277#2326376 , @nridge wrote:

> This is neat! Are there plans to add vscode client support to invoke this?

Yes, since we control the plugin in vscode we should have a way to invoke it 
there. (we already have support for `switchHeaderSource`).

We should first agree on the interaction though. This currently uses 
`showMessage` to display the dump, which is the most generic way LSP provides 
us with, but usually isn't good for multi-line output.
Other options we have are:

- Write it into logs - Hard to find, but at least will look reasonable on every 
editor.
- Insert as a commented block into the code. (like dumpast) - Clutters the 
code, but again looks reasonable on every editor and easier to find than logs.
- Provide a json object in clangd and use fancy client side features like tree 
views. - Probably the best output, but will only work on a handful of editors 
and will require a lot of effort to implement.

I am leaning towards using showMessage and also logging the tree. Do others 
have opinions on other possibilities or pros/cons of the options i've listed 
above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89277

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

[PATCH] D89220: [clang-rename] Simplify the code of handling class paritial specializations, NFC.

[Kirill Bobyrev via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89220

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

[PATCH] D89300: [clang-rename] Fix rename on variable templates.

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Oh, that's neat, thanks!




Comment at: clang/include/clang/AST/DeclTemplate.h:3098
   llvm::FoldingSetVector &
-  getPartialSpecializations();
+  getPartialSpecializations() const;
 

I believe these are from https://reviews.llvm.org/D89221, so probably rebase on 
top of it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89300

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

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Vitaly Buka via cfe-commits]

On Mon, 12 Oct 2020 at 22:01, Galina Kistanova  wrote:

> Thanks, Vitaly!
>
> Let's have them there for at least 24 hours, shall we?
>

We can do that.


>
> Could you move sanitizer-buildbot1, sanitizer-buildbot3,
> sanitizer-buildbot7 as well, please?
>

Done.


> AnnotatedCommand on the staging has been tested functionally and is good.
> My only concern at this point is how it would handle a heavy load, so the
> more bots we will have on the staging the better.
>
> If somebody else could move their AnnotatedCommand bots to the staging
> area, that would be much appreciated.
>
> Thanks
>
> Galina
>
> On Mon, Oct 12, 2020 at 9:45 PM Vitaly Buka  wrote:
>
>> Looks like staging AnnotatedCommand fixed step statuses, so we can see
>> which one is green.
>> Please let me know when to switch bots back from the staging.
>> Thank you!
>>
>> On Mon, 12 Oct 2020 at 21:38, Vitaly Buka  wrote:
>>
>>> Switched all but PPC, I don't have access to them. But they run the same
>>> script as sanitizer-x86_64-linux.
>>> http://lab.llvm.org:8014/#/waterfall?tags=sanitizer
>>>
>>> On Mon, 12 Oct 2020 at 19:19, Galina Kistanova 
>>> wrote:
>>>
 We have a better version of AnnotatedCommand on the staging. It should
 be a functional equivalent of the old one.
 We need to stress test it well before moving to the production build
 bot.

 For that we need all sanitizer + other bots which use the
 AnnotatedCommand directly or indirectly moved temporarily to the staging.

 Please let me know when that could be arranged.

 Thanks

 Galina

 On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner  wrote:

> On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
> lldb-comm...@lists.llvm.org> wrote:
>
>> They are online now -
>> http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>>
>> AnnotatedCommand has severe design conflict with the new buildbot.
>> We have changed it to be safe and still do something useful, but it
>> will need more love and care.
>>
>> Please let me know if you have some spare time to work on porting
>> AnnotatedCommand.
>>
>
> That's unfortunate, it would've been good to know that earlier. I and
> another team member have spent a fair amount of time porting things to use
> more AnnotatedCommand steps, because it gives us the flexibility to test
> steps locally and make changes to the steps without restarting the 
> buildbot
> master. IMO that is the Right Way to define steps: a script that you can
> run locally on a machine that satisfies the OS and dep requirements of the
> script.
>
> I am restarting the two bots that I am responsible for, and may need
> some help debugging further issues soon. I'll let you know.
>

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

[PATCH] D89303: [SyntaxTree] Improve safety of `replaceChildRangeLowLevel`

[Eduardo Caldas via Phabricator via cfe-commits]

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

- Add assertions for other preconditions.
- If nothing is modified, don't mark it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89303

Files:
  clang/lib/Tooling/Syntax/Tree.cpp


Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -94,48 +94,65 @@
 
 void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
  Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+  assert((!BeforeBegin || BeforeBegin->Parent == this) &&
+ "`BeforeBegin` is not a child of `this`.");
+  assert((!End || End->Parent == this) && "`End` is not a child of `this`.");
+  assert(canModify() && "Cannot modify `this`.");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (auto *N = New; N; N = N->NextSibling) {
 assert(N->Parent == nullptr);
 assert(N->getRole() != NodeRole::Detached && "Roles must be set");
 // FIXME: sanity-check the role.
   }
+
+  auto Reachable = [](Node *From, Node *N) {
+if (!N)
+  return true;
+for (auto *It = From; It; It = It->NextSibling)
+  if (It == N)
+return true;
+return false;
+  };
+  assert(Reachable(FirstChild, BeforeBegin) &&
+ "`BeforeBegin` is not reachable.");
+  assert(Reachable(BeforeBegin ? BeforeBegin->NextSibling : FirstChild, End) &&
+ "`End` is not after `BeforeBegin`.");
 #endif
+  auto GetBegin = [&BeforeBegin, &FirstChild = this->FirstChild]() -> Node *& {
+return BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  };
 
   // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  for (auto *N = GetBegin(); N != End;) {
 auto *Next = N->NextSibling;
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->NextSibling = nullptr;
 if (N->Original)
-  traverse(N, [&](Node *C) { C->Original = false; });
+  traverse(N, [](Node *C) { C->Original = false; });
 
 N = Next;
   }
 
-  // Attach new nodes.
-  if (BeforeBegin)
-BeforeBegin->NextSibling = New ? New : End;
-  else
-FirstChild = New ? New : End;
+  // Mark modification.
+  if (New || GetBegin() != End)
+for (auto *T = this; T && T->Original; T = T->Parent)
+  T->Original = false;
 
-  if (New) {
-auto *Last = New;
-for (auto *N = New; N != nullptr; N = N->getNextSibling()) {
-  Last = N;
-  N->Parent = this;
-}
-Last->NextSibling = End;
+  // Attach new nodes.
+  if (!New) {
+GetBegin() = End;
+return;
   }
-
-  // Mark the node as modified.
-  for (auto *T = this; T && T->Original; T = T->Parent)
-T->Original = false;
+  GetBegin() = New;
+  auto *Last = New;
+  for (auto *N = New; N != nullptr; N = N->NextSibling) {
+Last = N;
+N->Parent = this;
+  }
+  Last->NextSibling = End;
 }
 
 namespace {


Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -94,48 +94,65 @@
 
 void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
  Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+  assert((!BeforeBegin || BeforeBegin->Parent == this) &&
+ "`BeforeBegin` is not a child of `this`.");
+  assert((!End || End->Parent == this) && "`End` is not a child of `this`.");
+  assert(canModify() && "Cannot modify `this`.");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (auto *N = New; N; N = N->NextSibling) {
 assert(N->Parent == nullptr);
 assert(N->getRole() != NodeRole::Detached && "Roles must be set");
 // FIXME: sanity-check the role.
   }
+
+  auto Reachable = [](Node *From, Node *N) {
+if (!N)
+  return true;
+for (auto *It = From; It; It = It->NextSibling)
+  if (It == N)
+return true;
+return false;
+  };
+  assert(Reachable(FirstChild, BeforeBegin) &&
+ "`BeforeBegin` is not reachable.");
+  assert(Reachable(BeforeBegin ? BeforeBegin->NextSibling : FirstChild, End) &&
+ "`End` is not after `BeforeBegin`.");
 #endif
+  auto GetBegin = [&BeforeBegin, &FirstChild = this->FirstChild]() -> Node *& {
+return BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  };
 
   // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  for (auto *N = GetBegin(); N != End;) {
 auto *Next = N->NextSibling;
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->NextSibling = nullptr;
 if (N->Original)
-  traverse(N, [&](

[PATCH] D89303: [SyntaxTree] Improve safety of `replaceChildRangeLowLevel`

[Eduardo Caldas via Phabricator via cfe-commits]

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

minor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89303

Files:
  clang/lib/Tooling/Syntax/Tree.cpp


Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -94,48 +94,65 @@
 
 void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
  Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+  assert((!BeforeBegin || BeforeBegin->Parent == this) &&
+ "`BeforeBegin` is not a child of `this`.");
+  assert((!End || End->Parent == this) && "`End` is not a child of `this`.");
+  assert(canModify() && "Cannot modify `this`.");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (auto *N = New; N; N = N->NextSibling) {
 assert(N->Parent == nullptr);
 assert(N->getRole() != NodeRole::Detached && "Roles must be set");
 // FIXME: sanity-check the role.
   }
+
+  auto Reachable = [](Node *From, Node *N) {
+if (!N)
+  return true;
+for (auto *It = From; It; It = It->NextSibling)
+  if (It == N)
+return true;
+return false;
+  };
+  assert(Reachable(FirstChild, BeforeBegin) &&
+ "`BeforeBegin` is not reachable.");
+  assert(Reachable(BeforeBegin ? BeforeBegin->NextSibling : FirstChild, End) &&
+ "`End` is not after `BeforeBegin`.");
 #endif
+  auto GetBegin = [&BeforeBegin, &FirstChild = this->FirstChild]() -> Node *& {
+return BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  };
 
   // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  for (auto *N = GetBegin(); N != End;) {
 auto *Next = N->NextSibling;
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->NextSibling = nullptr;
 if (N->Original)
-  traverse(N, [&](Node *C) { C->Original = false; });
+  traverse(N, [](Node *C) { C->Original = false; });
 
 N = Next;
   }
 
-  // Attach new nodes.
-  if (BeforeBegin)
-BeforeBegin->NextSibling = New ? New : End;
-  else
-FirstChild = New ? New : End;
+  // Mark modification.
+  if (New || GetBegin() != End)
+for (auto *T = this; T && T->Original; T = T->Parent)
+  T->Original = false;
 
-  if (New) {
-auto *Last = New;
-for (auto *N = New; N != nullptr; N = N->getNextSibling()) {
-  Last = N;
-  N->Parent = this;
-}
-Last->NextSibling = End;
+  if (!New) {
+GetBegin() = End;
+return;
   }
-
-  // Mark the node as modified.
-  for (auto *T = this; T && T->Original; T = T->Parent)
-T->Original = false;
+  // Attach new nodes.
+  GetBegin() = New;
+  auto *Last = New;
+  for (auto *N = New; N != nullptr; N = N->NextSibling) {
+Last = N;
+N->Parent = this;
+  }
+  Last->NextSibling = End;
 }
 
 namespace {


Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -94,48 +94,65 @@
 
 void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
  Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+  assert((!BeforeBegin || BeforeBegin->Parent == this) &&
+ "`BeforeBegin` is not a child of `this`.");
+  assert((!End || End->Parent == this) && "`End` is not a child of `this`.");
+  assert(canModify() && "Cannot modify `this`.");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (auto *N = New; N; N = N->NextSibling) {
 assert(N->Parent == nullptr);
 assert(N->getRole() != NodeRole::Detached && "Roles must be set");
 // FIXME: sanity-check the role.
   }
+
+  auto Reachable = [](Node *From, Node *N) {
+if (!N)
+  return true;
+for (auto *It = From; It; It = It->NextSibling)
+  if (It == N)
+return true;
+return false;
+  };
+  assert(Reachable(FirstChild, BeforeBegin) &&
+ "`BeforeBegin` is not reachable.");
+  assert(Reachable(BeforeBegin ? BeforeBegin->NextSibling : FirstChild, End) &&
+ "`End` is not after `BeforeBegin`.");
 #endif
+  auto GetBegin = [&BeforeBegin, &FirstChild = this->FirstChild]() -> Node *& {
+return BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  };
 
   // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  for (auto *N = GetBegin(); N != End;) {
 auto *Next = N->NextSibling;
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->NextSibling = nullptr;
 if (N->Original)
-  traverse(N, [&](Node *C) { C->Original = false; });
+  traverse(N, [](Node *C) { C->Original = fa

[PATCH] D89303: [SyntaxTree] Improve safety of `replaceChildRangeLowLevel`

[Eduardo Caldas via Phabricator via cfe-commits]

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

minor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89303

Files:
  clang/lib/Tooling/Syntax/Tree.cpp


Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -94,48 +94,67 @@
 
 void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
  Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+  assert((!BeforeBegin || BeforeBegin->Parent == this) &&
+ "`BeforeBegin` is not a child of `this`.");
+  assert((!End || End->Parent == this) && "`End` is not a child of `this`.");
+  assert(canModify() && "Cannot modify `this`.");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (auto *N = New; N; N = N->NextSibling) {
 assert(N->Parent == nullptr);
 assert(N->getRole() != NodeRole::Detached && "Roles must be set");
 // FIXME: sanity-check the role.
   }
+
+  auto Reachable = [](Node *From, Node *N) {
+if (!N)
+  return true;
+for (auto *It = From; It; It = It->NextSibling)
+  if (It == N)
+return true;
+return false;
+  };
+  assert(Reachable(FirstChild, BeforeBegin) &&
+ "`BeforeBegin` is not reachable.");
+  assert(Reachable(BeforeBegin ? BeforeBegin->NextSibling : FirstChild, End) &&
+ "`End` is not after `BeforeBegin`.");
 #endif
+  auto GetBegin = [&BeforeBegin, &FirstChild = this->FirstChild]() -> Node *& {
+return BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  };
+
+  if (!New && GetBegin() == End)
+return;
+
+  // Mark modification.
+  for (auto *T = this; T && T->Original; T = T->Parent)
+T->Original = false;
 
   // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  for (auto *N = GetBegin(); N != End;) {
 auto *Next = N->NextSibling;
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->NextSibling = nullptr;
 if (N->Original)
-  traverse(N, [&](Node *C) { C->Original = false; });
+  traverse(N, [](Node *C) { C->Original = false; });
 
 N = Next;
   }
 
+  if (!New) {
+GetBegin() = End;
+return;
+  }
   // Attach new nodes.
-  if (BeforeBegin)
-BeforeBegin->NextSibling = New ? New : End;
-  else
-FirstChild = New ? New : End;
-
-  if (New) {
-auto *Last = New;
-for (auto *N = New; N != nullptr; N = N->getNextSibling()) {
-  Last = N;
-  N->Parent = this;
-}
-Last->NextSibling = End;
+  GetBegin() = New;
+  auto *Last = New;
+  for (auto *N = New; N != nullptr; N = N->NextSibling) {
+Last = N;
+N->Parent = this;
   }
-
-  // Mark the node as modified.
-  for (auto *T = this; T && T->Original; T = T->Parent)
-T->Original = false;
+  Last->NextSibling = End;
 }
 
 namespace {


Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -94,48 +94,67 @@
 
 void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
  Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+  assert((!BeforeBegin || BeforeBegin->Parent == this) &&
+ "`BeforeBegin` is not a child of `this`.");
+  assert((!End || End->Parent == this) && "`End` is not a child of `this`.");
+  assert(canModify() && "Cannot modify `this`.");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (auto *N = New; N; N = N->NextSibling) {
 assert(N->Parent == nullptr);
 assert(N->getRole() != NodeRole::Detached && "Roles must be set");
 // FIXME: sanity-check the role.
   }
+
+  auto Reachable = [](Node *From, Node *N) {
+if (!N)
+  return true;
+for (auto *It = From; It; It = It->NextSibling)
+  if (It == N)
+return true;
+return false;
+  };
+  assert(Reachable(FirstChild, BeforeBegin) &&
+ "`BeforeBegin` is not reachable.");
+  assert(Reachable(BeforeBegin ? BeforeBegin->NextSibling : FirstChild, End) &&
+ "`End` is not after `BeforeBegin`.");
 #endif
+  auto GetBegin = [&BeforeBegin, &FirstChild = this->FirstChild]() -> Node *& {
+return BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  };
+
+  if (!New && GetBegin() == End)
+return;
+
+  // Mark modification.
+  for (auto *T = this; T && T->Original; T = T->Parent)
+T->Original = false;
 
   // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  for (auto *N = GetBegin(); N != End;) {
 auto *Next = N->NextSibling;
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->Next

[PATCH] D89303: [SyntaxTree] Improve safety of `replaceChildRangeLowLevel`

[Eduardo Caldas via Phabricator via cfe-commits]

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



Comment at: clang/lib/Tooling/Syntax/Tree.cpp:103
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (auto *N = New; N; N = N->NextSibling) {
 assert(N->Parent == nullptr);

Throughout the function we use data members instead of accessors. Is one 
preferrable to the other?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89303

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

[PATCH] D87989: [Flang][Driver] Add infrastructure for basic frontend actions and file I/O

[Sourabh Singh Tomar via Phabricator via cfe-commits]

SouraVX added a comment.

Thanks for the patch! Overall it's in pretty nice state and conformant to 
design highlighted. 
Could you please `clang-format` this patch, There are lot of `lint` warning 
that causes lot of noise while reviewing.
Couple of NIT comments  inline. I've tried marking all but, Could you please 
finish comments with period.

I think it would good if someone from `clang` community can also take a brief 
look.




Comment at: clang/include/clang/Driver/Options.td:63
 
+// ClangOption - This option should not be accepted by Clang.
+def NoClangOption : OptionFlag;

 `NoClangOption` ? Is this a Typo, or am I missing the intent behind this ?



Comment at: clang/lib/Driver/Driver.cpp:1579
 
-  if (IsFlangMode())
+  if (IsFlangMode()) {
 IncludedFlagsBitmask |= options::FlangOption;

NIT: May choose to avoid trivial braces. 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements



Comment at: flang/include/flang/Frontend/CompilerInstance.h:136
+  /// Add an output file onto the list of tracked output files.
+  ///
+  /// \param outFile - The output file info.

NIT: Blank line ?



Comment at: flang/include/flang/Frontend/CompilerInstance.h:230
+
+  // Allow the frontend compiler to write in the output stream
+  void WriteOutputStream(const std::string &message) {

NIT: Please finish the comment with a period.



Comment at: flang/lib/Frontend/CompilerInstance.cpp:66
+
+  // Create the name of the output file
+  if (!outputPath.empty()) {

NIT: Missing period at the end ?



Comment at: flang/lib/Frontend/CompilerInstance.cpp:67
+  // Create the name of the output file
+  if (!outputPath.empty()) {
+outFile = std::string(outputPath);

Can this be simplified ? Maybe a switch case ?



Comment at: flang/lib/Frontend/FrontendActions.cpp:25
+
+  // Set/store input file info into compiler instace
+  CompilerInstance &ci = GetCompilerInstance();

NIT: Instance ? and period at end ?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87989

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

[PATCH] D89307: [clangd] Disable extract variable for RHS of assignments

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: adamcz.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89307

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -215,26 +215,26 @@
 int x = [[1]], y = [[a + 1]], a = [[1]], z = a + 1;
   // if without else
   if([[1]])
-a = [[1]];
+a = [[1]] + 1;
   // if with else
   if(a < [[3]])
 if(a == [[4]])
-  a = [[5]];
+  a = [[5]] + 1;
 else
-  a = [[5]];
+  a = [[5]] + 1;
   else if (a < [[4]])
-a = [[4]];
+a = [[4]] + 1;
   else
-a = [[5]];
+a = [[5]] + 1;
   // for loop
-  for(a = [[1]]; a > 3]] + [[4; a++)
-a = [[2]];
+  for(a = [[1]] + 1; a > 3]] + [[4; a++)
+a = [[2]] + 1;
   // while
   while(a < [[1]])
-a = [[1]];
+a = [[1]] + 1;
   // do while
   do
-a = [[1]];
+a = [[1]] + 1;
   while(a < [[3]]);
 }
   )cpp";
@@ -291,6 +291,7 @@
   xyz([[a *= 5]]);
   // Variable DeclRefExpr
   a = [[b]];
+  a = [[xyz()]];
   // statement expression
   [[xyz()]];
   while (a)
@@ -373,10 +374,10 @@
  })cpp"},
   // attribute testing
   {R"cpp(void f(int a) {
-[ [gsl::suppress("type")] ] for (;;) a = [[1]];
+[ [gsl::suppress("type")] ] for (;;) a = [[1]] + 1;
  })cpp",
R"cpp(void f(int a) {
-auto dummy = 1; [ [gsl::suppress("type")] ] for (;;) a = 
dummy;
+auto dummy = 1; [ [gsl::suppress("type")] ] for (;;) a = 
dummy + 1;
  })cpp"},
   // MemberExpr
   {R"cpp(class T {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -382,17 +382,27 @@
   if (BinOp.parse(*N) && BinaryOperator::isAssignmentOp(BinOp.Kind))
 return false;
 
+  const SelectionTree::Node &OuterImplicit = N->outerImplicit();
+  const auto *Parent = OuterImplicit.Parent;
+  if (!Parent)
+return false;
   // We don't want to extract expressions used as statements, that would leave
   // a `dummy;` around that has no effect.
   // Unfortunately because the AST doesn't have ExprStmt, we have to check in
   // this roundabout way.
-  const SelectionTree::Node &OuterImplicit = N->outerImplicit();
-  if (!OuterImplicit.Parent ||
-  childExprIsStmt(OuterImplicit.Parent->ASTNode.get(),
+  if (childExprIsStmt(Parent->ASTNode.get(),
   OuterImplicit.ASTNode.get()))
 return false;
 
-  // FIXME: ban extracting the RHS of an assignment: `a = [[foo()]]`
+  // Disable extraction of full RHS on assignment operations, e.g:
+  // auto x = [[RHS_EXPR]];
+  // This would just result in duplicating the code.
+  if (const auto *BO = Parent->ASTNode.get()) {
+if (BO->isAssignmentOp() &&
+BO->getRHS() == OuterImplicit.ASTNode.get())
+  return false;
+  }
+
   return true;
 }
 


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -215,26 +215,26 @@
 int x = [[1]], y = [[a + 1]], a = [[1]], z = a + 1;
   // if without else
   if([[1]])
-a = [[1]];
+a = [[1]] + 1;
   // if with else
   if(a < [[3]])
 if(a == [[4]])
-  a = [[5]];
+  a = [[5]] + 1;
 else
-  a = [[5]];
+  a = [[5]] + 1;
   else if (a < [[4]])
-a = [[4]];
+a = [[4]] + 1;
   else
-a = [[5]];
+a = [[5]] + 1;
   // for loop
-  for(a = [[1]]; a > 3]] + [[4; a++)
-a = [[2]];
+  for(a = [[1]] + 1; a > 3]] + [[4; a++)
+a = [[2]] + 1;
   // while
   while(a < [[1]])
-a = [[1]];
+a = [[1]] + 1;
   // do while
   do
-a = [[1]];
+a = [[1]] + 1;
   while(a < [[3]]);
 }
   )cpp";
@@ -291,6 +291,7 @@
   xyz([[a *= 5]]);
   // Variable DeclRefExpr
   a = [[b]];
+  a = [[xyz()]];
   // statement expression
   [[xyz()]];
   while (a)
@@ -373,10 +374,10 @@
   

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

Hi,

Thank you for modifying the implementation to facilitate the extension to other 
targets.

Please add `libmvec`-specific tests is `clang/Driver/fveclib.c`, and simplify 
the loop-vectorize tests.

Other than that, I only have minor comments.

Francesco




Comment at: clang/lib/CodeGen/BackendUtil.cpp:377
+  default:
+   break;
+  case llvm::Triple::x86_64:

Nit: is this misaligned?



Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -vector-library=LIBMVEC-X86 -inject-tli-mappings -loop-vectorize -S 
< %s | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"

`-inject-tli-mappings` is not required here, as a pass itself is required by 
the loop vectorizer.



Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:6
+
+declare float @__expf_finite(float) #0
+

Nit: it is standard practice to put all declarations at the end of the file.



Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:9
+define void @exp_f32(float* nocapture %varray) {
+; CHECK-LABEL: @exp_f32(
+; CHECK-NEXT:  entry:

I think you are over-testing here. It is enough to check that inside the vector 
body there is a call to the vector function you have listed in the mapping. You 
are not checking for the whole auto-vectorization process here, just the 
vectorization of the function call. Same for all the tests for this patch in 
which you are doing something similar to this one test.

```
; CHECK-LABEL: @exp_f32(
; CHECK-LABEL:   vector.body:
; CHECK: call fast <4 x float> @_ZGVbN4v___expf_finite(<4 x float> 
```



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

https://reviews.llvm.org/D88154

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

[clang] 9fa7f48 - [Fixed Point] Add fixed-point to floating point cast types and consteval.

[Bevin Hansson via cfe-commits]

Author: Bevin Hansson
Date: 2020-10-13T13:26:56+02:00
New Revision: 9fa7f48459761fa13205f4c931484b0977c35746

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

LOG: [Fixed Point] Add fixed-point to floating point cast types and consteval.

Reviewed By: leonardchan

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

Added: 


Modified: 
clang/include/clang/AST/OperationKinds.def
clang/lib/AST/Expr.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGExprComplex.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/Edit/RewriteObjCFoundationAPI.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
clang/test/Frontend/fixed_point_conversions_const.c
clang/test/Frontend/fixed_point_errors.c
clang/test/Frontend/fixed_point_unknown_conversions.c

Removed: 




diff  --git a/clang/include/clang/AST/OperationKinds.def 
b/clang/include/clang/AST/OperationKinds.def
index f29664e8eb33..6daab1ffcb0a 100644
--- a/clang/include/clang/AST/OperationKinds.def
+++ b/clang/include/clang/AST/OperationKinds.def
@@ -201,6 +201,14 @@ CAST_OPERATION(IntegralToBoolean)
 ///float f = i;
 CAST_OPERATION(IntegralToFloating)
 
+/// CK_FloatingToFixedPoint - Floating to fixed point.
+///_Accum a = f;
+CAST_OPERATION(FloatingToFixedPoint)
+
+/// CK_FixedPointToFloating - Fixed point to floating.
+///(float) 2.5k
+CAST_OPERATION(FixedPointToFloating)
+
 /// CK_FixedPointCast - Fixed point to fixed point.
 ///(_Accum) 0.5r
 CAST_OPERATION(FixedPointCast)

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 8e8dd75e975a..919d3220875c 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1691,6 +1691,8 @@ bool CastExpr::CastConsistency() const {
   case CK_ARCExtendBlockObject:
   case CK_ZeroToOCLOpaqueType:
   case CK_IntToOCLSampler:
+  case CK_FloatingToFixedPoint:
+  case CK_FixedPointToFloating:
   case CK_FixedPointCast:
   case CK_FixedPointToIntegral:
   case CK_IntegralToFixedPoint:

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 639a5733b34b..1327aa6876e4 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -12896,6 +12896,8 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) 
{
   case CK_NonAtomicToAtomic:
   case CK_AddressSpaceConversion:
   case CK_IntToOCLSampler:
+  case CK_FloatingToFixedPoint:
+  case CK_FixedPointToFloating:
   case CK_FixedPointCast:
   case CK_IntegralToFixedPoint:
 llvm_unreachable("invalid cast kind for integral value");
@@ -13140,6 +13142,26 @@ bool FixedPointExprEvaluator::VisitCastExpr(const 
CastExpr *E) {
 
 return Success(IntResult, E);
   }
+  case CK_FloatingToFixedPoint: {
+APFloat Src(0.0);
+if (!EvaluateFloat(SubExpr, Src, Info))
+  return false;
+
+bool Overflowed;
+APFixedPoint Result = APFixedPoint::getFromFloatValue(
+Src, Info.Ctx.getFixedPointSemantics(DestType), &Overflowed);
+
+if (Overflowed) {
+  if (Info.checkingForUndefinedBehavior())
+Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
+ 
diag::warn_fixedpoint_constant_overflow)
+  << Result.toString() << E->getType();
+  else if (!HandleOverflow(Info, E, Result, E->getType()))
+return false;
+}
+
+return Success(Result, E);
+  }
   case CK_NoOp:
   case CK_LValueToRValue:
 return ExprEvaluatorBaseTy::VisitCastExpr(E);
@@ -13446,6 +13468,15 @@ bool FloatExprEvaluator::VisitCastExpr(const CastExpr 
*E) {
 E->getType(), Result);
   }
 
+  case CK_FixedPointToFloating: {
+APFixedPoint 
FixResult(Info.Ctx.getFixedPointSemantics(SubExpr->getType()));
+if (!EvaluateFixedPoint(SubExpr, FixResult, Info))
+  return false;
+Result =
+FixResult.convertToFloat(Info.Ctx.getFloatTypeSemantics(E->getType()));
+return true;
+  }
+
   case CK_FloatingCast: {
 if (!Visit(SubExpr))
   return false;
@@ -13591,6 +13622,8 @@ bool ComplexExprEvaluator::VisitCastExpr(const CastExpr 
*E) {
   case CK_NonAtomicToAtomic:
   case CK_AddressSpaceConversion:
   case CK_IntToOCLSampler:
+  case CK_FloatingToFixedPoint:
+  case CK_FixedPointToFloating:
   case CK_FixedPointCast:
   case CK_FixedPointToBoolean:
   case CK_FixedPointToIntegral:

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 869bace18ffc..2f54097d9209 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -4593,6 +4593,8 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) 
{
   case CK_ARCExtendBlockObject:
   case CK_Co

[clang] 101309f - [AST] Change return type of getTypeInfoInChars to a proper struct instead of std::pair.

[Bevin Hansson via cfe-commits]

Author: Bevin Hansson
Date: 2020-10-13T13:26:56+02:00
New Revision: 101309fe048e66873cfd972c47c4b7e7f2b99f41

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

LOG: [AST] Change return type of getTypeInfoInChars to a proper struct instead 
of std::pair.

Followup to D85191.

This changes getTypeInfoInChars to return a TypeInfoChars
struct instead of a std::pair of CharUnits. This lets the
interface match getTypeInfo more closely.

Reviewed By: efriedma

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

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/CodeGen/CGAtomic.cpp
clang/lib/CodeGen/CGBlocks.cpp
clang/lib/CodeGen/CGCUDANV.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGObjC.cpp
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
clang/lib/CodeGen/CGValue.h
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index e261e29036e9..3f4079e2569b 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -171,6 +171,16 @@ struct TypeInfo {
   : Width(Width), Align(Align), AlignIsRequired(AlignIsRequired) {}
 };
 
+struct TypeInfoChars {
+  CharUnits Width;
+  CharUnits Align;
+  bool AlignIsRequired : 1;
+
+  TypeInfoChars() : AlignIsRequired(false) {}
+  TypeInfoChars(CharUnits Width, CharUnits Align, bool AlignIsRequired)
+  : Width(Width), Align(Align), AlignIsRequired(AlignIsRequired) {}
+};
+
 /// Holds long-lived AST nodes (such as types and decls) that can be
 /// referred to throughout the semantic analysis of a file.
 class ASTContext : public RefCountedBase {
@@ -2169,10 +2179,10 @@ class ASTContext : public RefCountedBase {
 
   // getTypeInfoDataSizeInChars - Return the size of a type, in chars. If the
   // type is a record, its data size is returned.
-  std::pair getTypeInfoDataSizeInChars(QualType T) const;
+  TypeInfoChars getTypeInfoDataSizeInChars(QualType T) const;
 
-  std::pair getTypeInfoInChars(const Type *T) const;
-  std::pair getTypeInfoInChars(QualType T) const;
+  TypeInfoChars getTypeInfoInChars(const Type *T) const;
+  TypeInfoChars getTypeInfoInChars(QualType T) const;
 
   /// Determine if the alignment the type has was required using an
   /// alignment attribute.

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a82d95461bb9..8caff6b33379 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1777,9 +1777,8 @@ CharUnits ASTContext::getExnObjectAlignment() const {
 // chars. If the type is a record, its data size is returned.  This is
 // the size of the memcpy that's performed when assigning this type
 // using a trivial copy/move assignment operator.
-std::pair
-ASTContext::getTypeInfoDataSizeInChars(QualType T) const {
-  std::pair sizeAndAlign = getTypeInfoInChars(T);
+TypeInfoChars ASTContext::getTypeInfoDataSizeInChars(QualType T) const {
+  TypeInfoChars Info = getTypeInfoInChars(T);
 
   // In C++, objects can sometimes be allocated into the tail padding
   // of a base-class subobject.  We decide whether that's possible
@@ -1787,44 +1786,43 @@ ASTContext::getTypeInfoDataSizeInChars(QualType T) 
const {
   if (getLangOpts().CPlusPlus) {
 if (const auto *RT = T->getAs()) {
   const ASTRecordLayout &layout = getASTRecordLayout(RT->getDecl());
-  sizeAndAlign.first = layout.getDataSize();
+  Info.Width = layout.getDataSize();
 }
   }
 
-  return sizeAndAlign;
+  return Info;
 }
 
 /// getConstantArrayInfoInChars - Performing the computation in CharUnits
 /// instead of in bits prevents overflowing the uint64_t for some large arrays.
-std::pair
+TypeInfoChars
 static getConstantArrayInfoInChars(const ASTContext &Context,
const ConstantArrayType *CAT) {
-  std::pair EltInfo =
-  Context.getTypeInfoInChars(CAT->getElementType());
+  TypeInfoChars EltInfo = Context.getTypeInfoInChars(CAT->getElementType());
   uint64_t Size = CAT->getSize().getZExtValue();
-  assert((Size == 0 || static_cast(EltInfo.first.getQuantity()) <=
+  assert((Size == 0 || static_cast(EltInfo.Width.getQuantity()) <=
   (uint64_t)(-1)/Size) &&
  "Overflow in array type char size evaluation");
-  uint64_t Width = EltInfo.first.getQuantity() * Size;
-  unsigned Align = EltInfo.second.getQuantity();
+  uint64_t Width = EltInfo.Width.getQuantity() * Size;
+  unsigned Align = EltInfo.Align.getQuantity();
   if (!Context.getTargetInfo().getCXXABI().isMicrosoft() ||

[PATCH] D86447: [AST] Change return type of getTypeInfoInChars to a proper struct instead of std::pair.

[Bevin Hansson 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 rG101309fe048e: [AST] Change return type of getTypeInfoInChars 
to a proper struct instead of… (authored by ebevhan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86447

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
  clang/lib/CodeGen/CGValue.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
@@ -248,8 +248,9 @@
   FieldInfo RetVal;
   RetVal.Field = FD;
   auto &Ctx = FD->getASTContext();
-  std::tie(RetVal.Size, RetVal.Align) =
-  Ctx.getTypeInfoInChars(FD->getType());
+  auto Info = Ctx.getTypeInfoInChars(FD->getType());
+  RetVal.Size = Info.Width;
+  RetVal.Align = Info.Align;
   assert(llvm::isPowerOf2_64(RetVal.Align.getQuantity()));
   if (auto Max = FD->getMaxAlignment())
 RetVal.Align = std::max(Ctx.toCharUnitsFromBits(Max), RetVal.Align);
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -359,7 +359,7 @@
 ///   leaving one or more empty slots behind as padding.
 static Address emitVoidPtrVAArg(CodeGenFunction &CGF, Address VAListAddr,
 QualType ValueTy, bool IsIndirect,
-std::pair ValueInfo,
+TypeInfoChars ValueInfo,
 CharUnits SlotSizeAndAlign,
 bool AllowHigherAlign) {
   // The size and alignment of the value that was passed directly.
@@ -368,8 +368,8 @@
 DirectSize = CGF.getPointerSize();
 DirectAlign = CGF.getPointerAlign();
   } else {
-DirectSize = ValueInfo.first;
-DirectAlign = ValueInfo.second;
+DirectSize = ValueInfo.Width;
+DirectAlign = ValueInfo.Align;
   }
 
   // Cast the address we've calculated to the right type.
@@ -383,7 +383,7 @@
 AllowHigherAlign);
 
   if (IsIndirect) {
-Addr = Address(CGF.Builder.CreateLoad(Addr), ValueInfo.second);
+Addr = Address(CGF.Builder.CreateLoad(Addr), ValueInfo.Align);
   }
 
   return Addr;
@@ -656,7 +656,7 @@
 "Unexpected IndirectRealign seen in arginfo in generic VAArg emitter!");
 
 auto TyInfo = CGF.getContext().getTypeInfoInChars(Ty);
-CharUnits TyAlignForABI = TyInfo.second;
+CharUnits TyAlignForABI = TyInfo.Align;
 
 llvm::Type *BaseTy =
 llvm::PointerType::getUnqual(CGF.ConvertTypeForMem(Ty));
@@ -2062,8 +2062,8 @@
   //
   // Just messing with TypeInfo like this works because we never pass
   // anything indirectly.
-  TypeInfo.second = CharUnits::fromQuantity(
-getTypeStackAlignInBytes(Ty, TypeInfo.second.getQuantity()));
+  TypeInfo.Align = CharUnits::fromQuantity(
+getTypeStackAlignInBytes(Ty, TypeInfo.Align.getQuantity()));
 
   return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*Indirect*/ false,
   TypeInfo, CharUnits::fromQuantity(4),
@@ -4067,10 +4067,9 @@
 RegAddr = CGF.Builder.CreateElementBitCast(RegAddr, LTy);
 
 // Copy to a temporary if necessary to ensure the appropriate alignment.
-std::pair SizeAlign =
-getContext().getTypeInfoInChars(Ty);
-uint64_t TySize = SizeAlign.first.getQuantity();
-CharUnits TyAlign = SizeAlign.second;
+auto TInfo = getContext().getTypeInfoInChars(Ty);
+uint64_t TySize = TInfo.Width.getQuantity();
+CharUnits TyAlign = TInfo.Align;
 
 // Copy into a temporary if the type is more aligned than the
 // register save area.
@@ -4573,7 +4572,7 @@
 llvm::report_fatal_error("vector type is not supported on AIX yet");
 
   auto TypeInfo = getContext().getTypeInfoInChars(Ty);
-  TypeInfo.second = getParamTypeAlignment(Ty);
+  TypeInfo.Align = getParamTypeAlignment(Ty);
 
   CharUnits SlotSize = CharUnits::fromQuantity(PtrByteSize);
 
@@ -4692,7 +4691,7 @@
   QualType Ty) const {
   if (getTarget().getTriple().isOSDarwin()) {
 auto TI = getContext().getTypeInfoInChars(Ty);
-TI.second = getParamTypeAlignment(Ty);
+TI.Align = getParamTypeAlignment(Ty);
 
 CharUnits Sl

[PATCH] D86631: [Fixed Point] Add fixed-point to floating point cast types and consteval.

[Bevin Hansson 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 rG9fa7f4845976: [Fixed Point] Add fixed-point to floating 
point cast types and consteval. (authored by ebevhan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86631

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Edit/RewriteObjCFoundationAPI.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/Frontend/fixed_point_conversions_const.c
  clang/test/Frontend/fixed_point_errors.c
  clang/test/Frontend/fixed_point_unknown_conversions.c

Index: clang/test/Frontend/fixed_point_unknown_conversions.c
===
--- clang/test/Frontend/fixed_point_unknown_conversions.c
+++ clang/test/Frontend/fixed_point_unknown_conversions.c
@@ -22,16 +22,12 @@
   _Fract fract = accum; // ok
   _Accum *accum_ptr;
 
-  accum = f;   // expected-error{{conversion between fixed point and 'float' is not yet supported}}
-  accum = d;   // expected-error{{conversion between fixed point and 'double' is not yet supported}}
   accum = dc;  // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
   accum = ic;  // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
   accum = s;   // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
   accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
   accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
 
-  f = accum;   // expected-error{{conversion between fixed point and 'float' is not yet supported}}
-  d = accum;   // expected-error{{conversion between fixed point and 'double' is not yet supported}}
   dc = accum;  // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
   ic = accum;  // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
   s = accum;   // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
Index: clang/test/Frontend/fixed_point_errors.c
===
--- clang/test/Frontend/fixed_point_errors.c
+++ clang/test/Frontend/fixed_point_errors.c
@@ -286,8 +286,3 @@
 
 // Division by zero
 short _Accum div_zero = 4.5k / 0.0lr;  // expected-error {{initializer element is not a compile-time constant}}
-
-void foo(void) {
-  _Accum x = 0.5k;
-  if (x == 0.5) {} // expected-error{{invalid operands to binary expression ('_Accum' and 'double')}}
-}
Index: clang/test/Frontend/fixed_point_conversions_const.c
===
--- clang/test/Frontend/fixed_point_conversions_const.c
+++ clang/test/Frontend/fixed_point_conversions_const.c
@@ -43,6 +43,37 @@
 short _Accum sa_const7 = -256;
 // CHECK-DAG: @sa_const7 = {{.*}}global i16 -32768, align 2
 
+// Fixed point to floating point
+float fl_const = 1.0hk;
+// CHECK-DAG: @fl_const = {{.*}}global float 1.00e+00, align 4
+float fl_const2 = -128.0k;
+// CHECK-DAG: @fl_const2 = {{.*}}global float -1.28e+02, align 4
+float fl_const3 = 0.0872802734375k;
+// CHECK-DAG: @fl_const3 = {{.*}}global float 0x3FB65800, align 4
+float fl_const4 = 192.5k;
+// CHECK-DAG: @fl_const4 = {{.*}}global float 1.925000e+02, align 4
+float fl_const5 = -192.5k;
+// CHECK-DAG: @fl_const5 = {{.*}}global float -1.925000e+02, align 4
+
+// Floating point to fixed point
+_Accum a_fl_const = 1.0f;
+// CHECK-DAG: @a_fl_const = {{.*}}global i32 32768, align 4
+_Accum a_fl_const2 = -128.0f;
+// CHECK-DAG: @a_fl_const2 = {{.*}}global i32 -4194304, align 4
+_Accum a_fl_const3 = 0.0872802734375f;
+// CHECK-DAG: @a_fl_const3 = {{.*}}global i32 2860, align 4
+_Accum a_fl_const4 = 0.0872802734375;
+// CHECK-DAG: @a_fl_const4 = {{.*}}global i32 2860, align 4
+_Accum a_fl_const5 = -0.0872802734375f;
+// CHECK-DAG: @a_fl_const5 = {{.*}}global i32 -2860, align 4
+_Fract f_fl_const = 0.5f;
+// CHECK-DAG: @f_fl_const = {{.*}}global i16 16384, align 2
+_Fract f_fl_const2 = -0.75;
+// CHECK-DAG: @f_fl_const2 = {{.*}}global i16 -24576, align 2
+unsigned short _Accum usa_fl_const = 48.75f;
+// SIGNED-DAG: @usa_fl_const = {{.*}}global i16 12480, align 2
+// UNSIGNED-DAG: @usa_fl_const = {{.*}}global i16 6240, align 2
+
 // Signedness
 unsigned short _Accum usa_const2 = 2.5hk;
 // SIGNED-DAG: @usa_const2  = {{.*}}global i16 640, align 2
Index: clang/lib/StaticAnalyzer/Core/ExprEngine

[PATCH] D89307: [clangd] Disable extract variable for RHS of assignments

[Adam Czachorowski via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:400
+  // This would just result in duplicating the code.
+  if (const auto *BO = Parent->ASTNode.get()) {
+if (BO->isAssignmentOp() &&

nit: you can fold the two if-s together.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89307

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

[clang-tools-extra] 002968a - [clang-tidy] Add an example for misc-unused-alias-decls

[Sylvestre Ledru via cfe-commits]

Author: Sylvestre Ledru
Date: 2020-10-13T13:56:04+02:00
New Revision: 002968a320463237f0ccea754a7482b229e34cf1

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

LOG: [clang-tidy] Add an example for misc-unused-alias-decls

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

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst 
b/clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
index d0e8c7188a3f..1b0a0cde059b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
@@ -5,3 +5,10 @@ misc-unused-alias-decls
 
 
 Finds unused namespace alias declarations.
+
+.. code-block:: c++
+
+  namespace my_namespace {
+  class C {};
+  }
+  namespace unused_alias = ::my_namespace;



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

[PATCH] D89270: [clang-tidy] Add an example for misc-unused-alias-decls

[Sylvestre Ledru 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 rG002968a32046: [clang-tidy] Add an example for 
misc-unused-alias-decls (authored by sylvestre.ledru).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89270

Files:
  clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst


Index: clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
@@ -5,3 +5,10 @@
 
 
 Finds unused namespace alias declarations.
+
+.. code-block:: c++
+
+  namespace my_namespace {
+  class C {};
+  }
+  namespace unused_alias = ::my_namespace;


Index: clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc-unused-alias-decls.rst
@@ -5,3 +5,10 @@
 
 
 Finds unused namespace alias declarations.
+
+.. code-block:: c++
+
+  namespace my_namespace {
+  class C {};
+  }
+  namespace unused_alias = ::my_namespace;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 297828.
balazske added a comment.

Removed C++ support.
Other small changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87449

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp
  clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-sig30-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cert-sig30-c_cpp.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stdlib.h
  clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.c

Index: clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-sig30-c.c
@@ -0,0 +1,81 @@
+// RUN: %check_clang_tidy %s cert-sig30-c %t -- -- -isystem %S/Inputs/Headers
+
+// The function should be classified as system call even if there is a
+// declaration before or after the one in a system header.
+int printf(const char *, ...);
+
+typedef void (*sighandler_t)(int);
+sighandler_t signal(int signum, sighandler_t handler);
+
+#include "signal.h"
+#include "stdio.h"
+#include "stdlib.h"
+
+int printf(const char *, ...);
+
+sighandler_t signal(int signum, sighandler_t handler);
+
+void handler_abort(int) {
+  abort();
+}
+
+void handler__Exit(int) {
+  _Exit(0);
+}
+
+void handler_quick_exit(int) {
+  quick_exit(0);
+}
+
+void handler_other(int) {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [cert-sig30-c]
+}
+
+void handler_signal(int) {
+  // FIXME: It is only OK to call signal with the current signal number.
+  signal(0, SIG_DFL);
+}
+
+void f_ok() {
+  abort();
+}
+
+void f_bad() {
+  printf("1234");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'printf' may not be asynchronous-safe; calling it from a signal handler may be dangerous [cert-sig30-c]
+}
+
+void f_extern();
+
+void handler_ok(int) {
+  f_ok();
+}
+
+void handler_bad(int) {
+  f_bad();
+}
+
+void handler_extern(int) {
+  f_extern();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'f_extern' may not be asynchronous-safe; calling it from a signal handler may be dangerous [cert-sig30-c]
+}
+
+void test() {
+  signal(SIGINT, handler_abort);
+  signal(SIGINT, handler__Exit);
+  signal(SIGINT, handler_quick_exit);
+  signal(SIGINT, handler_signal);
+  signal(SIGINT, handler_other);
+
+  signal(SIGINT, handler_ok);
+  signal(SIGINT, handler_bad);
+  signal(SIGINT, handler_extern);
+
+  signal(SIGINT, quick_exit);
+  signal(SIGINT, other_call);
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'other_call' may not be asynchronous-safe; calling it from a signal handler may be dangerous [cert-sig30-c]
+
+  signal(SIGINT, SIG_IGN);
+  signal(SIGINT, SIG_DFL);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stdlib.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/stdlib.h
@@ -0,0 +1,18 @@
+//===--- stdlib.h - Stub header for tests ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef _STDLIB_H_
+#define _STDLIB_H_
+
+void abort(void);
+void _Exit(int __status);
+void quick_exit(int __status);
+
+void other_call(int);
+
+#endif // _STDLIB_H_
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/signal.h
@@ -0,0 +1,22 @@
+//===--- signal.h - Stub header for tests ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef _SIGNAL_H_
+#define _SIGNAL_H_
+
+void _sig_ign(int);
+void _sig_dfl(int);
+
+#define SIGINT 1
+#define SIG_IGN _sig_ign
+#define SIG_DFL _sig_dfl
+
+typedef void (*sighandler_t)(int);
+sighandler_t signal(int signum, sighandler_t handler);
+
+#endif // _SIGNAL_H_
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cert-sig30-c_cpp.h
==

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

[Denys Petrov via Phabricator via cfe-commits]

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

Updated. Removed a new test file, moved the test to an existing file instead.


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

https://reviews.llvm.org/D89055

Files:
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/casts.c
  clang/test/Analysis/string.c


Index: clang/test/Analysis/string.c
===
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -363,6 +363,14 @@
 strcpy(x, y); // no-warning
 }
 
+// PR37503
+void *get_void_ptr();
+char ***type_punned_ptr;
+void strcpy_no_assertion(char c) {
+  *(unsigned char **)type_punned_ptr = (unsigned char *)(get_void_ptr());
+  strcpy(**type_punned_ptr, &c); // no-crash
+}
+
 //===--===
 // stpcpy()
 //===--===
Index: clang/test/Analysis/casts.c
===
--- clang/test/Analysis/casts.c
+++ clang/test/Analysis/casts.c
@@ -245,3 +245,8 @@
   return a * a;
 }
 
+void no_crash_reinterpret_char_as_uchar(char ***a, int *b) {
+  *(unsigned char **)a = (unsigned char *)b;
+  if (**a == 0) // no-crash
+;
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -426,12 +426,17 @@
   // We might need to do that for non-void pointers as well.
   // FIXME: We really need a single good function to perform casts for us
   // correctly every time we need it.
-  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+  if (castTy->isPointerType() && !castTy->isVoidPointerType()) {
 if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) {
   QualType sr = SR->getSymbol()->getType();
   if (!hasSameUnqualifiedPointeeType(sr, castTy))
-  return loc::MemRegionVal(castRegion(SR, castTy));
+return loc::MemRegionVal(castRegion(SR, castTy));
 }
+// Next fixes pointer dereference using type different from its initial 
one.
+// See PR37503 for details
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  return loc::MemRegionVal(castRegion(SR, castTy));
+  }
 
   return svalBuilder.dispatchCast(V, castTy);
 }


Index: clang/test/Analysis/string.c
===
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -363,6 +363,14 @@
 strcpy(x, y); // no-warning
 }
 
+// PR37503
+void *get_void_ptr();
+char ***type_punned_ptr;
+void strcpy_no_assertion(char c) {
+  *(unsigned char **)type_punned_ptr = (unsigned char *)(get_void_ptr());
+  strcpy(**type_punned_ptr, &c); // no-crash
+}
+
 //===--===
 // stpcpy()
 //===--===
Index: clang/test/Analysis/casts.c
===
--- clang/test/Analysis/casts.c
+++ clang/test/Analysis/casts.c
@@ -245,3 +245,8 @@
   return a * a;
 }
 
+void no_crash_reinterpret_char_as_uchar(char ***a, int *b) {
+  *(unsigned char **)a = (unsigned char *)b;
+  if (**a == 0) // no-crash
+;
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -426,12 +426,17 @@
   // We might need to do that for non-void pointers as well.
   // FIXME: We really need a single good function to perform casts for us
   // correctly every time we need it.
-  if (castTy->isPointerType() && !castTy->isVoidPointerType())
+  if (castTy->isPointerType() && !castTy->isVoidPointerType()) {
 if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) {
   QualType sr = SR->getSymbol()->getType();
   if (!hasSameUnqualifiedPointeeType(sr, castTy))
-  return loc::MemRegionVal(castRegion(SR, castTy));
+return loc::MemRegionVal(castRegion(SR, castTy));
 }
+// Next fixes pointer dereference using type different from its initial one.
+// See PR37503 for details
+if (const auto *SR = dyn_cast_or_null(V.getAsRegion()))
+  return loc::MemRegionVal(castRegion(SR, castTy));
+  }
 
   return svalBuilder.dispatchCast(V, castTy);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89309: [ThinLTO] In documentation, mention possible value for concurrency flags

[Alexandre Ganea via Phabricator via cfe-commits]

aganea created this revision.
aganea added reviewers: thakis, hans.
Herald added subscribers: cfe-commits, dexonsmith, steven_wu, hiraditya, 
inglorion.
Herald added a project: clang.
aganea requested review of this revision.

As suggested here: https://reviews.llvm.org/D75153#2323939


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89309

Files:
  clang/docs/ThinLTO.rst


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -123,6 +123,11 @@
 - lld-link:
   ``/opt:lldltojobs=N``
 
+Other possible values for ``N`` are:
+- 0 : only use one hyper-thread per core on all CPU sockets (default).
+- 1 : disable multi-threading.
+- all : use all hyper-threads on all CPU sockets.
+
 Incremental
 ---
 .. _incremental:


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -123,6 +123,11 @@
 - lld-link:
   ``/opt:lldltojobs=N``
 
+Other possible values for ``N`` are:
+- 0 : only use one hyper-thread per core on all CPU sockets (default).
+- 1 : disable multi-threading.
+- all : use all hyper-threads on all CPU sockets.
+
 Incremental
 ---
 .. _incremental:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] bddef54 - Raise the timeout in DirectoryWatcherTest to 10 s

[Hans Wennborg via cfe-commits]

Author: Hans Wennborg
Date: 2020-10-13T14:25:25+02:00
New Revision: bddef54c502811fa1406d1161d4baa15b56ebc32

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

LOG: Raise the timeout in DirectoryWatcherTest to 10 s

After D88666, which implemented DirectoryWatcher on Windows, we're
seeing test failures on Chromium's Windows bots.

Try raising the timeout in case the test is failing due to high load on
the machine.

Added: 


Modified: 
clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp

Removed: 




diff  --git a/clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp 
b/clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
index 650c0fc49764..4d4a4614740d 100644
--- a/clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ b/clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -243,7 +243,7 @@ void checkEventualResultWithTimeout(VerifyingConsumer 
&TestConsumer) {
   std::thread worker(std::move(task));
   worker.detach();
 
-  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(3)) ==
+  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(10)) ==
   std::future_status::ready)
   << "The expected result state wasn't reached before the time-out.";
   std::unique_lock L(TestConsumer.Mtx);



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

[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

We're seeing the tests for this fail in Chromium's Clang builds: 
https://bugs.chromium.org/p/chromium/issues/detail?id=1137737

I'll try increasing the test's timeout for now and see if that helps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88666

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

[PATCH] D89312: [SVE][CodeGen]Use TypeSize comparison operators in clang

[Caroline via Phabricator via cfe-commits]

CarolineConcatto created this revision.
Herald added subscribers: cfe-commits, psnobl, tschuett.
Herald added a reviewer: efriedma.
Herald added a project: clang.
CarolineConcatto requested review of this revision.

The patch replaces comparison greater/lower or equal than by
TypeSize::isKnownXY, where XY is: GE, GT, LE, LT
in the place where getPrimitiveSizeInBits is being used inside Clang.
TypeSize::isKnowXY is flexible and allows to compares mixing fixed
 width and scalable vector types.

This patch excludes:

- comparison with constant as in:

clang/lib/CodeGen/TargetInfo.cpp:2333 and

- constant type size as in: clang/lib/CodeGen/CGExpr.cpp:2992.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89312

Files:
  clang/lib/CodeGen/CGBuiltin.cpp


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -5599,8 +5599,8 @@
 
   Value *Result = CGF.EmitNeonCall(F, Ops, s);
   llvm::Type *ResultType = CGF.ConvertType(E->getType());
-  if (ResultType->getPrimitiveSizeInBits() <
-  Result->getType()->getPrimitiveSizeInBits())
+  if (TypeSize::isKnownLT(ResultType->getPrimitiveSizeInBits(),
+  Result->getType()->getPrimitiveSizeInBits()))
 return CGF.Builder.CreateExtractElement(Result, C0);
 
   return CGF.Builder.CreateBitCast(Result, ResultType, s);


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -5599,8 +5599,8 @@
 
   Value *Result = CGF.EmitNeonCall(F, Ops, s);
   llvm::Type *ResultType = CGF.ConvertType(E->getType());
-  if (ResultType->getPrimitiveSizeInBits() <
-  Result->getType()->getPrimitiveSizeInBits())
+  if (TypeSize::isKnownLT(ResultType->getPrimitiveSizeInBits(),
+  Result->getType()->getPrimitiveSizeInBits()))
 return CGF.Builder.CreateExtractElement(Result, C0);
 
   return CGF.Builder.CreateBitCast(Result, ResultType, s);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D76620#2326551 , @rsmith wrote:

> In D76620#2324858 , @erichkeane 
> wrote:
>
>> The feature that this supports is a part of the SYCL 2020 Provisional Spec, 
>> I thought that was sufficient.  We'll look into an RFC to re-submit in the 
>> future.
>
> Does that cover only functionality that can be implemented in terms of this 
> builtin, or the builtin itself? If so, what is the functionality that needs 
> to be supported? That information will be a good starting point for the RFC.

Functionality that is implemented in terms of this builtin.  Basically: When 
compiling a SYCL program, we do so in 2 invocations, once for the Host and once 
for the Device.  The 'sycl_kernel' is the interface function that both 
invocations of the compiler need to know the name of.  For the most part, using 
the the mangled name of the functor would work, except when using lambdas, 
where the name is not sufficiently stable, and frequently changes across 
invocations (even further so when there is macros that introduce lambdas 
above/below it).

The earlier SYCL spec gets around this by requiring the user to specify a 
unique-name for the kernel name themselves, in a way that is otherwise unused, 
something like:
`kernel_call([](){});`

The 2020 spec supports removing the specific name (so that 
`kernel_call([](){})` works).  In order to do that, we need some interface that 
provides a name based on the functor that is both unique and stable. As I 
mentioned before, lambdas proved difficult, so we added a builtin to produce a 
string version of the name (that the host runtime can then use to lookup the 
kernel later), and the implementation of it in SEMA is used to generate the 
kernel's name.

> In D76620#2326499 , @keryell wrote:
>
>> Otherwise, just removing a feature used for almost 6 months will cause some 
>> kind of forking pain...
>
> Well, there have at least not been any LLVM releases with this feature yet. 
> (We're on the cusp of releasing LLVM 11, which would be the first release 
> with this functionality.)

In fairness, all of the downstreams that I'm aware for SYCL of are working off 
of trunk. For my two downstreams, this is only a mild inconvenience.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D89309: [ThinLTO] In documentation, mention possible values for concurrency flags

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Thanks for documenting these!

I wonder if including the "CPU sockets" part is more confusing than helpful 
(especially if the reader doesn't know about CPU sockets). The sockets don't 
really matter here anyway.

Sometimes the term "physical core" is used in relation to hyper-threading and I 
find that pretty understandable. How about something line:

- 0: Use one thread per physical core (default)
- 1: Use a single thread only (disable multi-threading)
- all: Use one thread per logical core (uses all hyper threads)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89309

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

[clang] c78da03 - [clang] Improve handling of physical registers in inline assembly operands.

[Jonas Paulsson via cfe-commits]

Author: Jonas Paulsson
Date: 2020-10-13T15:09:52+02:00
New Revision: c78da037783bda0f27f4d82060149166e6f0c796

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

LOG: [clang] Improve handling of physical registers in inline assembly operands.

Change EmitAsmStmt() to

- Not tie physregs with the "+r" constraint, but instead add the hard
  register as an input constraint. This makes "+r" and "=r":"r" look the same
  in the output.

  Background: Macro intensive user code may contain inline assembly
  statements with multiple operands constrained to the same physreg. Such a
  case (with the operand constraints "+r" : "r") currently triggers the
  TwoAddressInstructionPass assertion against any extra use of a tied
  register. Furthermore, TwoAddress will insert a COPY to that physreg even
  though isel has already done so (for the non-tied use), which may lead to a
  second redundant instruction currently. A simple fix for this is to not
  emit tied physreg uses in the first place for the "+r" constraint, which is
  what this patch does.

- Give an error on multiple outputs to the same physical register.

  This should be reported and this is also what GCC does.

Review: Ulrich Weigand, Aaron Ballman, Jennifer Yu, Craig Topper

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

Added: 
clang/test/CodeGen/systemz-inline-asm-02.c

Modified: 
clang/lib/CodeGen/CGStmt.cpp
clang/test/CodeGen/systemz-inline-asm.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index c9e6ce2df2c0..a69007e67b26 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -21,6 +21,7 @@
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"
@@ -1836,7 +1837,8 @@ SimplifyConstraint(const char *Constraint, const 
TargetInfo &Target,
 static std::string
 AddVariableConstraints(const std::string &Constraint, const Expr &AsmExpr,
const TargetInfo &Target, CodeGenModule &CGM,
-   const AsmStmt &Stmt, const bool EarlyClobber) {
+   const AsmStmt &Stmt, const bool EarlyClobber,
+   std::string *GCCReg = nullptr) {
   const DeclRefExpr *AsmDeclRef = dyn_cast(&AsmExpr);
   if (!AsmDeclRef)
 return Constraint;
@@ -1861,6 +1863,8 @@ AddVariableConstraints(const std::string &Constraint, 
const Expr &AsmExpr,
   }
   // Canonicalize the register here before returning it.
   Register = Target.getNormalizedGCCRegisterName(Register);
+  if (GCCReg != nullptr)
+*GCCReg = Register.str();
   return (EarlyClobber ? "&{" : "{") + Register.str() + "}";
 }
 
@@ -2059,6 +2063,9 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
   // Keep track of out constraints for tied input operand.
   std::vector OutputConstraints;
 
+  // Keep track of defined physregs.
+  llvm::SmallSet PhysRegOutputs;
+
   // An inline asm can be marked readonly if it meets the following conditions:
   //  - it doesn't have any sideeffects
   //  - it doesn't clobber memory
@@ -2078,9 +2085,15 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
 const Expr *OutExpr = S.getOutputExpr(i);
 OutExpr = OutExpr->IgnoreParenNoopCasts(getContext());
 
+std::string GCCReg;
 OutputConstraint = AddVariableConstraints(OutputConstraint, *OutExpr,
   getTarget(), CGM, S,
-  Info.earlyClobber());
+  Info.earlyClobber(),
+  &GCCReg);
+// Give an error on multiple outputs to same physreg.
+if (!GCCReg.empty() && !PhysRegOutputs.insert(GCCReg).second)
+  CGM.Error(S.getAsmLoc(), "multiple outputs to hard register: " + GCCReg);
+
 OutputConstraints.push_back(OutputConstraint);
 LValue Dest = EmitLValue(OutExpr);
 if (!Constraints.empty())
@@ -2167,7 +2180,8 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
 LargestVectorWidth =
 std::max((uint64_t)LargestVectorWidth,
  VT->getPrimitiveSizeInBits().getKnownMinSize());
-  if (Info.allowsRegister())
+  // Don't tie physregs.
+  if (Info.allowsRegister() && GCCReg.empty())
 InOutConstraints += llvm::utostr(i);
   else
 InOutConstraints += OutputConstraint;

diff  --git a/clang/test/CodeGen/systemz-inline-asm-02.c 
b/clang/test/CodeGen/systemz-inline-asm-02.c
new file mode 100644
index ..754d7e66f04b
--- /dev/null
+++ b/clang/test/CodeGen/systemz-inline-asm-02.c
@@ -0

[PATCH] D87279: [clang] Fix handling of physical registers in inline assembly operands.

[Jonas Paulsson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc78da037783b: [clang] Improve handling of physical registers 
in inline assembly operands. (authored by jonpa).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87279

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/CodeGen/systemz-inline-asm-02.c
  clang/test/CodeGen/systemz-inline-asm.c

Index: clang/test/CodeGen/systemz-inline-asm.c
===
--- clang/test/CodeGen/systemz-inline-asm.c
+++ clang/test/CodeGen/systemz-inline-asm.c
@@ -129,3 +129,17 @@
 // CHECK: [[RESULT:%.*]] = tail call fp128 asm "axbr $0, $2", "=f,0,f"(fp128 %f, fp128 %g)
 // CHECK: store fp128 [[RESULT]], fp128* [[DEST]]
 }
+
+// Test that there are no tied physreg uses. TwoAddress pass cannot deal with them.
+int test_physregs(void) {
+  // CHECK-LABEL: define signext i32 @test_physregs()
+  register int l __asm__("r7") = 0;
+
+  // CHECK: call i32 asm "lr $0, $1", "={r7},{r7}"
+  __asm__("lr %0, %1" : "+r"(l));
+
+  // CHECK: call i32 asm "$0 $1 $2", "={r7},{r7},{r7}"
+  __asm__("%0 %1 %2" : "+r"(l) : "r"(l));
+
+  return l;
+}
Index: clang/test/CodeGen/systemz-inline-asm-02.c
===
--- /dev/null
+++ clang/test/CodeGen/systemz-inline-asm-02.c
@@ -0,0 +1,13 @@
+// RUN: not %clang_cc1 -triple s390x-linux-gnu -O2 -emit-llvm -o - %s 2>&1 \
+// RUN:  | FileCheck %s
+// REQUIRES: systemz-registered-target
+
+// Test that an error is given if a physreg is defined by multiple operands.
+int test_physreg_defs(void) {
+  register int l __asm__("r7") = 0;
+
+  // CHECK: error: multiple outputs to hard register: r7
+  __asm__("" : "+r"(l), "=r"(l));
+
+  return l;
+}
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -21,6 +21,7 @@
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/InlineAsm.h"
@@ -1836,7 +1837,8 @@
 static std::string
 AddVariableConstraints(const std::string &Constraint, const Expr &AsmExpr,
const TargetInfo &Target, CodeGenModule &CGM,
-   const AsmStmt &Stmt, const bool EarlyClobber) {
+   const AsmStmt &Stmt, const bool EarlyClobber,
+   std::string *GCCReg = nullptr) {
   const DeclRefExpr *AsmDeclRef = dyn_cast(&AsmExpr);
   if (!AsmDeclRef)
 return Constraint;
@@ -1861,6 +1863,8 @@
   }
   // Canonicalize the register here before returning it.
   Register = Target.getNormalizedGCCRegisterName(Register);
+  if (GCCReg != nullptr)
+*GCCReg = Register.str();
   return (EarlyClobber ? "&{" : "{") + Register.str() + "}";
 }
 
@@ -2059,6 +2063,9 @@
   // Keep track of out constraints for tied input operand.
   std::vector OutputConstraints;
 
+  // Keep track of defined physregs.
+  llvm::SmallSet PhysRegOutputs;
+
   // An inline asm can be marked readonly if it meets the following conditions:
   //  - it doesn't have any sideeffects
   //  - it doesn't clobber memory
@@ -2078,9 +2085,15 @@
 const Expr *OutExpr = S.getOutputExpr(i);
 OutExpr = OutExpr->IgnoreParenNoopCasts(getContext());
 
+std::string GCCReg;
 OutputConstraint = AddVariableConstraints(OutputConstraint, *OutExpr,
   getTarget(), CGM, S,
-  Info.earlyClobber());
+  Info.earlyClobber(),
+  &GCCReg);
+// Give an error on multiple outputs to same physreg.
+if (!GCCReg.empty() && !PhysRegOutputs.insert(GCCReg).second)
+  CGM.Error(S.getAsmLoc(), "multiple outputs to hard register: " + GCCReg);
+
 OutputConstraints.push_back(OutputConstraint);
 LValue Dest = EmitLValue(OutExpr);
 if (!Constraints.empty())
@@ -2167,7 +2180,8 @@
 LargestVectorWidth =
 std::max((uint64_t)LargestVectorWidth,
  VT->getPrimitiveSizeInBits().getKnownMinSize());
-  if (Info.allowsRegister())
+  // Don't tie physregs.
+  if (Info.allowsRegister() && GCCReg.empty())
 InOutConstraints += llvm::utostr(i);
   else
 InOutConstraints += OutputConstraint;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89313: Fix implicit TypeSize casts in EmitCheckValue

[Caroline via Phabricator via cfe-commits]

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

Using getFixedSize() with:
 V->getType()->getPrimitiveSizeInBits()
like this:
 V->getType()->getPrimitiveSizeInBits().getFixedSize()
 makes the cast from TypeSize to uint64_t explicit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89313

Files:
  clang/lib/CodeGen/CGExpr.cpp


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -2988,7 +2988,7 @@
   // Floating-point types which fit into intptr_t are bitcast to integers
   // and then passed directly (after zero-extension, if necessary).
   if (V->getType()->isFloatingPointTy()) {
-unsigned Bits = V->getType()->getPrimitiveSizeInBits();
+unsigned Bits = V->getType()->getPrimitiveSizeInBits().getFixedSize();
 if (Bits <= TargetTy->getIntegerBitWidth())
   V = Builder.CreateBitCast(V, llvm::Type::getIntNTy(getLLVMContext(),
  Bits));


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -2988,7 +2988,7 @@
   // Floating-point types which fit into intptr_t are bitcast to integers
   // and then passed directly (after zero-extension, if necessary).
   if (V->getType()->isFloatingPointTy()) {
-unsigned Bits = V->getType()->getPrimitiveSizeInBits();
+unsigned Bits = V->getType()->getPrimitiveSizeInBits().getFixedSize();
 if (Bits <= TargetTy->getIntegerBitWidth())
   V = Builder.CreateBitCast(V, llvm::Type::getIntNTy(getLLVMContext(),
  Bits));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

[Sanjay Patel via Phabricator via cfe-commits]

spatel added inline comments.



Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:9
+define void @exp_f32(float* nocapture %varray) {
+; CHECK-LABEL: @exp_f32(
+; CHECK-NEXT:  entry:

fpetrogalli wrote:
> I think you are over-testing here. It is enough to check that inside the 
> vector body there is a call to the vector function you have listed in the 
> mapping. You are not checking for the whole auto-vectorization process here, 
> just the vectorization of the function call. Same for all the tests for this 
> patch in which you are doing something similar to this one test.
> 
> ```
> ; CHECK-LABEL: @exp_f32(
> ; CHECK-LABEL:   vector.body:
> ; CHECK: call fast <4 x float> @_ZGVbN4v___expf_finite(<4 x float> 
> ```
> 
I requested using "utils/update_test_checks.py" to auto-generate the assertions 
consistently.

We have standardized on this practice for tests in several passes because it 
provides extra test coverage (at the risk of over-testing), and it makes 
updating tests in the future nearly automatic.

The time cost of checking the extra lines is negligible vs. the benefit that we 
have gotten in finding/avoiding bugs. If the consensus is that it's not worth 
it on this particular file, I'm ok with that. But the general trend is 
definitely towards auto-generating full checks.



Comment at: 
llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:69
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+  %tmp = trunc i64 %indvars.iv to i32
+  %conv = sitofp i32 %tmp to float

The script should have warned you about using variables named "tmp". 
Independent of whether we choose to use the scripted assertions or not, you 
should change this value name (even plain "t" for "trunc" is an improvement 
over "tmp").


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

https://reviews.llvm.org/D88154

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

[PATCH] D89314: [SyntaxTree] Bug fix in `MutationsImpl::addAfter`.

[Eduardo Caldas via Phabricator via cfe-commits]

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

- Add assertions to other `MutationsImpl` member functions
- `findPrevious` is a free function


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89314

Files:
  clang/lib/Tooling/Syntax/Mutations.cpp


Index: clang/lib/Tooling/Syntax/Mutations.cpp
===
--- clang/lib/Tooling/Syntax/Mutations.cpp
+++ clang/lib/Tooling/Syntax/Mutations.cpp
@@ -23,6 +23,19 @@
 
 using namespace clang;
 
+static syntax::Node *findPrevious(syntax::Node *N) {
+  assert(N);
+  assert(N->getParent());
+  if (N->getParent()->getFirstChild() == N)
+return nullptr;
+  for (syntax::Node *C = N->getParent()->getFirstChild(); C != nullptr;
+   C = C->getNextSibling()) {
+if (C->getNextSibling() == N)
+  return C;
+  }
+  llvm_unreachable("could not find a child node");
+}
+
 // This class has access to the internals of tree nodes. Its sole purpose is to
 // define helpers that allow implementing the high-level mutation operations.
 class syntax::MutationsImpl {
@@ -30,14 +43,15 @@
   /// Add a new node with a specified role.
   static void addAfter(syntax::Node *Anchor, syntax::Node *New, NodeRole Role) 
{
 assert(Anchor != nullptr);
+assert(Anchor->Parent != nullptr);
 assert(New->Parent == nullptr);
 assert(New->NextSibling == nullptr);
-assert(!New->isDetached());
+assert(New->isDetached());
 assert(Role != NodeRole::Detached);
 
 New->setRole(Role);
 auto *P = Anchor->getParent();
-P->replaceChildRangeLowLevel(Anchor, Anchor, New);
+P->replaceChildRangeLowLevel(Anchor, Anchor->getNextSibling(), New);
 
 P->assertInvariants();
   }
@@ -60,6 +74,10 @@
 
   /// Completely remove the node from its parent.
   static void remove(syntax::Node *N) {
+assert(N != nullptr);
+assert(N->Parent != nullptr);
+assert(N->canModify());
+
 auto *P = N->getParent();
 P->replaceChildRangeLowLevel(findPrevious(N), N->getNextSibling(),
  /*New=*/nullptr);
@@ -67,18 +85,6 @@
 P->assertInvariants();
 N->assertInvariants();
   }
-
-private:
-  static syntax::Node *findPrevious(syntax::Node *N) {
-if (N->getParent()->getFirstChild() == N)
-  return nullptr;
-for (syntax::Node *C = N->getParent()->getFirstChild(); C != nullptr;
- C = C->getNextSibling()) {
-  if (C->getNextSibling() == N)
-return C;
-}
-llvm_unreachable("could not find a child node");
-  }
 };
 
 void syntax::removeStatement(syntax::Arena &A, syntax::Statement *S) {


Index: clang/lib/Tooling/Syntax/Mutations.cpp
===
--- clang/lib/Tooling/Syntax/Mutations.cpp
+++ clang/lib/Tooling/Syntax/Mutations.cpp
@@ -23,6 +23,19 @@
 
 using namespace clang;
 
+static syntax::Node *findPrevious(syntax::Node *N) {
+  assert(N);
+  assert(N->getParent());
+  if (N->getParent()->getFirstChild() == N)
+return nullptr;
+  for (syntax::Node *C = N->getParent()->getFirstChild(); C != nullptr;
+   C = C->getNextSibling()) {
+if (C->getNextSibling() == N)
+  return C;
+  }
+  llvm_unreachable("could not find a child node");
+}
+
 // This class has access to the internals of tree nodes. Its sole purpose is to
 // define helpers that allow implementing the high-level mutation operations.
 class syntax::MutationsImpl {
@@ -30,14 +43,15 @@
   /// Add a new node with a specified role.
   static void addAfter(syntax::Node *Anchor, syntax::Node *New, NodeRole Role) {
 assert(Anchor != nullptr);
+assert(Anchor->Parent != nullptr);
 assert(New->Parent == nullptr);
 assert(New->NextSibling == nullptr);
-assert(!New->isDetached());
+assert(New->isDetached());
 assert(Role != NodeRole::Detached);
 
 New->setRole(Role);
 auto *P = Anchor->getParent();
-P->replaceChildRangeLowLevel(Anchor, Anchor, New);
+P->replaceChildRangeLowLevel(Anchor, Anchor->getNextSibling(), New);
 
 P->assertInvariants();
   }
@@ -60,6 +74,10 @@
 
   /// Completely remove the node from its parent.
   static void remove(syntax::Node *N) {
+assert(N != nullptr);
+assert(N->Parent != nullptr);
+assert(N->canModify());
+
 auto *P = N->getParent();
 P->replaceChildRangeLowLevel(findPrevious(N), N->getNextSibling(),
  /*New=*/nullptr);
@@ -67,18 +85,6 @@
 P->assertInvariants();
 N->assertInvariants();
   }
-
-private:
-  static syntax::Node *findPrevious(syntax::Node *N) {
-if (N->getParent()->getFirstChild() == N)
-  return nullptr;
-for (syntax::Node *C = N->getParent()->getFirstChild(); C != nullptr;
- C = C->getNextSibling()) {
-  if (C->getNextSibling() == N)
-return C;
-}
-llvm_unreachable("could not find a child node");
-  }
 };
 
 void syn

[PATCH] D89146: [SyntaxTree] Fix rtti for `Expression`.

[Eduardo Caldas via Phabricator via cfe-commits]

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

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89146

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h


Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -206,7 +206,7 @@
   Expression(NodeKind K) : Tree(K) {}
   static bool classof(const Node *N) {
 return NodeKind::UnknownExpression <= N->getKind() &&
-   N->getKind() <= NodeKind::UnknownExpression;
+   N->getKind() <= NodeKind::CallExpression;
   }
 };
 


Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -206,7 +206,7 @@
   Expression(NodeKind K) : Tree(K) {}
   static bool classof(const Node *N) {
 return NodeKind::UnknownExpression <= N->getKind() &&
-   N->getKind() <= NodeKind::UnknownExpression;
+   N->getKind() <= NodeKind::CallExpression;
   }
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89284: [clangd] Propagate CollectMainFileRefs to BackgroundIndex

[Nathan Ridge 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 rGcb3c13fab6be: [clangd] Propagate CollectMainFileRefs to 
BackgroundIndex (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89284

Files:
  clang-tools-extra/clangd/ClangdServer.cpp


Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -219,6 +219,7 @@
 BGOpts.ContextProvider = [this](PathRef P) {
   return createProcessingContext(P);
 };
+BGOpts.CollectMainFileRefs = Opts.CollectMainFileRefs;
 BackgroundIdx = std::make_unique(
 TFS, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(


Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -219,6 +219,7 @@
 BGOpts.ContextProvider = [this](PathRef P) {
   return createProcessingContext(P);
 };
+BGOpts.CollectMainFileRefs = Opts.CollectMainFileRefs;
 BackgroundIdx = std::make_unique(
 TFS, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] cb3c13f - [clangd] Propagate CollectMainFileRefs to BackgroundIndex

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-10-13T09:20:18-04:00
New Revision: cb3c13fab6beac4666865b68bea59aae593aaf83

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

LOG: [clangd] Propagate CollectMainFileRefs to BackgroundIndex

This appears to have been an omission in D83536.

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 82dd7436b6f4..41c26be970d0 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -219,6 +219,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase 
&CDB,
 BGOpts.ContextProvider = [this](PathRef P) {
   return createProcessingContext(P);
 };
+BGOpts.CollectMainFileRefs = Opts.CollectMainFileRefs;
 BackgroundIdx = std::make_unique(
 TFS, CDB,
 BackgroundIndexStorage::createDiskBackedStorageFactory(



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

[clang] f84c77f - Revert "Raise the timeout in DirectoryWatcherTest to 10 s"

[Hans Wennborg via cfe-commits]

Author: Hans Wennborg
Date: 2020-10-13T15:21:06+02:00
New Revision: f84c77f424e15316f7f46f484880162a7cbcd80b

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

LOG: Revert "Raise the timeout in DirectoryWatcherTest to 10 s"

It didn't help.

This reverts commit bddef54c502811fa1406d1161d4baa15b56ebc32.

Added: 


Modified: 
clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp

Removed: 




diff  --git a/clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp 
b/clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
index 4d4a4614740d..650c0fc49764 100644
--- a/clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ b/clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -243,7 +243,7 @@ void checkEventualResultWithTimeout(VerifyingConsumer 
&TestConsumer) {
   std::thread worker(std::move(task));
   worker.detach();
 
-  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(10)) ==
+  EXPECT_TRUE(WaitForExpectedStateResult.wait_for(std::chrono::seconds(3)) ==
   std::future_status::ready)
   << "The expected result state wasn't reached before the time-out.";
   std::unique_lock L(TestConsumer.Mtx);



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

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.



Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -vector-library=LIBMVEC-X86 -inject-tli-mappings -loop-vectorize -S 
< %s | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"

fpetrogalli wrote:
> `-inject-tli-mappings` is not required here, as a pass itself is required by 
> the loop vectorizer.
I guess it still doesn't hurt to be explicit. Also, can you add a line for the 
new pass manager?



Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:9
+define void @exp_f32(float* nocapture %varray) {
+; CHECK-LABEL: @exp_f32(
+; CHECK-NEXT:  entry:

spatel wrote:
> fpetrogalli wrote:
> > I think you are over-testing here. It is enough to check that inside the 
> > vector body there is a call to the vector function you have listed in the 
> > mapping. You are not checking for the whole auto-vectorization process 
> > here, just the vectorization of the function call. Same for all the tests 
> > for this patch in which you are doing something similar to this one test.
> > 
> > ```
> > ; CHECK-LABEL: @exp_f32(
> > ; CHECK-LABEL:   vector.body:
> > ; CHECK: call fast <4 x float> @_ZGVbN4v___expf_finite(<4 x float> 
> > ```
> > 
> I requested using "utils/update_test_checks.py" to auto-generate the 
> assertions consistently.
> 
> We have standardized on this practice for tests in several passes because it 
> provides extra test coverage (at the risk of over-testing), and it makes 
> updating tests in the future nearly automatic.
> 
> The time cost of checking the extra lines is negligible vs. the benefit that 
> we have gotten in finding/avoiding bugs. If the consensus is that it's not 
> worth it on this particular file, I'm ok with that. But the general trend is 
> definitely towards auto-generating full checks.
FWIW in this case I would also slightly prefer to have more targeted test lines 
than auto-generating them (same for most loop-vectorize tests). The tests are 
large and LV generates a lot of code, and a lot of the code is completely 
unrelated/uninteresting to the code in the patch. IMO it would be enough to 
check the arguments of the vector function calls, together with the calls and 
maybe the induction variable.

The auto-generated check lines make things much more brittle and unrelated 
changes lead to us requiring to update lots of tests. And I am not sure if it 
is feasible to audit all details of the generated check lines (in the current 
patch ~500-800 new CHECK lines). So to me it seems like auto-generating checks 
here gives a false sense of security and make things harder down the line.



Comment at: 
llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:82
+
+!1 = distinct !{!1, !2, !3}
+!2 = !{!"llvm.loop.vectorize.width", i32 4}

I don't think we need this. You can just pass `-force-vector-width=4` to the 
command line and avoid the extra metadata duplicated for each test


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

https://reviews.llvm.org/D88154

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

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.



Comment at: llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls.ll:1
+; RUN: opt -vector-library=LIBMVEC  -inject-tli-mappings -force-vector-width=4 
-force-vector-interleave=1 -loop-vectorize -S < %s | FileCheck %s
+

spatel wrote:
> Why does this test file use command-line options to specify the vector factor 
> and the other uses metadata?
> If we can use metadata, then can you vary it to get better coverage (for 
> example <2 x double> or <8 x float>)?
FWIW most LV tests use `-force-vector-width`. If we decide to just go for the 
'essential' check lines, it should be easy to add multiple run lines with 
difference VFs from the command line?


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

https://reviews.llvm.org/D88154

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

[PATCH] D88154: Initial support for vectorization using Libmvec (GLIBC vector math library).

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.



Comment at: 
llvm/test/Transforms/LoopVectorize/X86/libm-vector-calls-finite.ll:82
+
+!1 = distinct !{!1, !2, !3}
+!2 = !{!"llvm.loop.vectorize.width", i32 4}

fhahn wrote:
> I don't think we need this. You can just pass `-force-vector-width=4` to the 
> command line and avoid the extra metadata duplicated for each test
Apologies, I missed that this was suggested earlier to test different VFs. If 
we decide to go with the 'essential' check lines approach it might make sense 
to just invoke opt with different VFs using `force-vector-width`.


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

https://reviews.llvm.org/D88154

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

[PATCH] D89309: [ThinLTO] In documentation, mention possible values for concurrency flags

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 297845.
aganea added a comment.

As suggested by @hans.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89309

Files:
  clang/docs/ThinLTO.rst


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -123,6 +123,11 @@
 - lld-link:
   ``/opt:lldltojobs=N``
 
+Other possible values for ``N`` are:
+- 0: Use one thread per physical core (default)
+- 1: Use a single thread only (disable multi-threading)
+- all: Use one thread per logical core (uses all hyper-threads)
+
 Incremental
 ---
 .. _incremental:


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -123,6 +123,11 @@
 - lld-link:
   ``/opt:lldltojobs=N``
 
+Other possible values for ``N`` are:
+- 0: Use one thread per physical core (default)
+- 1: Use a single thread only (disable multi-threading)
+- all: Use one thread per logical core (uses all hyper-threads)
+
 Incremental
 ---
 .. _incremental:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88737: [AIX] Turn -fdata-sections on by default in Clang

[Jason Liu via Phabricator via cfe-commits]

jasonliu added a comment.

Hi Fangrui(@MaskRay), are you okay with this patch to land as is?


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

https://reviews.llvm.org/D88737

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

[PATCH] D89064: [AIX] Disable two itanium alignment LIT testcases

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 297848.
Xiangling_L added a comment.

Use regex to match AIX layout dumping format.


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

https://reviews.llvm.org/D89064

Files:
  clang/test/Layout/itanium-pack-and-align.cpp
  clang/test/Layout/itanium-union-bitfield.cpp


Index: clang/test/Layout/itanium-union-bitfield.cpp
===
--- clang/test/Layout/itanium-union-bitfield.cpp
+++ clang/test/Layout/itanium-union-bitfield.cpp
@@ -18,12 +18,11 @@
 // CHECK:*** Dumping AST Record Layout
 // CHECK-NEXT: 0 | union A
 // CHECK-NEXT: 0:0-2 |   int f1
-// CHECK-NEXT:   | [sizeof=4, dsize=1, align=4
-// CHECK-NEXT:   |  nvsize=1, nvalign=4]
+// CHECK-NEXT:   | [sizeof=4, dsize=1, align=4{{(, preferredalign=4,)*}}
+// CHECK-NEXT:   |  nvsize=1, nvalign=4{{(, preferrednvalign=4)*}}]
 
 // CHECK:*** Dumping AST Record Layout
 // CHECK-NEXT:  0 | union B
 // CHECK-NEXT: 0:0-34 |   char f1
-// CHECK-NEXT:| [sizeof=8, dsize=5, align=4
-// CHECK-NEXT:|  nvsize=5, nvalign=4]
-
+// CHECK-NEXT:| [sizeof=8, dsize=5, align=4{{(, preferredalign=4,)*}}
+// CHECK-NEXT:|  nvsize=5, nvalign=4{{(, preferrednvalign=4)*}}]
Index: clang/test/Layout/itanium-pack-and-align.cpp
===
--- clang/test/Layout/itanium-pack-and-align.cpp
+++ clang/test/Layout/itanium-pack-and-align.cpp
@@ -16,11 +16,11 @@
 // CHECK:  0 | struct T
 // CHECK-NEXT:  0 |   char x
 // CHECK-NEXT:  1 |   int y
-// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
-// CHECK-NEXT:|  nvsize=8, nvalign=8]
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,{{( 
preferredalign=8,)*}}
+// CHECK-NEXT:|  nvsize=8, nvalign=8{{(, preferrednvalign=8)*}}]
 
 // CHECK:  0 | struct S
 // CHECK-NEXT:  0 |   char x
 // CHECK-NEXT:  1 |   int y
-// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
-// CHECK-NEXT:|  nvsize=8, nvalign=8]
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,{{( 
preferredalign=8,)*}}
+// CHECK-NEXT:|  nvsize=8, nvalign=8{{(, preferrednvalign=8)*}}]


Index: clang/test/Layout/itanium-union-bitfield.cpp
===
--- clang/test/Layout/itanium-union-bitfield.cpp
+++ clang/test/Layout/itanium-union-bitfield.cpp
@@ -18,12 +18,11 @@
 // CHECK:*** Dumping AST Record Layout
 // CHECK-NEXT: 0 | union A
 // CHECK-NEXT: 0:0-2 |   int f1
-// CHECK-NEXT:   | [sizeof=4, dsize=1, align=4
-// CHECK-NEXT:   |  nvsize=1, nvalign=4]
+// CHECK-NEXT:   | [sizeof=4, dsize=1, align=4{{(, preferredalign=4,)*}}
+// CHECK-NEXT:   |  nvsize=1, nvalign=4{{(, preferrednvalign=4)*}}]
 
 // CHECK:*** Dumping AST Record Layout
 // CHECK-NEXT:  0 | union B
 // CHECK-NEXT: 0:0-34 |   char f1
-// CHECK-NEXT:| [sizeof=8, dsize=5, align=4
-// CHECK-NEXT:|  nvsize=5, nvalign=4]
-
+// CHECK-NEXT:| [sizeof=8, dsize=5, align=4{{(, preferredalign=4,)*}}
+// CHECK-NEXT:|  nvsize=5, nvalign=4{{(, preferrednvalign=4)*}}]
Index: clang/test/Layout/itanium-pack-and-align.cpp
===
--- clang/test/Layout/itanium-pack-and-align.cpp
+++ clang/test/Layout/itanium-pack-and-align.cpp
@@ -16,11 +16,11 @@
 // CHECK:  0 | struct T
 // CHECK-NEXT:  0 |   char x
 // CHECK-NEXT:  1 |   int y
-// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
-// CHECK-NEXT:|  nvsize=8, nvalign=8]
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,{{( preferredalign=8,)*}}
+// CHECK-NEXT:|  nvsize=8, nvalign=8{{(, preferrednvalign=8)*}}]
 
 // CHECK:  0 | struct S
 // CHECK-NEXT:  0 |   char x
 // CHECK-NEXT:  1 |   int y
-// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
-// CHECK-NEXT:|  nvsize=8, nvalign=8]
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,{{( preferredalign=8,)*}}
+// CHECK-NEXT:|  nvsize=8, nvalign=8{{(, preferrednvalign=8)*}}]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89064: [AIX] Support two itanium alignment LIT testcases for AIX using regex

[Hubert Tong via Phabricator via cfe-commits]

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

LGTM with suggestion.




Comment at: clang/test/Layout/itanium-pack-and-align.cpp:19
 // CHECK-NEXT:  1 |   int y
-// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
-// CHECK-NEXT:|  nvsize=8, nvalign=8]
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,{{( 
preferredalign=8,)*}}
+// CHECK-NEXT:|  nvsize=8, nvalign=8{{(, preferrednvalign=8)*}}]

If it works, use `?` in place of `*`. Do also for all of the other cases.


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

https://reviews.llvm.org/D89064

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

[PATCH] D89309: [ThinLTO] In documentation, mention possible values for concurrency flags

[Alexandre Ganea 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 rG1dbf05f5b44d: [ThinLTO][Documentation] Mention possible 
values for concurrency flags (authored by aganea).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89309

Files:
  clang/docs/ThinLTO.rst


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -123,6 +123,11 @@
 - lld-link:
   ``/opt:lldltojobs=N``
 
+Other possible values for ``N`` are:
+- 0: Use one thread per physical core (default)
+- 1: Use a single thread only (disable multi-threading)
+- all: Use one thread per logical core (uses all hyper-threads)
+
 Incremental
 ---
 .. _incremental:


Index: clang/docs/ThinLTO.rst
===
--- clang/docs/ThinLTO.rst
+++ clang/docs/ThinLTO.rst
@@ -123,6 +123,11 @@
 - lld-link:
   ``/opt:lldltojobs=N``
 
+Other possible values for ``N`` are:
+- 0: Use one thread per physical core (default)
+- 1: Use a single thread only (disable multi-threading)
+- all: Use one thread per logical core (uses all hyper-threads)
+
 Incremental
 ---
 .. _incremental:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1dbf05f - [ThinLTO][Documentation] Mention possible values for concurrency flags

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2020-10-13T09:57:58-04:00
New Revision: 1dbf05f5b44db17dcd8520b032e83061189ff4f8

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

LOG: [ThinLTO][Documentation] Mention possible values for concurrency flags

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

Added: 


Modified: 
clang/docs/ThinLTO.rst

Removed: 




diff  --git a/clang/docs/ThinLTO.rst b/clang/docs/ThinLTO.rst
index 528530c5ae98..0da822f498b9 100644
--- a/clang/docs/ThinLTO.rst
+++ b/clang/docs/ThinLTO.rst
@@ -123,6 +123,11 @@ be reduced to ``N`` via:
 - lld-link:
   ``/opt:lldltojobs=N``
 
+Other possible values for ``N`` are:
+- 0: Use one thread per physical core (default)
+- 1: Use a single thread only (disable multi-threading)
+- all: Use one thread per logical core (uses all hyper-threads)
+
 Incremental
 ---
 .. _incremental:



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

[libunwind] 6b7a49b - Fix all the CMake code that can only handle -stdlib= but not --stdlib=

[Raphael Isemann via cfe-commits]

Author: Raphael Isemann
Date: 2020-10-13T16:05:21+02:00
New Revision: 6b7a49bb43d58c2c08fddb9f6c538ee52806de0a

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

LOG: Fix all the CMake code that can only handle -stdlib= but not --stdlib=

There are several places in LLVM's CMake setup that try to remove the
`stdlib=...` flag from the CMake flags. All this code however only considered
the `-stdlib=` variant of the flag but not the alternative spelling with a
double dash. This causes that when one adds `--stdlib=...` to the user-provided
CMake flags that this gets transformed into just `-` which ends up causing the
build system to think it should read the source from stdin (which then lead to
very confusing build errors).

This just adds the alternative spelling before the`-stdlib=` variant in all
these places

Reviewed By: ldionne

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

Added: 


Modified: 
libcxx/CMakeLists.txt
libcxxabi/CMakeLists.txt
libunwind/CMakeLists.txt

Removed: 




diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 287059548e42..ee250374732d 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -478,7 +478,7 @@ if (NOT LIBCXX_STANDALONE_BUILD)
   remove_flags(-DNDEBUG -UNDEBUG -D_DEBUG
-lc++abi)
 endif()
-remove_flags(-stdlib=libc++ -stdlib=libstdc++)
+remove_flags(--stdlib=libc++ -stdlib=libc++ --stdlib=libstdc++ 
-stdlib=libstdc++)
 
 # FIXME: Remove all debug flags and flags that change which Windows
 # default libraries are linked. Currently we only support linking the

diff  --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index 10ac112c90d9..c4d76ea22eca 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -248,6 +248,8 @@ if (LIBCXXABI_HAS_NOSTDINCXX_FLAG)
   # See: https://gitlab.kitware.com/cmake/cmake/issues/19227
   set(CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES "")
   # Remove -stdlib flags to prevent them from causing an unused flag warning.
+  string(REPLACE "--stdlib=libc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+  string(REPLACE "--stdlib=libstdc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
   string(REPLACE "-stdlib=libc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
   string(REPLACE "-stdlib=libstdc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
 endif()

diff  --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index 7851f3e45d0c..ebe9e449ec02 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -321,6 +321,8 @@ add_cxx_compile_flags_if_supported(-fno-rtti)
 if (LIBUNWIND_HAS_NOSTDINCXX_FLAG)
   list(APPEND LIBUNWIND_COMPILE_FLAGS -nostdinc++)
   # Remove -stdlib flags to prevent them from causing an unused flag warning.
+  string(REPLACE "--stdlib=libc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+  string(REPLACE "--stdlib=libstdc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
   string(REPLACE "-stdlib=libc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
   string(REPLACE "-stdlib=libstdc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
 endif()



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

[PATCH] D89318: [ASTImporter] Fix crash caused by unset AttributeSpellingListIndex

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: teemperor, shafik.
Herald added subscribers: cfe-commits, pengfei, gamesh411, Szelethus, arphaman, 
dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.
martong requested review of this revision.

During the import of attributes we forgot to set the spelling list
index. This caused a segfault when we wanted to traverse the AST
(e.g. by the dump() method).

Here is the relevant valgrind trace of the ASTMerge test when accessing
an already freed memory region:

  ==25717== Invalid read of size 8
  ==25717==at 0x86E96FA: 
clang::AttributeCommonInfo::calculateAttributeSpellingListIndex() const (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangBasic.so.12git)
  ==25717==by 0xAE73598: clang::RestrictAttr::getSpelling() const (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==25717==by 0xB11D93B: clang::attrvisitor::Base::Visit(clang::Attr const*) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==25717==by 0xADC0880: std::_Function_handler::Visit(clang::Attr 
const*)::{lambda()#1}>(llvm::StringRef, 
clang::ASTNodeTraverser::Visit(clang::Attr 
const*)::{lambda()#1})::{lambda(bool)#1}>::_M_invoke(std::_Any_data const&, 
bool&&) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==25717==by 0xADC9DAD: std::_Function_handler::Visit(clang::Decl 
const*)::{lambda()#1}>(llvm::StringRef, 
clang::ASTNodeTraverser::Visit(clang::Decl 
const*)::{lambda()#1})::{lambda(bool)#1}>::_M_invoke(std::_Any_data const&, 
bool&&) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==25717==by 0xADBE58A: void 
clang::TextTreeStructure::AddChild::Visit(clang::Decl 
const*)::{lambda()#1}>(llvm::StringRef, 
clang::ASTNodeTraverser::Visit(clang::Decl const*)::{lambda()#1}) [clone 
.constprop.2050] (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==25717==by 0xADBE2BA: clang::ASTNodeTraverser::Visit(clang::Decl const*)::{lambda()#1}::operator()() 
const (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==25717==by 0xADBE706: void 
clang::TextTreeStructure::AddChild::Visit(clang::Decl 
const*)::{lambda()#1}>(llvm::StringRef, 
clang::ASTNodeTraverser::Visit(clang::Decl const*)::{lambda()#1}) [clone 
.constprop.2050] (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==25717==by 0xADD4226: clang::Decl::dump(llvm::raw_ostream&, bool, 
clang::ASTDumpOutputFormat) const (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==25717==by 0x73CA1B5: (anonymous 
namespace)::ASTPrinter::HandleTranslationUnit(clang::ASTContext&) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==25717==by 0xB84A568: clang::ParseAST(clang::Sema&, bool, bool) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangParse.so.12git)
  ==25717==by 0x7401AA0: clang::ASTMergeAction::ExecuteAction() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==25717==  Address 0xe42fa88 is 3,528 bytes inside a block of size 4,096 
free'd
  ==25717==at 0x4C3123B: operator delete(void*) (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==25717==by 0xB4A4D3F: clang::Preprocessor::~Preprocessor() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangLex.so.12git)
  ==25717==by 0x7402D6C: std::_Sp_counted_deleter::_Deleter >, 
std::allocator, (__gnu_cxx::_Lock_policy)2>::_M_dispose() 
(in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==25717==by 0x57548D5: 
std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangCodeGen.so.12git)
  ==25717==by 0x741DB08: clang::ASTUnit::~ASTUnit() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==25717==by 0x7401CFF: clang::ASTMergeAction::ExecuteAction() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==25717==by 0x747C298: clang::FrontendAction::Execute() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==25717==by 0x742EA69: 
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==25717==by 0x40DC565: 
clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontendTool.so.12git)
  ==25717==by 0x11BE7B: cc1_main(llvm::ArrayRef, char const*, 
void*) (in /home/egbomrt/WORK/llvm5/build/release_assert/bin/clang-11)
  ==25717==by 0x116E38: ExecuteCC1Tool(llvm::SmallVectorImpl&) 
(in /home/egbomrt/WORK/llvm5/build/release_assert/bin/clang-11)
  ==25717==by 0x114DC3: main (in 
/home/egbomrt/W

[PATCH] D89319: [ASTImporter] Fix crash caused by unimported type of FromatAttr

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: teemperor, shafik.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.
martong requested review of this revision.

During the import of FormatAttrs we forgot to import the type (e.g
`__scanf__`) of the attribute. This caused a segfault when we wanted to
traverse the AST (e.g. by the dump() method).

Here is the relevant valgrind trace of the ASTMerge test when accessing
an already freed memory region:

  ==1941== Invalid read of size 8
  ==1941==at 0xB112EE6: 
clang::TextNodeDumper::VisitFormatAttr(clang::FormatAttr const*) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==1941==by 0xB11CB6C: clang::attrvisitor::Base::Visit(clang::Attr const*) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==1941==by 0xADC0880: std::_Function_handler::Visit(clang::Attr 
const*)::{lambda()#1}>(llvm::StringRef, 
clang::ASTNodeTraverser::Visit(clang::Attr 
const*)::{lambda()#1})::{lambda(bool)#1}>::_M_invoke(std::_Any_data const&, 
bool&&) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==1941==by 0xADC9DAD: std::_Function_handler::Visit(clang::Decl 
const*)::{lambda()#1}>(llvm::StringRef, 
clang::ASTNodeTraverser::Visit(clang::Decl 
const*)::{lambda()#1})::{lambda(bool)#1}>::_M_invoke(std::_Any_data const&, 
bool&&) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==1941==by 0xADBE58A: void 
clang::TextTreeStructure::AddChild::Visit(clang::Decl 
const*)::{lambda()#1}>(llvm::StringRef, 
clang::ASTNodeTraverser::Visit(clang::Decl const*)::{lambda()#1}) [clone 
.constprop.2050] (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==1941==by 0xADBE2BA: clang::ASTNodeTraverser::Visit(clang::Decl const*)::{lambda()#1}::operator()() 
const (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==1941==by 0xADBE706: void 
clang::TextTreeStructure::AddChild::Visit(clang::Decl 
const*)::{lambda()#1}>(llvm::StringRef, 
clang::ASTNodeTraverser::Visit(clang::Decl const*)::{lambda()#1}) [clone 
.constprop.2050] (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==1941==by 0xADD4226: clang::Decl::dump(llvm::raw_ostream&, bool, 
clang::ASTDumpOutputFormat) const (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangAST.so.12git)
  ==1941==by 0x73CA1B5: (anonymous 
namespace)::ASTPrinter::HandleTranslationUnit(clang::ASTContext&) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==1941==by 0xB84A568: clang::ParseAST(clang::Sema&, bool, bool) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangParse.so.12git)
  ==1941==by 0x7401AA0: clang::ASTMergeAction::ExecuteAction() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==1941==by 0x747C298: clang::FrontendAction::Execute() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==1941==  Address 0xe42fab8 is 3,576 bytes inside a block of size 4,096 free'd
  ==1941==at 0x4C3123B: operator delete(void*) (in 
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==1941==by 0xB4A4D3F: clang::Preprocessor::~Preprocessor() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangLex.so.12git)
  ==1941==by 0x7402D6C: std::_Sp_counted_deleter::_Deleter >, 
std::allocator, (__gnu_cxx::_Lock_policy)2>::_M_dispose() 
(in /home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==1941==by 0x57548D5: 
std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangCodeGen.so.12git)
  ==1941==by 0x741DB08: clang::ASTUnit::~ASTUnit() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==1941==by 0x7401CFF: clang::ASTMergeAction::ExecuteAction() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==1941==by 0x747C298: clang::FrontendAction::Execute() (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==1941==by 0x742EA69: 
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontend.so.12git)
  ==1941==by 0x40DC565: 
clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (in 
/home/egbomrt/WORK/llvm5/build/release_assert/lib/libclangFrontendTool.so.12git)
  ==1941==by 0x11BE7B: cc1_main(llvm::ArrayRef, char const*, 
void*) (in /home/egbomrt/WORK/llvm5/build/release_assert/bin/clang-11)
  ==1941==by 0x116E38: ExecuteCC1Tool(llvm::SmallVectorImpl&) 
(in /home/egbomrt/WORK/llvm5/build/release_assert/bin/clang-11)
  ==1941==by 0x114DC3: main (in 
/home/egbomrt/WORK/llvm5/build/release_assert/bin/

[PATCH] D78658: [clang][Frontend] Add missing error handling

[LemonBoy via Phabricator via cfe-commits]

LemonBoy added a comment.

> Perhaps this'd be more robust with ScopeExit?

Not really, `OnError` is not executed when/if the function succeeds.


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

https://reviews.llvm.org/D78658

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

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

CUDA/HIP are facing similar issues, i.e. consistency of name mangling of 
kernels between host/device compilation of the same TU. I hope this feature to 
be implemented in a generic way so that it may be reusable for other offloading 
languages.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D76620#2327617 , @yaxunl wrote:

> CUDA/HIP are facing similar issues, i.e. consistency of name mangling of 
> kernels between host/device compilation of the same TU. I hope this feature 
> to be implemented in a generic way so that it may be reusable for other 
> offloading languages.

This implementation SHOULD work for that I'd expect, but we're currently 
re-visiting it in order to get the names to be demanglable (at least on Linux, 
and will likely delay an RFC).  We've used the Itanium Mangler plus some 
modification to the lambda mangling to make it depend on source-lines instead 
of arbitrary ordering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D89146: [SyntaxTree] Fix rtti for `Expression`.

[Eduardo Caldas 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 rGa8f1790fdb8c: [SyntaxTree] Fix rtti for `Expression`. 
(authored by eduucaldas).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89146

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h


Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -206,7 +206,7 @@
   Expression(NodeKind K) : Tree(K) {}
   static bool classof(const Node *N) {
 return NodeKind::UnknownExpression <= N->getKind() &&
-   N->getKind() <= NodeKind::UnknownExpression;
+   N->getKind() <= NodeKind::CallExpression;
   }
 };
 


Index: clang/include/clang/Tooling/Syntax/Nodes.h
===
--- clang/include/clang/Tooling/Syntax/Nodes.h
+++ clang/include/clang/Tooling/Syntax/Nodes.h
@@ -206,7 +206,7 @@
   Expression(NodeKind K) : Tree(K) {}
   static bool classof(const Node *N) {
 return NodeKind::UnknownExpression <= N->getKind() &&
-   N->getKind() <= NodeKind::UnknownExpression;
+   N->getKind() <= NodeKind::CallExpression;
   }
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a8f1790 - [SyntaxTree] Fix rtti for `Expression`.

[Eduardo Caldas via cfe-commits]

Author: Eduardo Caldas
Date: 2020-10-13T14:47:43Z
New Revision: a8f1790fdb8ce1c53f024870cd51f32724d4c55f

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

LOG: [SyntaxTree] Fix rtti for `Expression`.

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

Added: 


Modified: 
clang/include/clang/Tooling/Syntax/Nodes.h

Removed: 




diff  --git a/clang/include/clang/Tooling/Syntax/Nodes.h 
b/clang/include/clang/Tooling/Syntax/Nodes.h
index ed4449adb0f0..33ed2ec5c349 100644
--- a/clang/include/clang/Tooling/Syntax/Nodes.h
+++ b/clang/include/clang/Tooling/Syntax/Nodes.h
@@ -206,7 +206,7 @@ class Expression : public Tree {
   Expression(NodeKind K) : Tree(K) {}
   static bool classof(const Node *N) {
 return NodeKind::UnknownExpression <= N->getKind() &&
-   N->getKind() <= NodeKind::UnknownExpression;
+   N->getKind() <= NodeKind::CallExpression;
   }
 };
 



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

[PATCH] D89319: [ASTImporter] Fix crash caused by unimported type of FromatAttr

[Raphael Isemann via Phabricator via cfe-commits]

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

LGTM, thanks!




Comment at: clang/test/ASTMerge/attr/Inputs/FormatAttr.cpp:2
+int foo(const char * fmt, ...)
+__attribute__ ((__format__ (__scanf__, 1, 2)));

(Not sure if we care about that in tests, but that's technically not in LLVM 
code style)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89319

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

[PATCH] D89319: [ASTImporter] Fix crash caused by unimported type of FromatAttr

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik accepted this revision.
shafik added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89319

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

[PATCH] D89025: [RISCV] Add -mtune support

[Luís Marques via Phabricator via cfe-commits]

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

LGTM, but I would like other people to also review this, if possible.
(Just be sure to check/fix the clang-format warnings and the inline comments).




Comment at: clang/test/Driver/riscv-cpus.c:29
+
+// Check mtune alias CPU has resolve to the right CPU according XLEN.
+// RUN: %clang -target riscv32 -### -c %s 2>&1 -mtune=generic | FileCheck 
-check-prefix=MTUNE-GENERIC-32 %s

Nit: resolve -> resolved.



Comment at: clang/test/Driver/riscv-cpus.c:82
+// Check interaction between mcpu and mtune.
+//
+// RUN: %clang -target riscv32 -### -c %s 2>&1 -mcpu=sifive-e31 
-mtune=sifive-e76 | FileCheck -check-prefix=MTUNE-E31-MCPU-E76 %s

khchen wrote:
> maybe we can describe what is expected interaction behavior somewhere.
+1



Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:201
   const RISCVTargetMachine &RTM = static_cast(TM);
-  const RISCVSubtarget STI(TT, CPU, FS, /*ABIName=*/"", RTM);
+  /* TuneCPU don't impact emission for ELF attributes, ELF attribute only
+ care about arch related features, so we can set TuneCPU as CPU.  */

Nit: don't -> doesn't; for -> of; attribute -> attributes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89025

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

[PATCH] D89025: [RISCV] Add -mtune support

[Luís Marques via Phabricator via cfe-commits]

luismarques added a comment.

In D89025#2324334 , @khchen wrote:

> RISCV supports `-mcpu` with default empty arch to align gcc's `-mtune` 
> behavior since clang didn't support `-mtune` before. But now clang has 
> `-mtune`, is it a good idea to remove those options? (ex. `rocket-rv32/rv64`, 
> `sifive-7-rv32/64`)

If possible that would good, since -mcpu is deprecated (for e.g. x86_64) or 
unsupported in GCC (for e.g. RISC-V). So doing that would further align Clang 
with GCC. But I wonder if this might be too problematic, in terms of 
compatibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89025

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

[PATCH] D88737: [AIX] Turn -fdata-sections on by default in Clang

[Fangrui Song via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D88737

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

[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

The longer timeout didn't help :(

I'm not sure what's different about the machine where this is failing. Maybe 
it's some filesystem issue due to being a VM?

Any ideas for good printfs or similar that could be added to figure out exactly 
what part is failing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88666

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

[PATCH] D89286: [DebugInfo] Check for templated static data member when adding constant to record static fields

[Sylvain Audi via Phabricator via cfe-commits]

saudi added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1420
+  if (Var->getInit())
+Value = Var->evaluateValue();
+  else if (auto *TemplateDecl = Var->getInstantiatedFromStaticDataMember()) {

I encountered an assert inside `evaluateValue()` call :
`assert(!Init->isValueDependent());`

repro code:
```
template  class A {
 static constexpr int dep = VAL;
};
A<10> a;
```
Note that it crashed with `static constexp` member but not with `static const`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89286

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

[PATCH] D89286: [DebugInfo] Check for templated static data member when adding constant to record static fields

[Sylvain Audi via Phabricator via cfe-commits]

saudi added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1420
+  if (Var->getInit())
+Value = Var->evaluateValue();
+  else if (auto *TemplateDecl = Var->getInstantiatedFromStaticDataMember()) {

saudi wrote:
> I encountered an assert inside `evaluateValue()` call :
> `assert(!Init->isValueDependent());`
> 
> repro code:
> ```
> template  class A {
>  static constexpr int dep = VAL;
> };
> A<10> a;
> ```
> Note that it crashed with `static constexp` member but not with `static 
> const`.
> 
Sorry, I got the wrong line for this comment.
This was actually the case 1. from @dblaikie 's comment for the `TemplateDecl` 
evaluation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89286

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

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

You know on both sides that a lambda is used as a kernel, yes?  Why not simply 
introduce that into the mangling of lambdas, so that the subset of lambdas used 
as kernels form a stable sequence?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D76620#2327901 , @rjmccall wrote:

> You know on both sides that a lambda is used as a kernel, yes?  Why not 
> simply introduce that into the mangling of lambdas, so that the subset of 
> lambdas used as kernels form a stable sequence?

Coming up with said stable sequence is somewhat difficult as well.  A strict 
integer ordering didn't end up being stable, as some of the kernel handling can 
cause instantiations to happen in a slightly different ordering, which messed 
that up, as would use of the builtin. We wanted to find a way that was 
dependent on the source code alone.  The "Quick and Dirty" solution was 
line/column numbers, though we considered a hash of that same information to at 
least make it shorter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D89312: [SVE][CodeGen]Use TypeSize comparison functions in clang

[Caroline via Phabricator via cfe-commits]

CarolineConcatto updated this revision to Diff 297884.
CarolineConcatto added a comment.

-Use getFixedSize instead of isKnowXY


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89312

Files:
  clang/lib/CodeGen/CGBuiltin.cpp


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -5599,8 +5599,8 @@
 
   Value *Result = CGF.EmitNeonCall(F, Ops, s);
   llvm::Type *ResultType = CGF.ConvertType(E->getType());
-  if (ResultType->getPrimitiveSizeInBits() <
-  Result->getType()->getPrimitiveSizeInBits())
+  if (ResultType->getPrimitiveSizeInBits().getFixedSize() <
+  Result->getType()->getPrimitiveSizeInBits().getFixedSize())
 return CGF.Builder.CreateExtractElement(Result, C0);
 
   return CGF.Builder.CreateBitCast(Result, ResultType, s);


Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -5599,8 +5599,8 @@
 
   Value *Result = CGF.EmitNeonCall(F, Ops, s);
   llvm::Type *ResultType = CGF.ConvertType(E->getType());
-  if (ResultType->getPrimitiveSizeInBits() <
-  Result->getType()->getPrimitiveSizeInBits())
+  if (ResultType->getPrimitiveSizeInBits().getFixedSize() <
+  Result->getType()->getPrimitiveSizeInBits().getFixedSize())
 return CGF.Builder.CreateExtractElement(Result, C0);
 
   return CGF.Builder.CreateBitCast(Result, ResultType, s);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89125: AMDGPU: Remove -mamdgpu-debugger-abi option

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe2eaa914514c: AMDGPU: Remove -mamdgpu-debugger-abi option 
(authored by kzhuravl).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89125

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/test/Driver/amdgpu-features.c


Index: clang/test/Driver/amdgpu-features.c
===
--- clang/test/Driver/amdgpu-features.c
+++ clang/test/Driver/amdgpu-features.c
@@ -1,11 +1,3 @@
-// RUN: %clang -### -target amdgcn -x cl -S -emit-llvm -mcpu=kaveri 
-mamdgpu-debugger-abi=0.0 %s -o - 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-MAMDGPU-DEBUGGER-ABI-0-0 %s
-// CHECK-MAMDGPU-DEBUGGER-ABI-0-0: the clang compiler does not support 
'-mamdgpu-debugger-abi=0.0'
-
-// RUN: %clang -### -target amdgcn -x cl -S -emit-llvm -mcpu=kaveri 
-mamdgpu-debugger-abi=1.0 %s -o - 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-MAMDGPU-DEBUGGER-ABI-1-0 %s
-// CHECK-MAMDGPU-DEBUGGER-ABI-1-0: the clang compiler does not support 
'-mamdgpu-debugger-abi=1.0'
-
 // RUN: %clang -### -target amdgcn -mcpu=gfx700 -mcode-object-v3 %s 2>&1 | 
FileCheck --check-prefix=CODE-OBJECT-V3 %s
 // CODE-OBJECT-V3: "-target-feature" "+code-object-v3"
 
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -365,9 +365,6 @@
  const llvm::Triple &Triple,
  const llvm::opt::ArgList &Args,
  std::vector &Features) {
-  if (const Arg *dAbi = Args.getLastArg(options::OPT_mamdgpu_debugger_abi))
-D.Diag(diag::err_drv_clang_unsupported) << dAbi->getAsString(Args);
-
   // Add target ID features to -target-feature options. No diagnostics should
   // be emitted here since invalid target ID is diagnosed at other places.
   StringRef TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2461,12 +2461,6 @@
  Values<"command,reactor">,
  HelpText<"Execution model (WebAssembly only)">;
 
-def mamdgpu_debugger_abi : Joined<["-"], "mamdgpu-debugger-abi=">,
-  Flags<[HelpHidden]>,
-  Group,
-  HelpText<"Generate additional code for specified  of debugger ABI 
(AMDGPU only)">,
-  MetaVarName<"">;
-
 def mcode_object_v3 : Flag<["-"], "mcode-object-v3">, 
Group,
   HelpText<"Enable code object v3 (AMDGPU only)">;
 def mno_code_object_v3 : Flag<["-"], "mno-code-object-v3">, 
Group,
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2655,10 +2655,6 @@
 
 .. option:: -mcmodel=, -mcmodel=medany (equivalent to -mcmodel=medium), 
-mcmodel=medlow (equivalent to -mcmodel=small)
 
-.. option:: -mcode-object-v3, -mno-code-object-v3
-
-Enable code object v3 (AMDGPU only)
-
 .. option:: -mconsole
 
 .. program:: clang1
@@ -2939,6 +2935,10 @@
 
 AMDGPU
 --
+.. option:: -mcode-object-v3, -mno-code-object-v3
+
+Enable code object v3 (AMDGPU only)
+
 .. option:: -mcumode, -mno-cumode
 
 CU wavefront execution mode is used (AMDGPU only)


Index: clang/test/Driver/amdgpu-features.c
===
--- clang/test/Driver/amdgpu-features.c
+++ clang/test/Driver/amdgpu-features.c
@@ -1,11 +1,3 @@
-// RUN: %clang -### -target amdgcn -x cl -S -emit-llvm -mcpu=kaveri -mamdgpu-debugger-abi=0.0 %s -o - 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-MAMDGPU-DEBUGGER-ABI-0-0 %s
-// CHECK-MAMDGPU-DEBUGGER-ABI-0-0: the clang compiler does not support '-mamdgpu-debugger-abi=0.0'
-
-// RUN: %clang -### -target amdgcn -x cl -S -emit-llvm -mcpu=kaveri -mamdgpu-debugger-abi=1.0 %s -o - 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-MAMDGPU-DEBUGGER-ABI-1-0 %s
-// CHECK-MAMDGPU-DEBUGGER-ABI-1-0: the clang compiler does not support '-mamdgpu-debugger-abi=1.0'
-
 // RUN: %clang -### -target amdgcn -mcpu=gfx700 -mcode-object-v3 %s 2>&1 | FileCheck --check-prefix=CODE-OBJECT-V3 %s
 // CODE-OBJECT-V3: "-target-feature" "+code-object-v3"
 
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -365,9 +365,6 @@
  const llvm::Triple &Triple,

[clang] e2eaa91 - AMDGPU: Remove -mamdgpu-debugger-abi option

[Konstantin Zhuravlyov via cfe-commits]

Author: Konstantin Zhuravlyov
Date: 2020-10-13T12:20:28-04:00
New Revision: e2eaa914514c26c8e51c76148996a2e9cf74613c

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

LOG: AMDGPU: Remove -mamdgpu-debugger-abi option

It has been unsupported for few years now.

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

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/test/Driver/amdgpu-features.c

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index ff3decbca70c..97a96631cc21 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -2655,10 +2655,6 @@ Align selected branches (fused, jcc, jmp) within 32-byte 
boundary
 
 .. option:: -mcmodel=, -mcmodel=medany (equivalent to -mcmodel=medium), 
-mcmodel=medlow (equivalent to -mcmodel=small)
 
-.. option:: -mcode-object-v3, -mno-code-object-v3
-
-Enable code object v3 (AMDGPU only)
-
 .. option:: -mconsole
 
 .. program:: clang1
@@ -2939,6 +2935,10 @@ Specify the size in bits of an SVE vector register. 
Defaults to the vector lengt
 
 AMDGPU
 --
+.. option:: -mcode-object-v3, -mno-code-object-v3
+
+Enable code object v3 (AMDGPU only)
+
 .. option:: -mcumode, -mno-cumode
 
 CU wavefront execution mode is used (AMDGPU only)

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 9980dda23bb0..f5e745b1dbe2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2461,12 +2461,6 @@ def mexec_model_EQ : Joined<["-"], "mexec-model=">, 
Group,
  HelpText<"Execution model (WebAssembly only)">;
 
-def mamdgpu_debugger_abi : Joined<["-"], "mamdgpu-debugger-abi=">,
-  Flags<[HelpHidden]>,
-  Group,
-  HelpText<"Generate additional code for specified  of debugger ABI 
(AMDGPU only)">,
-  MetaVarName<"">;
-
 def mcode_object_v3 : Flag<["-"], "mcode-object-v3">, 
Group,
   HelpText<"Enable code object v3 (AMDGPU only)">;
 def mno_code_object_v3 : Flag<["-"], "mno-code-object-v3">, 
Group,

diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp 
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index 6781045886f2..5df7236f0223 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -365,9 +365,6 @@ void amdgpu::getAMDGPUTargetFeatures(const Driver &D,
  const llvm::Triple &Triple,
  const llvm::opt::ArgList &Args,
  std::vector &Features) {
-  if (const Arg *dAbi = Args.getLastArg(options::OPT_mamdgpu_debugger_abi))
-D.Diag(diag::err_drv_clang_unsupported) << dAbi->getAsString(Args);
-
   // Add target ID features to -target-feature options. No diagnostics should
   // be emitted here since invalid target ID is diagnosed at other places.
   StringRef TargetID = Args.getLastArgValue(options::OPT_mcpu_EQ);

diff  --git a/clang/test/Driver/amdgpu-features.c 
b/clang/test/Driver/amdgpu-features.c
index 71fd63715e00..17142ae23d6d 100644
--- a/clang/test/Driver/amdgpu-features.c
+++ b/clang/test/Driver/amdgpu-features.c
@@ -1,11 +1,3 @@
-// RUN: %clang -### -target amdgcn -x cl -S -emit-llvm -mcpu=kaveri 
-mamdgpu-debugger-abi=0.0 %s -o - 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-MAMDGPU-DEBUGGER-ABI-0-0 %s
-// CHECK-MAMDGPU-DEBUGGER-ABI-0-0: the clang compiler does not support 
'-mamdgpu-debugger-abi=0.0'
-
-// RUN: %clang -### -target amdgcn -x cl -S -emit-llvm -mcpu=kaveri 
-mamdgpu-debugger-abi=1.0 %s -o - 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-MAMDGPU-DEBUGGER-ABI-1-0 %s
-// CHECK-MAMDGPU-DEBUGGER-ABI-1-0: the clang compiler does not support 
'-mamdgpu-debugger-abi=1.0'
-
 // RUN: %clang -### -target amdgcn -mcpu=gfx700 -mcode-object-v3 %s 2>&1 | 
FileCheck --check-prefix=CODE-OBJECT-V3 %s
 // CODE-OBJECT-V3: "-target-feature" "+code-object-v3"
 



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

Re: [PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Adrian McCarthy via cfe-commits]

If I had to guess, my money would be on a deadlock.  To unblock, I'd
propose reverting this patch until we can figure it out.

During the review, a deadlock was fixed related to the watcher thread, but
perhaps we missed one for the notifier thread.

On Tue, Oct 13, 2020 at 8:40 AM Hans Wennborg via Phabricator <
revi...@reviews.llvm.org> wrote:

> hans added a comment.
>
> The longer timeout didn't help :(
>
> I'm not sure what's different about the machine where this is failing.
> Maybe it's some filesystem issue due to being a VM?
>
> Any ideas for good printfs or similar that could be added to figure out
> exactly what part is failing?
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D88666/new/
>
> https://reviews.llvm.org/D88666
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

I've also been seeing some failures on phab reviews, e.g. 
https://reviews.llvm.org/D89188.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88666

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

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D76620#2327910 , @erichkeane wrote:

> In D76620#2327901 , @rjmccall wrote:
>
>> You know on both sides that a lambda is used as a kernel, yes?  Why not 
>> simply introduce that into the mangling of lambdas, so that the subset of 
>> lambdas used as kernels form a stable sequence?
>
> Coming up with said stable sequence is somewhat difficult as well.  A strict 
> integer ordering didn't end up being stable, as some of the kernel handling 
> can cause instantiations to happen in a slightly different ordering, which 
> messed that up, as would use of the builtin. We wanted to find a way that was 
> dependent on the source code alone.  The "Quick and Dirty" solution was 
> line/column numbers, though we considered a hash of that same information to 
> at least make it shorter.

Certainly you wouldn't want a *global* sequence ID.  However, lambdas can't 
just occur globally, they're always part of some declaration that they can be 
scoped to, so that you have e.g. "the third kernel lambda within function X".  
I fail to see how that wouldn't address the concern about instantiation order, 
and it's still source-directed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D76620#2327976 , @rjmccall wrote:

> In D76620#2327910 , @erichkeane 
> wrote:
>
>> In D76620#2327901 , @rjmccall wrote:
>>
>>> You know on both sides that a lambda is used as a kernel, yes?  Why not 
>>> simply introduce that into the mangling of lambdas, so that the subset of 
>>> lambdas used as kernels form a stable sequence?
>>
>> Coming up with said stable sequence is somewhat difficult as well.  A strict 
>> integer ordering didn't end up being stable, as some of the kernel handling 
>> can cause instantiations to happen in a slightly different ordering, which 
>> messed that up, as would use of the builtin. We wanted to find a way that 
>> was dependent on the source code alone.  The "Quick and Dirty" solution was 
>> line/column numbers, though we considered a hash of that same information to 
>> at least make it shorter.
>
> Certainly you wouldn't want a *global* sequence ID.  However, lambdas can't 
> just occur globally, they're always part of some declaration that they can be 
> scoped to, so that you have e.g. "the third kernel lambda within function X". 
>  I fail to see how that wouldn't address the concern about instantiation 
> order, and it's still source-directed.

Hmm... I'll have to consider that.  That is an interesting thought. Presumably 
we could just copy the 'getLambdaManglingNumber' stuff in that case, and place 
the value in the same location in the mangling, with some sort of discriminator 
(to avoid conflicting manglings).  The only other concern I have is WHEN we 
know that a lambda is used in a kernel, which we don't until it is called (and 
can cause confusion when it is a template parameter and called later).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D88913: [FPEnv] Use strictfp metadata in casting nodes

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added inline comments.



Comment at: clang/lib/CodeGen/CodeGenFunction.h:618-619
 FPOptions OldFPFeatures;
+llvm::fp::ExceptionBehavior OldExcept;
+llvm::RoundingMode OldRounding;
 Optional FMFGuard;

sepavloff wrote:
> Is it possible to merge this variables into `OldFPFeatures`? In what cases 
> state of `Builder` may differ from the state of `CGF.CurFPFeatures`?
Without this patch they will differ if the AST changes CGF.CurFPFeatures and 
the Builder hasn't been updated yet. Say, because we're going back up the tree.

The concern is that we'll miss places that need to be updated to respect the 
AST. But if we don't make a point of resetting the Builder then we'll end up 
with stale data in the Builder that "just happens" to work. We need tests to 
actually test what they are supposed to test without relying on stale data.

The comments in the tests about "maytrap" being wrong show us that we 
absolutely have exactly this bug in multiple places. The Builder isn't getting 
properly updated and this change makes that visible and clear.



Comment at: clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c:6
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon 
-target-feature +fullfp16 -target-feature +v8.2a\
-// RUN: -ffp-exception-behavior=strict \
+// RUN: -ffp-exception-behavior=maytrap -DEXCEPT=1 \
 // RUN: -fexperimental-strict-floating-point \

sepavloff wrote:
> Why did you change exception behavior to `maytrap`?
Because "maytrap" is not the IRBuilder's default of "strict" and it isn't 
clang's default of "ignore".

The #pragma sets the exception handling to "strict". Having the command line 
arguments set a global "maytrap" with a #pragma that sets it to "strict" at the 
top of the file means that places in clang that need to be updated will become 
visible.



Comment at: clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c:26
+// metadata from the AST instead of the global default from the command line.
+// FIXME: All cases of "fpexcept.maytrap" in this test are wrong.
+

sepavloff wrote:
> Why they are wrong?
Because the #pragma covers the entire file and sets exception handling to 
"strict". Thus all constrained intrinsic calls should be "strict", and if they 
are "maytrap" or "ignore" then we have a bug.


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

https://reviews.llvm.org/D88913

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

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D76620#2327988 , @erichkeane wrote:

> In D76620#2327976 , @rjmccall wrote:
>
>> In D76620#2327910 , @erichkeane 
>> wrote:
>>
>>> In D76620#2327901 , @rjmccall 
>>> wrote:
>>>
 You know on both sides that a lambda is used as a kernel, yes?  Why not 
 simply introduce that into the mangling of lambdas, so that the subset of 
 lambdas used as kernels form a stable sequence?
>>>
>>> Coming up with said stable sequence is somewhat difficult as well.  A 
>>> strict integer ordering didn't end up being stable, as some of the kernel 
>>> handling can cause instantiations to happen in a slightly different 
>>> ordering, which messed that up, as would use of the builtin. We wanted to 
>>> find a way that was dependent on the source code alone.  The "Quick and 
>>> Dirty" solution was line/column numbers, though we considered a hash of 
>>> that same information to at least make it shorter.
>>
>> Certainly you wouldn't want a *global* sequence ID.  However, lambdas can't 
>> just occur globally, they're always part of some declaration that they can 
>> be scoped to, so that you have e.g. "the third kernel lambda within function 
>> X".  I fail to see how that wouldn't address the concern about instantiation 
>> order, and it's still source-directed.
>
> Hmm... I'll have to consider that.  That is an interesting thought. 
> Presumably we could just copy the 'getLambdaManglingNumber' stuff in that 
> case, and place the value in the same location in the mangling, with some 
> sort of discriminator (to avoid conflicting manglings).

The Itanium mangling already produces a different lambda mangling number for 
different lambda signatures.  You just need the kernel-ness to be part of the 
signature.

> The only other concern I have is WHEN we know that a lambda is used in a 
> kernel, which we don't until it is called (and can cause confusion when it is 
> a template parameter and called later).

Oh, yes, if you don't know that a lambda is used as a kernel locally then this 
falls apart a bit.  You could of course pretend for ABI purposes that there's a 
new intermediate lambda at the kernel use point when the lambda is not local to 
the current function.  I don't think there's anything which relies on mangling 
lambdas before the function they're contained within is fully type-checked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D76620#2328011 , @rjmccall wrote:

> In D76620#2327988 , @erichkeane 
> wrote:
>
>> In D76620#2327976 , @rjmccall wrote:
>>
>>> In D76620#2327910 , @erichkeane 
>>> wrote:
>>>
 In D76620#2327901 , @rjmccall 
 wrote:

> You know on both sides that a lambda is used as a kernel, yes?  Why not 
> simply introduce that into the mangling of lambdas, so that the subset of 
> lambdas used as kernels form a stable sequence?

 Coming up with said stable sequence is somewhat difficult as well.  A 
 strict integer ordering didn't end up being stable, as some of the kernel 
 handling can cause instantiations to happen in a slightly different 
 ordering, which messed that up, as would use of the builtin. We wanted to 
 find a way that was dependent on the source code alone.  The "Quick and 
 Dirty" solution was line/column numbers, though we considered a hash of 
 that same information to at least make it shorter.
>>>
>>> Certainly you wouldn't want a *global* sequence ID.  However, lambdas can't 
>>> just occur globally, they're always part of some declaration that they can 
>>> be scoped to, so that you have e.g. "the third kernel lambda within 
>>> function X".  I fail to see how that wouldn't address the concern about 
>>> instantiation order, and it's still source-directed.
>>
>> Hmm... I'll have to consider that.  That is an interesting thought. 
>> Presumably we could just copy the 'getLambdaManglingNumber' stuff in that 
>> case, and place the value in the same location in the mangling, with some 
>> sort of discriminator (to avoid conflicting manglings).
>
> The Itanium mangling already produces a different lambda mangling number for 
> different lambda signatures.  You just need the kernel-ness to be part of the 
> signature.

It does it via the getLambdaManglingNumber I believe, right?  The idea would be 
to have a getKernelLambdaManglingNumber that does something similar, and needs 
something to avoid conflicting, which is the discriminator I was mentioning.

>> The only other concern I have is WHEN we know that a lambda is used in a 
>> kernel, which we don't until it is called (and can cause confusion when it 
>> is a template parameter and called later).
>
> Oh, yes, if you don't know that a lambda is used as a kernel locally then 
> this falls apart a bit.  You could of course pretend for ABI purposes that 
> there's a new intermediate lambda at the kernel use point when the lambda is 
> not local to the current function.  I don't think there's anything which 
> relies on mangling lambdas before the function they're contained within is 
> fully type-checked.

One thing that might save us from this, is we aren't actually modifying the 
LAMBDAs mangling, we're modifying the KERNEL's mangling. The lambda itself 
keeps its mangling, but the kernel that calls it (which is the device/host 
boundary) is what has its name changed.  So at least at that point we know some 
about it.  And, actually, since we are generating the 'integeration header' 
lookup table AND the kernel in identical (if not the same) invocation, perhaps 
a global counter WOULD work...  My coworker is investigating this now, so 
hopefully she'll be able to prove out this idea and give further feedback here.

Thank you so much for the discussion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76620

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

[PATCH] D89318: [ASTImporter] Fix crash caused by unset AttributeSpellingListIndex

[Shafik Yaghmour via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/lib/AST/ASTImporter.cpp:8109
+  FromAttr->getAttributeSpellingListIndex());
+  ToAttr->setPackExpansion(FromAttr->isPackExpansion());
+  ToAttr->setImplicit(FromAttr->isImplicit());

Why move these two but not `setInherited(...)` it is not really explained in 
the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89318

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

[PATCH] D89184: Support complex target features combinations

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added a subscriber: echristo.
tra added a comment.
This revision is now accepted and ready to land.

@echristo : FYI, just in case you have an opinion on bringing required feature 
constraint checks one step closer to being Turing-complete. :-)

LGTM as far as syntax and use for NVPTX goes. No opinion on whether it 
**should** go into codegen.
While the change appears to be sensible on general principles, I'm not aware of 
any specific case where it's actually needed.
It would help if you could elaborate on why you need this functionality.




Comment at: clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp:16
+  };
+  ASSERT_TRUE(doCheck("A|B,C|D", "A"));
+  ASSERT_FALSE(doCheck("(A|B),(C|D)", "A"));

LiuChen3 wrote:
> pengfei wrote:
> > tra wrote:
> > > Is `doCheck("A,B,C,D", ...)`  expected to work? I'd add test cases for 
> > > that, too.
> > > 
> > > 
> > I may not get your point here.
> > We added the unittest to check the correctness for the function. It's an 
> > easier and more flexible way checking for complex functions than using llc.
> > I don't know if there's criteria for the unittests.
> I think @tra 's mean is adding tests like ASSERT_FALSE(doCheck("A,B,C,D", 
> "A")); .
Yes. I should've phrased it better.  I meant tests w/o parens in the feature 
requirements.


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

https://reviews.llvm.org/D89184

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

[PATCH] D89184: Support complex target features combinations

[Artem Belevich via Phabricator via cfe-commits]

tra requested changes to this revision.
tra added a comment.
This revision now requires changes to proceed.

Sorry, I didn't mean to stamp the change as LGTM overall yet. Can't find a way 
to un-LGTM, so marking it as `Request Changes` for now.


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

https://reviews.llvm.org/D89184

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

[PATCH] D89332: Always allow std::function to be copied.

[Felix Berger via Phabricator via cfe-commits]

flx created this revision.
flx added a reviewer: aaron.ballman.
Herald added a project: clang.
flx requested review of this revision.

Since its call operator is const but can modify the state of its underlying
functor we cannot tell whether the copy is necessary or not.

This avoids false positives.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89332

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -405,3 +405,23 @@
   ExpensiveToCopyType Orig;
   const ExpensiveToCopyType Copy = freeFunctionWithDefaultArg(&Orig);
 }
+
+namespace std {
+
+template 
+class function;
+template 
+class function {
+public:
+  function();
+  function(const function &other);
+  R operator()(Args... args) const;
+};
+
+} // namespace std
+
+void negativeStdFunction() {
+  std::function Orig;
+  std::function Copy = Orig;
+  int i = Orig();
+}
Index: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -19,6 +19,10 @@
 namespace performance {
 namespace {
 
+using namespace ::clang::ast_matchers;
+using ::clang::ast_matchers::internal::Matcher;
+using utils::decl_ref_expr::isOnlyUsedAsConst;
+
 void recordFixes(const VarDecl &Var, ASTContext &Context,
  DiagnosticBuilder &Diagnostic) {
   Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context);
@@ -29,10 +33,17 @@
   }
 }
 
-} // namespace
+// Always allow std::function to be copied. Since its call operator is const 
but
+// can modify the state of the underlying functor we cannot ell whether the 
copy
+// is unnecessary.
+AST_MATCHER(NamedDecl, isStdFunction) {
+  // First use the fast getName() method to avoid unnecessary calls to the
+  // slow getQualifiedNameAsString().
+  return Node.getName() == "function" &&
+ Node.getQualifiedNameAsString() == "std::function";
+}
 
-using namespace ::clang::ast_matchers;
-using utils::decl_ref_expr::isOnlyUsedAsConst;
+} // namespace
 
 UnnecessaryCopyInitialization::UnnecessaryCopyInitialization(
 StringRef Name, ClangTidyContext *Context)
@@ -57,7 +68,7 @@
unless(callee(cxxMethodDecl(
   .bind("initFunctionCall");
 
-  auto localVarCopiedFrom = [this](const internal::Matcher &CopyCtorArg) 
{
+  auto localVarCopiedFrom = [this](const Matcher &CopyCtorArg) {
 return compoundStmt(
forEachDescendant(
declStmt(
@@ -66,8 +77,9 @@
hasCanonicalType(
matchers::isExpensiveToCopy()),
unless(hasDeclaration(namedDecl(
-   matchers::matchesAnyListedName(
-   AllowedTypes)),
+   
anyOf(matchers::matchesAnyListedName(
+ AllowedTypes),
+ isStdFunction())),
unless(isImplicit()),
hasInitializer(traverse(
ast_type_traits::TK_AsIs,


Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -405,3 +405,23 @@
   ExpensiveToCopyType Orig;
   const ExpensiveToCopyType Copy = freeFunctionWithDefaultArg(&Orig);
 }
+
+namespace std {
+
+template 
+class function;
+template 
+class function {
+public:
+  function();
+  function(const function &other);
+  R operator()(Args... args) const;
+};
+
+} // namespace std
+
+void negativeStdFunction() {
+  std::function Orig;
+  std::function Copy = Orig;
+  int i = Orig();
+}
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -19,6 +19,10 @@
 namespace pe

[PATCH] D89184: Support complex target features combinations

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

This is needed for D89105  and was split from 
it.


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

https://reviews.llvm.org/D89184

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

[PATCH] D88314: Added llvm-string-referencing check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D88314#2322870 , @bogser01 wrote:

> @aaron.ballman Thank you for picking up this review! Running the check over 
> the entire LLVM causes ~74K warnings across 430 files. As to the false 
> positive rate it's tricky to measure. Based on previous analysis on //flang// 
> codebase, I would say roughly 50% of the fixes would cause compiler errors 
> when applied.

Woof, that's a pretty high false-positive rate for the check -- I don't think 
we should issue a fix-it for this case because anyone who accidentally applies 
fixits automatically will be in for a fair amount of pain.

I'm a bit worried that the number of diagnostics this produces will basically 
mean the check has to be turned off for the only project that would use it. 
What is the expected use case for the check? For instance, are you expecting to 
craft a .clang-tidy file to disable this check on source files in the repo that 
are known to have a lot of diagnostics (so that the check mostly only fires for 
known-good files and new files)?




Comment at: clang-tools-extra/clang-tidy/add_new_check.py:139
   const auto *MatchedDecl = Result.Nodes.getNodeAs("x");
+<<< HEAD
+  if ((!MatchedDecl->getIdentifier()) || 
MatchedDecl->getName().startswith("awesome_"))

Looks like there are some accidental merge markers in the patch.



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:204
   void AwesomeFunctionNamesCheck::check(const MatchFinder::MatchResult 
&Result) {
+<<< HEAD
+const auto *MatchedDecl = Result.Nodes.getNodeAs("x");

More merge conflict markers.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/llvm-string-referencing.cpp:25
+void f1(const string &P);
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter of type 'const 
std::string &' could potentially be replaced with 'llvm::StringRef' 
[llvm-string-referencing]
+// CHECK-FIXES: void f1(llvm::StringRef P);{{$}}

You can drop the name of the check from the `CHECK-MESSAGES` line (we usually 
only check the full diagnostic once).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88314

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

[clang] b76dc11 - [analyzer] NFC: Move IssueHash to libAnalysis.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2020-10-13T10:53:10-07:00
New Revision: b76dc111dd02672488df794570d82e3edbbfa5d8

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

LOG: [analyzer] NFC: Move IssueHash to libAnalysis.

IssueHash is an attempt to introduce stable warning identifiers
that won't change when code around them gets moved around.
Path diagnostic consumers print issue hashes for the emitted diagnostics.

This move will allow us to ultimately move path diagnostic consumers
to libAnalysis.

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

Added: 
clang/include/clang/Analysis/IssueHash.h
clang/lib/Analysis/IssueHash.cpp

Modified: 
clang/lib/Analysis/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
clang/lib/StaticAnalyzer/Core/CMakeLists.txt
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

Removed: 
clang/include/clang/StaticAnalyzer/Core/IssueHash.h
clang/lib/StaticAnalyzer/Core/IssueHash.cpp



diff  --git a/clang/include/clang/Analysis/IssueHash.h 
b/clang/include/clang/Analysis/IssueHash.h
new file mode 100644
index ..9c02b79f58f9
--- /dev/null
+++ b/clang/include/clang/Analysis/IssueHash.h
@@ -0,0 +1,49 @@
+//===-- IssueHash.h - Generate identification hashes *- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#ifndef LLVM_CLANG_STATICANALYZER_CORE_ISSUE_HASH_H
+#define LLVM_CLANG_STATICANALYZER_CORE_ISSUE_HASH_H
+
+#include "llvm/ADT/SmallString.h"
+
+namespace clang {
+class Decl;
+class FullSourceLoc;
+class LangOptions;
+
+/// Returns an opaque identifier for a diagnostic.
+///
+/// This opaque identifier is intended to be stable even when the source code
+/// is changed. It allows to track diagnostics in the long term, for example,
+/// find which diagnostics are "new", maintain a database of suppressed
+/// diagnostics etc.
+///
+/// We may introduce more variants of issue hashes in the future
+/// but older variants will still be available for compatibility.
+///
+/// This hash is based on the following information:
+///   - Name of the checker that emitted the diagnostic.
+///   - Warning message.
+///   - Name of the enclosing declaration.
+///   - Contents of the line of code with the issue, excluding whitespace.
+///   - Column number (but not the line number! - which makes it stable).
+llvm::SmallString<32> getIssueHash(const FullSourceLoc &IssueLoc,
+   llvm::StringRef CheckerName,
+   llvm::StringRef WarningMessage,
+   const Decl *IssueDecl,
+   const LangOptions &LangOpts);
+
+/// Get the unhashed string representation of the V1 issue hash.
+/// When hashed, it becomes the actual issue hash. Useful for testing.
+/// See GetIssueHashV1() for more information.
+std::string getIssueString(const FullSourceLoc &IssueLoc,
+   llvm::StringRef CheckerName,
+   llvm::StringRef WarningMessage,
+   const Decl *IssueDecl, const LangOptions &LangOpts);
+} // namespace clang
+
+#endif

diff  --git a/clang/include/clang/StaticAnalyzer/Core/IssueHash.h 
b/clang/include/clang/StaticAnalyzer/Core/IssueHash.h
deleted file mode 100644
index 38d5f847fc29..
--- a/clang/include/clang/StaticAnalyzer/Core/IssueHash.h
+++ /dev/null
@@ -1,50 +0,0 @@
-//===-- IssueHash.h - Generate identification hashes *- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_ISSUE_HASH_H
-#define LLVM_CLANG_STATICANALYZER_CORE_ISSUE_HASH_H
-
-#include "llvm/ADT/SmallString.h"
-
-namespace clang {
-class Decl;
-class SourceManager;
-class FullSourceLoc;
-class LangOptions;
-
-/// Get an MD5 hash to help identify bugs.
-///
-/// This function returns a hash that helps identify bugs within a source file.
-/// This identification can be utilized to 
diff  diagnostic results on 
diff erent
-/// snapshots of a projects, or maintain a database of suppressed diagnotics.
-///
-/// The hash contains the normalized text of the location associated with the
-/// diagnostic. Normalization means removing the whitespa

[clang] 44b7cf2 - [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2020-10-13T10:53:10-07:00
New Revision: 44b7cf2983b6a8373c99a9b254f8c3f944e03f35

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

LOG: [analyzer] NFC: Move path diagnostic consumer implementations to 
libAnalysis.

With this change, we're more or less ready to allow users outside
of the Static Analyzer to take advantage of path diagnostic consumers
for emitting their warnings in different formats.

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

Added: 
clang/include/clang/Analysis/PathDiagnosticConsumers.def
clang/include/clang/Analysis/PathDiagnosticConsumers.h
clang/lib/Analysis/HTMLPathDiagnosticConsumer.cpp
clang/lib/Analysis/PlistHTMLPathDiagnosticConsumer.cpp
clang/lib/Analysis/PlistPathDiagnosticConsumer.cpp
clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp
clang/lib/Analysis/TextPathDiagnosticConsumer.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/Analyses.def
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
clang/include/clang/module.modulemap
clang/lib/Analysis/CMakeLists.txt
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/StaticAnalyzer/Core/CMakeLists.txt
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Removed: 
clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp



diff  --git a/clang/include/clang/Analysis/PathDiagnosticConsumers.def 
b/clang/include/clang/Analysis/PathDiagnosticConsumers.def
new file mode 100644
index ..33d2072fcf31
--- /dev/null
+++ b/clang/include/clang/Analysis/PathDiagnosticConsumers.def
@@ -0,0 +1,50 @@
+//===-- PathDiagnosticConsumers.def - Visualizing warnings --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines the set of path diagnostic consumers - objects that
+// implement 
diff erent representations of static analysis results.
+//
+//===--===//
+
+#ifndef ANALYSIS_DIAGNOSTICS
+#define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)
+#endif
+
+ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML",
+ createHTMLDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(
+HTML_SINGLE_FILE, "html-single-file",
+"Output analysis results using HTML (not allowing for multi-file bugs)",
+createHTMLSingleFileDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists",
+ createPlistDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(
+PLIST_MULTI_FILE, "plist-multi-file",
+"Output analysis results using Plists (allowing for multi-file bugs)",
+createPlistMultiFileDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html",
+ "Output analysis results using HTML wrapped with Plists",
+ createPlistHTMLDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(SARIF, "sarif", "Output analysis results in a SARIF file",
+ createSarifDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(TEXT, "text", "Text output of analysis results to stderr",
+ createTextPathDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(TEXT_MINIMAL, "text-minimal",
+ "Emits minimal diagnostics to stderr, stating only the "
+ "warning message and the associated notes. Usually "
+ "used in addition to other analysis types",
+ createTextMinimalPathDiagnosticConsumer)
+
+#undef ANALYSIS_DIAGNOSTICS

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h 
b/clang/include/clang/Analysis/PathDiagnosticConsumers.h
similarity index 88%
rename from clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
rename to clang/include/clang/Analysis/PathDiagnosticConsumers.h
index 4b63812b2f83..9b309e6d3607 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
+++ b/clang/include/clang/Analysis/PathDiagnosticConsumers.h
@@ -16,6 +16,8 @@
 #include 
 #include 
 
+#include "clang/Analysis/PathDiagnostic.h"
+
 namespace clang {
 
 class AnalyzerOptions;
@@ -27,14 +29,14 @@ class CrossTranslationUnitContext;
 namespace ento

[PATCH] D67420: [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfd4b3f123d6e: [analyzer] NFC: Separate 
PathDiagnosticConsumer options from AnalyzerOptions. (authored by dergachev.a).
Herald added a subscriber: steakhal.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67420

Files:
  clang/include/clang/Analysis/PathDiagnostic.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -150,7 +150,7 @@
   break;
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)\
   case PD_##NAME:  \
-CREATEFN(*Opts.get(), PathConsumers, OutDir, PP, CTU); \
+CREATEFN(Opts->getDiagOpts(), PathConsumers, OutDir, PP, CTU); \
 break;
 #include "clang/StaticAnalyzer/Core/Analyses.def"
 default:
Index: clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
@@ -34,20 +34,17 @@
 /// type to the standard error, or to to compliment many others. Emits detailed
 /// diagnostics in textual format for the 'text' output type.
 class TextDiagnostics : public PathDiagnosticConsumer {
+  PathDiagnosticConsumerOptions DiagOpts;
   DiagnosticsEngine &DiagEng;
   const LangOptions &LO;
-  const bool IncludePath = false;
-  const bool ShouldEmitAsError = false;
-  const bool ApplyFixIts = false;
-  const bool ShouldDisplayCheckerName = false;
+  bool ShouldDisplayPathNotes;
 
 public:
-  TextDiagnostics(DiagnosticsEngine &DiagEng, const LangOptions &LO,
-  bool ShouldIncludePath, const AnalyzerOptions &AnOpts)
-  : DiagEng(DiagEng), LO(LO), IncludePath(ShouldIncludePath),
-ShouldEmitAsError(AnOpts.AnalyzerWerror),
-ApplyFixIts(AnOpts.ShouldApplyFixIts),
-ShouldDisplayCheckerName(AnOpts.ShouldDisplayCheckerNameForText) {}
+  TextDiagnostics(PathDiagnosticConsumerOptions DiagOpts,
+  DiagnosticsEngine &DiagEng, const LangOptions &LO,
+  bool ShouldDisplayPathNotes)
+  : DiagOpts(std::move(DiagOpts)), DiagEng(DiagEng), LO(LO),
+ShouldDisplayPathNotes(ShouldDisplayPathNotes) {}
   ~TextDiagnostics() override {}
 
   StringRef getName() const override { return "TextDiagnostics"; }
@@ -56,13 +53,13 @@
   bool supportsCrossFileDiagnostics() const override { return true; }
 
   PathGenerationScheme getGenerationScheme() const override {
-return IncludePath ? Minimal : None;
+return ShouldDisplayPathNotes ? Minimal : None;
   }
 
   void FlushDiagnosticsImpl(std::vector &Diags,
 FilesMade *filesMade) override {
 unsigned WarnID =
-ShouldEmitAsError
+DiagOpts.ShouldDisplayWarningsAsErrors
 ? DiagEng.getCustomDiagID(DiagnosticsEngine::Error, "%0")
 : DiagEng.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
 unsigned NoteID = DiagEng.getCustomDiagID(DiagnosticsEngine::Note, "%0");
@@ -72,7 +69,7 @@
 auto reportPiece = [&](unsigned ID, FullSourceLoc Loc, StringRef String,
ArrayRef Ranges,
ArrayRef Fixits) {
-  if (!ApplyFixIts) {
+  if (!DiagOpts.ShouldApplyFixIts) {
 DiagEng.Report(Loc, ID) << String << Ranges << Fixits;
 return;
   }
@@ -92,9 +89,10 @@
  E = Diags.end();
  I != E; ++I) {
   const PathDiagnostic *PD = *I;
-  std::string WarningMsg =
-  (ShouldDisplayCheckerName ? " [" + PD->getCheckerName() + "]" : "")
-  .str();
+  std::string WarningMsg = (DiagOpts.ShouldDisplayDiagnosticName
+? " [" + PD->getCheckerName() + "]"
+: "")
+   .str();
 
   reportPiece(WarnID, PD->getLocation().asLocation(),
   (PD->getShortDescription() + WarningMsg).str(),
@@ -110,7 +108,7 @@
 Piece->getFixits());
   }
 
-  if (!IncludePath)
+  if (!ShouldDisplayPathNotes)
 continue;
 
   // Then, add the path notes if necessary.
@@ -125,7 +123,7 @@
   }
 }
 
-if (!ApplyFixIts || Repls.empty())
+ 

[clang] fd4b3f1 - [analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2020-10-13T10:53:10-07:00
New Revision: fd4b3f123d6e64769881e4c6351d5bbbdac30ce3

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

LOG: [analyzer] NFC: Separate PathDiagnosticConsumer options from 
AnalyzerOptions.

The AnalyzerOptions object contains too much information that's
entirely specific to the Analyzer. It is also being referenced by
path diagnostic consumers to tweak their behavior. In order for path
diagnostic consumers to function separately from the analyzer,
make a smaller options object that only contains relevant options.

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

Added: 


Modified: 
clang/include/clang/Analysis/PathDiagnostic.h
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/PathDiagnostic.h 
b/clang/include/clang/Analysis/PathDiagnostic.h
index c4b042a51bb5..544e9f4662d3 100644
--- a/clang/include/clang/Analysis/PathDiagnostic.h
+++ b/clang/include/clang/Analysis/PathDiagnostic.h
@@ -58,6 +58,47 @@ namespace ento {
 
 class PathDiagnostic;
 
+/// These options tweak the behavior of path diangostic consumers.
+/// Most of these options are currently supported by very few consumers.
+struct PathDiagnosticConsumerOptions {
+  /// Run-line of the tool that produced the diagnostic.
+  /// It can be included with the diagnostic for debugging purposes.
+  std::string ToolInvocation;
+
+  /// Whether to include additional information about macro expansions
+  /// with the diagnostics, because otherwise they can be hard to obtain
+  /// without re-compiling the program under analysis.
+  bool ShouldDisplayMacroExpansions;
+
+  /// Whether to include LLVM statistics of the process in the diagnostic.
+  /// Useful for profiling the tool on large real-world codebases.
+  bool ShouldSerializeStats;
+
+  /// If the consumer intends to produce multiple output files, should it
+  /// use randomly generated file names for these files (with the tiny risk of
+  /// having random collisions) or deterministic human-readable file names
+  /// (with a larger risk of deterministic collisions or invalid characters
+  /// in the file name). We should not really give this choice to the users
+  /// because deterministic mode is always superior when done right, but
+  /// for some consumers this mode is experimental and needs to be
+  /// off by default.
+  bool ShouldWriteStableReportFilename;
+
+  /// Whether the consumer should treat consumed diagnostics as hard errors.
+  /// Useful for breaking your build when issues are found.
+  bool ShouldDisplayWarningsAsErrors;
+
+  /// Whether the consumer should attempt to rewrite the source file
+  /// with fix-it hints attached to the diagnostics it consumes.
+  bool ShouldApplyFixIts;
+
+  /// Whether the consumer should present the name of the entity that emitted
+  /// the diagnostic (eg., a checker) so that the user knew how to disable it.
+  bool ShouldDisplayDiagnosticName;
+
+  PathDiagnosticConsumerOptions() = delete;
+};
+
 class PathDiagnosticConsumer {
 public:
   class PDFileEntry : public llvm::FoldingSetNode {

diff  --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h 
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 4907b0757a8a..e1093772e02c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_STATICANALYZER_CORE_ANALYZEROPTIONS_H
 #define LLVM_CLANG_STATICANALYZER_CORE_ANALYZEROPTIONS_H
 
+#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
@@ -255,7 +256,7 @@ class AnalyzerOptions : public 
RefCountedBase {
   unsigned NoRetryExhausted : 1;
 
   /// Emit analyzer warnings as errors.
-  unsigned AnalyzerWerror : 1;
+  bool AnalyzerWerror : 1;
 
   /// The inlining stack depth limit.
   // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls).
@@ -390,6 +391,16 @@ class AnalyzerOptions : public 
RefCountedBase {
   ///
   /// \sa CXXMemberInliningMode
   bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const;
+
+  ento::PathDiagnosticConsumerOptions getDiagOpts() const {
+return {FullCompilerInvocation,
+ShouldDisplayMacroExpansions,
+ShouldSerializeStats,
+ShouldWrit

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Artem Dergachev via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG44b7cf2983b6: [analyzer] NFC: Move path diagnostic consumer 
implementations to libAnalysis. (authored by dergachev.a).

Changed prior to commit:
  https://reviews.llvm.org/D67422?vs=285417&id=297910#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67422

Files:
  clang/include/clang/Analysis/PathDiagnosticConsumers.def
  clang/include/clang/Analysis/PathDiagnosticConsumers.h
  clang/include/clang/StaticAnalyzer/Core/Analyses.def
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
  clang/include/clang/module.modulemap
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/HTMLPathDiagnosticConsumer.cpp
  clang/lib/Analysis/PlistHTMLPathDiagnosticConsumer.cpp
  clang/lib/Analysis/PlistPathDiagnosticConsumer.cpp
  clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp
  clang/lib/Analysis/TextPathDiagnosticConsumer.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -21,6 +21,7 @@
 #include "clang/Analysis/CallGraph.h"
 #include "clang/Analysis/CodeInjector.h"
 #include "clang/Analysis/PathDiagnostic.h"
+#include "clang/Analysis/PathDiagnosticConsumers.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -30,7 +31,6 @@
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "llvm/ADT/PostOrderIterator.h"
@@ -152,7 +152,7 @@
   case PD_##NAME:  \
 CREATEFN(Opts->getDiagOpts(), PathConsumers, OutDir, PP, CTU); \
 break;
-#include "clang/StaticAnalyzer/Core/Analyses.def"
+#include "clang/Analysis/PathDiagnosticConsumers.def"
 default:
   llvm_unreachable("Unknown analyzer output type!");
 }
Index: clang/lib/StaticAnalyzer/Core/CMakeLists.txt
===
--- clang/lib/StaticAnalyzer/Core/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Core/CMakeLists.txt
@@ -30,16 +30,13 @@
   ExprEngineCallAndReturn.cpp
   ExprEngineObjC.cpp
   FunctionSummary.cpp
-  HTMLDiagnostics.cpp
   LoopUnrolling.cpp
   LoopWidening.cpp
   MemRegion.cpp
-  PlistDiagnostics.cpp
   ProgramState.cpp
   RangeConstraintManager.cpp
   RangedConstraintManager.cpp
   RegionStore.cpp
-  SarifDiagnostics.cpp
   SimpleConstraintManager.cpp
   SimpleSValBuilder.cpp
   SMTConstraintManager.cpp
@@ -47,7 +44,6 @@
   SValBuilder.cpp
   SVals.cpp
   SymbolManager.cpp
-  TextDiagnostics.cpp
   WorkList.cpp
 
   LINK_LIBS
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -301,7 +301,7 @@
 AnalysisDiagClients Value = llvm::StringSwitch(Name)
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATFN) \
   .Case(CMDFLAG, PD_##NAME)
-#include "clang/StaticAnalyzer/Core/Analyses.def"
+#include "clang/Analysis/PathDiagnosticConsumers.def"
   .Default(NUM_ANALYSIS_DIAG_CLIENTS);
 if (Value == NUM_ANALYSIS_DIAG_CLIENTS) {
   Diags.Report(diag::err_drv_invalid_value)
Index: clang/lib/Analysis/TextPathDiagnosticConsumer.cpp
===
--- clang/lib/Analysis/TextPathDiagnosticConsumer.cpp
+++ clang/lib/Analysis/TextPathDiagnosticConsumer.cpp
@@ -1,4 +1,4 @@
-//===--- TextDiagnostics.cpp - Text Diagnostics for Paths ---*- C++ -*-===//
+//===--- TextPathDiagnosticConsumer.cpp - Text Diagnostics --*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,19 +6,18 @@
 //
 //===-

[PATCH] D67421: [analyzer] NFC: Move IssueHash to libAnalysis.

[Artem Dergachev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb76dc111dd02: [analyzer] NFC: Move IssueHash to libAnalysis. 
(authored by dergachev.a).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67421

Files:
  clang/include/clang/Analysis/IssueHash.h
  clang/include/clang/StaticAnalyzer/Core/IssueHash.h
  clang/lib/Analysis/CMakeLists.txt
  clang/lib/Analysis/IssueHash.cpp
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/IssueHash.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

Index: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "clang/Analysis/IssueHash.h"
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/PlistSupport.h"
@@ -20,7 +21,6 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/TokenConcatenation.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
-#include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
@@ -697,7 +697,7 @@
 : D->getLocation().asLocation()),
 SM);
 const Decl *DeclWithIssue = D->getDeclWithIssue();
-EmitString(o, GetIssueHash(SM, L, D->getCheckerName(), D->getBugType(),
+EmitString(o, getIssueHash(L, D->getCheckerName(), D->getBugType(),
DeclWithIssue, LangOpts))
 << '\n';
 
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "clang/Analysis/IssueHash.h"
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
@@ -23,7 +24,6 @@
 #include "clang/Lex/Token.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
@@ -583,8 +583,8 @@
 os  << "\n\n";
 
 os << "\n\n";
 
 os << "\n

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:52
   AllowedTypes(
   utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
 

Just put it here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89332

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

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp:307
   .Case(FULLNAME, HELPTEXT)
 #include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 #undef CHECKER

Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > This thing still worries me but this definitely isn't a link-time 
> > > > dependency.
> > > D53277#1285757, rGb8cfcc71469d40a98f4cc79fcdc46cd67bea45f7
> > Ok, what's the proper solution here? This is clearly a layering violation 
> > now; this generic diagnostic consumer shouldn't know anything about the 
> > Static Analyzer specifically. I guess we could separate it into an 
> > independent polymorphic object ("DescriptionGetter" or something like that) 
> > that the Static Analyzer would instantiate manually. Or ideally we could 
> > ship this information with the bug report itself.
> > 
> > I'll add a FIXME and try to reproduce potential modules problems locally.
> I am puzzled myself :/ Maybe we could ask @aaron.ballman, since he landed 
> most of these changes back in the day?
I fixed a couple other bugs that caused modules build to fail before pushing 
but this one wasn't a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67422

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

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp:307
   .Case(FULLNAME, HELPTEXT)
 #include "clang/StaticAnalyzer/Checkers/Checkers.inc"
 #undef CHECKER

Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > This thing still worries me but this definitely isn't a link-time 
> > > > dependency.
> > > D53277#1285757, rGb8cfcc71469d40a98f4cc79fcdc46cd67bea45f7
> > Ok, what's the proper solution here? This is clearly a layering violation 
> > now; this generic diagnostic consumer shouldn't know anything about the 
> > Static Analyzer specifically. I guess we could separate it into an 
> > independent polymorphic object ("DescriptionGetter" or something like that) 
> > that the Static Analyzer would instantiate manually. Or ideally we could 
> > ship this information with the bug report itself.
> > 
> > I'll add a FIXME and try to reproduce potential modules problems locally.
> I am puzzled myself :/ Maybe we could ask @aaron.ballman, since he landed 
> most of these changes back in the day?
The original code was added because it's needed for the required SARIF output. 
At the time, it didn't seem like it was a layering violation because the SARIF 
output was limited to just the clang static analyzer diagnostics -- however, I 
agree that work needs to be done here now. For instance, one of the issues with 
the SARIF output is that it only captures output from the static analyzer and 
doesn't report diagnostics from Clang itself, IIRC. We'll need to solve this 
for clang-tidy diagnostics as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67422

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

[PATCH] D89303: [SyntaxTree] Improve safety of `replaceChildRangeLowLevel`

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang/lib/Tooling/Syntax/Tree.cpp:103
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (auto *N = New; N; N = N->NextSibling) {
 assert(N->Parent == nullptr);

eduucaldas wrote:
> Throughout the function we use data members instead of accessors. Is one 
> preferrable to the other?
I don't think there is a difference.



Comment at: clang/lib/Tooling/Syntax/Tree.cpp:124
+return BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  };
+

Why lambda and not:

```
Node *&Begin = BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89303

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

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

[Felix Berger via Phabricator via cfe-commits]

flx added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:52
   AllowedTypes(
   utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
 

lebedev.ri wrote:
> Just put it here?
I tried this first, but this list  is substring matched against only on the 
non-qualified type name, so std::function would not match anything and if we 
added "function" here it would match many other types that contain the word 
function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89332

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

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:46
+
+void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
+  const auto ConstType = hasType(isConstQualified());

JonasToth wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > aaron.ballman wrote:
> > > > Should this check only fire in C++? I'm sort of on the fence. It's a 
> > > > C++ core guideline, so it stands to reason it should be disabled for C 
> > > > code. But const-correctness is a thing in C too. WDYT?
> > > I do not know all the subtle differences between C and C++ here. Judging 
> > > from this: 
> > > https://stackoverflow.com/questions/5248571/is-there-const-in-c the 
> > > current form would not be correct for C for pointers.
> > > The assumptions of this check and especially the transformations are done 
> > > for C++. I dont see a reason why the value-semantic would not apply for 
> > > both.
> > > 
> > > Maybe there is a way to make the code compatible for both languages. The 
> > > easiest solution would probably to not do the 'pointer-as-value' 
> > > transformation. This is not that relevant as a feature anyway. I expect 
> > > not nearly as much usage of this option as for the others.
> > > 
> > > In the end of the day i would like to support both C and C++. Right now 
> > > it is untested and especially the transformation might break the code. It 
> > > should run on both languages though, as there is no language checking.
> > > I will add some real world C code-bases for the transformation testing 
> > > and see what happens :)
> > > Judging from this: 
> > > https://stackoverflow.com/questions/5248571/is-there-const-in-c the 
> > > current form would not be correct for C for pointers.
> > 
> > Sure, there may be additional changes needed to support the other oddities 
> > of C. I was asking more at the high level.
> > 
> > > In the end of the day i would like to support both C and C++. Right now 
> > > it is untested and especially the transformation might break the code. 
> > 
> > Another option is for us to restrict the check in its current form to just 
> > C++ and then expose a C check from it in a follow-up (likely under a 
> > different module than cppcodeguidelines). For instance, CERT has a 
> > recommendation (not a rule) about this for C 
> > (https://wiki.sei.cmu.edu/confluence/display/c/DCL00-C.+Const-qualify+immutable+objects)
> >  and I would not be surprised to find it in other coding standards as well.
> Another const check is probably better. The clang-tidy part should be minimal 
> effort. I think there isn't alot of duplication either.
> We could still think of proper configurability of this check and alias it 
> into other modules with proper defaults for C.
Okay, I'm sold -- can you not register the check unless in C++ mode? We can add 
a C-specific check later.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:830
+
+void complex_usage() {
+  int np_local0 = 42;

Here's another terrible one to try -- can you make sure we don't try to make 
something const when the variable is evaluated in a context where it's usually 
not:
```
int N = 1; // Can't make N 'const' because VLAs make everything awful
sizeof(int[++N]);
```



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:923
+
+#if 0
+template 

Rather than comment out the code, I think it makes more sense to leave the code 
here, CHECK for the existing diagnostics, but add a FIXME comment to any 
diagnostics (or lack of diagnostics) that are incorrect. It makes it more clear 
what the current behavior is and what we want it to eventually be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D89078: Add `-f[no-]split-cold-code` toggling outlining & enable in -O

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd abandoned this revision.
compnerd added a comment.

Sure, I think I can do that instead.  Thanks for pointing me to that!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89078

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

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Reid Kleckner via cfe-commits]

FWIW, I don't see any issues with my two bots that use buildbot annotated
commands:
http://lab.llvm.org:8011/#/builders/sanitizer-windows
http://lab.llvm.org:8011/#/builders/clang-x64-windows-msvc
The individual steps don't highlight as green or red, but that's OK for now.

On Mon, Oct 12, 2020 at 7:19 PM Galina Kistanova 
wrote:

> We have a better version of AnnotatedCommand on the staging. It should be
> a functional equivalent of the old one.
> We need to stress test it well before moving to the production build bot.
>
> For that we need all sanitizer + other bots which use the AnnotatedCommand
> directly or indirectly moved temporarily to the staging.
>
> Please let me know when that could be arranged.
>
> Thanks
>
> Galina
>
> On Mon, Oct 12, 2020 at 11:39 AM Reid Kleckner  wrote:
>
>> On Wed, Oct 7, 2020 at 4:32 PM Galina Kistanova via lldb-commits <
>> lldb-comm...@lists.llvm.org> wrote:
>>
>>> They are online now -
>>> http://lab.llvm.org:8011/#/waterfall?tags=sanitizer
>>>
>>> AnnotatedCommand has severe design conflict with the new buildbot.
>>> We have changed it to be safe and still do something useful, but it will
>>> need more love and care.
>>>
>>> Please let me know if you have some spare time to work on porting
>>> AnnotatedCommand.
>>>
>>
>> That's unfortunate, it would've been good to know that earlier. I and
>> another team member have spent a fair amount of time porting things to use
>> more AnnotatedCommand steps, because it gives us the flexibility to test
>> steps locally and make changes to the steps without restarting the buildbot
>> master. IMO that is the Right Way to define steps: a script that you can
>> run locally on a machine that satisfies the OS and dep requirements of the
>> script.
>>
>> I am restarting the two bots that I am responsible for, and may need some
>> help debugging further issues soon. I'll let you know.
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89184: Support complex target features combinations

[Artem Belevich via Phabricator via cfe-commits]

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

In D89184#2328144 , @craig.topper 
wrote:

> This is needed for D89105  and was split 
> from it.

D89105   appears to use only `"avx512vl , 
avx512vnni | avxvnni"`.
Does it mean `(avx512vl , avx512vnni) | avxvnni` or `avx512vl , (avx512vnni | 
avxvnni)` ?

This change would be needed for the former, but not the latter. I assume that 
it's the former. If that's the case, LGTM.
Caveat: I'm not the owner of CodeGen, so you may want to give someone familiar 
with it a chance to chime in before landing the change.


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

https://reviews.llvm.org/D89184

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

Re: [Lldb-commits] Upcoming upgrade of LLVM buildbot

[Siva Chandra via cfe-commits]

On Mon, Oct 12, 2020 at 10:01 PM Galina Kistanova via lldb-commits
 wrote:
> If somebody else could move their AnnotatedCommand bots to the staging area, 
> that would be much appreciated.

I moved the libc bots to staging to now.

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