[PATCH] D110257: [CFE][Codegen] Make sure to maintain the contiguity of all the static allocas

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Please update the documentation with this new constraint. It would be helpful 
to know exactly when we now require alloca instructions to be adjacent to one 
another. If you wish to avoid other passes breaking the invariant in future, I 
think it needs to be added to the IR verifier, and should have been as part of 
this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257

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

[PATCH] D113107: Support of expression granularity for _Float16.

[John McCall via Phabricator via cfe-commits]

rjmccall added a reviewer: scanon.
rjmccall added a comment.

+ Steve Canon




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315
+  if ((SrcType->isHalfType() || iSFloat16Allowed) &&
+  !CGF.getContext().getLangOpts().NativeHalfType) {
 // Cast to FP using the intrinsic if the half type itself isn't supported.

pengfei wrote:
> rjmccall wrote:
> > pengfei wrote:
> > > rjmccall wrote:
> > > > Okay, this condition is pretty ridiculous to be repeating in three 
> > > > different places across the compiler.  Especially since you're going to 
> > > > change it when you implement the new option, right?
> > > > 
> > > > Can we state this condition more generally?  I'm not sure why this is 
> > > > so narrowly restricted, and the variable name isn't telling me 
> > > > anything, since `_Float16` must by definition be "allowed" if we have 
> > > > an expression of `_Float16` type.
> > > > since _Float16 must by definition be "allowed" if we have an expression 
> > > > of _Float16 type.
> > > 
> > > _Float16 is allowed only on a few targets. 
> > > https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
> > > By the way, we should update for X86 since it's not limited to avx512fp16 
> > > now.
> > > _Float16 is allowed only on a few targets.
> > 
> > Yes, I know that.  But if `SrcType->isFloat16Type()` is true, we must be on 
> > one of those targets, because the type doesn't otherwise exist.
> I see your point now. The problem here is we want to allow the `_Float16` to 
> be used more broadly. But the target doesn't really support it sometime. 
> Currently full arithmatic operations are supported only on target with 
> AVX512FP16.
> We should cast for those targets without AVX512FP16 while avoid to do on 
> AVX512FP16.
I agree that many targets don't natively support arithmetic on this format, but 
x86 is not the first one that does.  Unless I'm misunderstanding, we already 
track this property in Clang's TargetInfo as `hasLegalHalfType()`.  
`+avx512fp16` presumably ought to set this.

I'm not sure what the interaction with the `NativeHalfType` LangOpt is supposed 
to be here.  My understanding is that that option is just supposed to affect 
`__fp16`, basically turning it into a proper arithmetic type, i.e. essentially 
`_Float16`.  Whatever effect you want to apply to `_Float16` should presumably 
happen even if that option not set.

More broadly, I don't think your approach in this patch is correct.  The type 
of operations on `_Float16` should not change based on whether the target 
natively supports `_Float16`.  If we need to emulate those operations on 
targets that don't provide them natively, we should do that at a lower level 
than the type system.

The most appropriate place to do that is going to depend on the exact semantics 
we want.

If we want to preserve `half` semantics exactly regardless of target, we should 
have Clang's IR generation actually emit `half` operations.  Targets that don't 
support those operations natively will have to lower at least some of those 
operations into compiler-rt calls, but that's not at all unprecedented.

If we're okay with playing loose for performance reasons, we can promote to 
`float` immediately around individual arithmetic operations.  IR generation is 
probably the most appropriate place to do that.  But I'm quite concerned about 
that making `_Float16` feel like an unpredictable/unportable type; it seems to 
me that software emulation is much better.

If you're proposing the latter, I think you need to raise that more widely than 
a code review; please make a post on llvm-dev.


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

https://reviews.llvm.org/D113107

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

[PATCH] D110257: [CFE][Codegen] Make sure to maintain the contiguity of all the static allocas

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Please change the commit message to say why this change is necessary / an 
improvement on what we have now.

My recollection is that the amdgpu backend crashes on some IR and this 
decreases the probability of that IR pattern occuring, which still sounds like 
fixing the wrong place to me. Was this the one where hoisting static size 
alloca into the entry block near the backend would the problem?

I think this patch is missing a documentation update adding the new constraint 
that allocas must be contiguous in IR. That would help to answer questions 
about which alloca must be contiguous and which can occur separated by 
instructions, as currently none of them need to be adjacent. Also, is this only 
intended to constrain the entry basic block?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257

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

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D112730#3133822 , @carlosgalvezp 
wrote:

> @tstellar @tonic I have finally received an answer from AUTOSAR. Let me know 
> if you'd like me to CC you the mail chain to see the exact discussion.

Can you cc bo...@llvm.org on the mail chain.


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

https://reviews.llvm.org/D112730

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

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

@tstellar @tonic I have finally received an answer from AUTOSAR. Let me know if 
you'd like me to CC you the mail chain to see the exact discussion.

> The AUTOSAR confidential statement states that these documents are considered 
> under the license of AUTOSAR, thus a commercial usage without being a partner 
> is not allowed.



> As you stated correctly, the C++14 guidelines have been handed over to MISRA 
> and will be released there in spring 2022 according to my knowledge.



> Since we had other similar requests already, I can state that this usage of 
> the guidelines is uncritical from AUTOSAR’s perspective.




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

https://reviews.llvm.org/D112730

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

[clang-tools-extra] 9699c0f - [clang-tidy][NFC] Simplify ClangTidyStats

[Carlos Galvez via cfe-commits]

Author: Carlos Galvez
Date: 2021-11-16T07:36:05Z
New Revision: 9699c0fea3552f687f8357032199890633d58fe2

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

LOG: [clang-tidy][NFC] Simplify ClangTidyStats

- Use NSDMI and remove constructor.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index f93deae64bb3..234455c5bea2 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -45,18 +45,13 @@ struct ClangTidyError : tooling::Diagnostic {
   std::vector EnabledDiagnosticAliases;
 };
 
-/// Contains displayed and ignored diagnostic counters for a ClangTidy
-/// run.
+/// Contains displayed and ignored diagnostic counters for a ClangTidy run.
 struct ClangTidyStats {
-  ClangTidyStats()
-  : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), 
ErrorsIgnoredNOLINT(0),
-ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
-
-  unsigned ErrorsDisplayed;
-  unsigned ErrorsIgnoredCheckFilter;
-  unsigned ErrorsIgnoredNOLINT;
-  unsigned ErrorsIgnoredNonUserCode;
-  unsigned ErrorsIgnoredLineFilter;
+  unsigned ErrorsDisplayed = 0;
+  unsigned ErrorsIgnoredCheckFilter = 0;
+  unsigned ErrorsIgnoredNOLINT = 0;
+  unsigned ErrorsIgnoredNonUserCode = 0;
+  unsigned ErrorsIgnoredLineFilter = 0;
 
   unsigned errorsIgnored() const {
 return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +



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

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

[Carlos Galvez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9699c0fea355: [clang-tidy][NFC] Simplify ClangTidyStats 
(authored by carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113847

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -45,18 +45,13 @@
   std::vector EnabledDiagnosticAliases;
 };
 
-/// Contains displayed and ignored diagnostic counters for a ClangTidy
-/// run.
+/// Contains displayed and ignored diagnostic counters for a ClangTidy run.
 struct ClangTidyStats {
-  ClangTidyStats()
-  : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), 
ErrorsIgnoredNOLINT(0),
-ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
-
-  unsigned ErrorsDisplayed;
-  unsigned ErrorsIgnoredCheckFilter;
-  unsigned ErrorsIgnoredNOLINT;
-  unsigned ErrorsIgnoredNonUserCode;
-  unsigned ErrorsIgnoredLineFilter;
+  unsigned ErrorsDisplayed = 0;
+  unsigned ErrorsIgnoredCheckFilter = 0;
+  unsigned ErrorsIgnoredNOLINT = 0;
+  unsigned ErrorsIgnoredNonUserCode = 0;
+  unsigned ErrorsIgnoredLineFilter = 0;
 
   unsigned errorsIgnored() const {
 return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -45,18 +45,13 @@
   std::vector EnabledDiagnosticAliases;
 };
 
-/// Contains displayed and ignored diagnostic counters for a ClangTidy
-/// run.
+/// Contains displayed and ignored diagnostic counters for a ClangTidy run.
 struct ClangTidyStats {
-  ClangTidyStats()
-  : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), ErrorsIgnoredNOLINT(0),
-ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
-
-  unsigned ErrorsDisplayed;
-  unsigned ErrorsIgnoredCheckFilter;
-  unsigned ErrorsIgnoredNOLINT;
-  unsigned ErrorsIgnoredNonUserCode;
-  unsigned ErrorsIgnoredLineFilter;
+  unsigned ErrorsDisplayed = 0;
+  unsigned ErrorsIgnoredCheckFilter = 0;
+  unsigned ErrorsIgnoredNOLINT = 0;
+  unsigned ErrorsIgnoredNonUserCode = 0;
+  unsigned ErrorsIgnoredLineFilter = 0;
 
   unsigned errorsIgnored() const {
 return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

[Henry Linjamäki via Phabricator via cfe-commits]

linjamaki added inline comments.



Comment at: clang/lib/Basic/Targets/SPIR.h:233
+if (Opts.HIP && Opts.CUDAIsDevice)
+  // Enable address space mapping from HIP to SPIR-V.
+  // See comment on the SPIRDefIsGenMap table.

Anastasia wrote:
> linjamaki wrote:
> > Anastasia wrote:
> > > My guess is that this is not only HIP specific but for example the same 
> > > applies to SYCL.
> > > 
> > > I am not sure if it makes more sense to move this into a 
> > > `BaseSPIRTargetInfo` since this is not really SPIR-V specific logic. It 
> > > is just a clang design misalignment between two address space concepts 
> > > that has to be addressed properly at some point.
> > > 
> > The DefaultIsGeneric AS mapping is enabled for SYCL in the 
> > BaseSPIRTargetInfo::adjust (which also means the mapping is available for 
> > both the SPIR and SPIR-V targets). On the other hand, the AS mapping for 
> > HIPSPV is enabled in SPIRVTargetInfo::adjust only as we intend to emit 
> > SPIR-V only. I’m under the impression that this is what was wanted.
> I think the issues here is not related to the target but to the flaw in the 
> address space design in clang. So right now all languages that don't derive 
> the address space semantic from embedded C (SYCL/CUDA/HIP) would need to 
> reset the address space map. See FIXME comment in the definition of `adjust`.
> 
> So the right thing to do would be to set the address space map correctly 
> straight away based on the language being compiled for which would avoid 
> overriding this in `adjust`. But if we do override it then it makes more 
> sense to at least unify the logic among targets.
> 
> 
> > 
> > On the other hand, the AS mapping for HIPSPV is enabled in 
> > SPIRVTargetInfo::adjust only as we intend to emit SPIR-V only. 
> > 
> 
> 
> I am not really sure how you would support one target only.
> Clang architecture (at least originally) assumes that all languages can map 
> to all targets which in practice is not true in some cases. This is why we 
> need to provide an address space map even for targets that have no memory 
> segmented language compiled to it. 

> So the right thing to do would be to set the address space map correctly 
> straight away based on the language being compiled for which would avoid 
> overriding this in `adjust`. But if we do override it then it makes more 
> sense to at least unify the logic among targets.
> 

Since we are not sure how we would solve this issue optimally, we adjusted the 
patch to avoid adding more overrides for the `adjust` method and the logic 
previously in the `SPIRVTargetInfo::adjust` is moved to 
`BaseSPIRTargetInfo::adjust` with the SYCL.  Would this be sufficient for the 
functionality added by this patch?

> I am not really sure how you would support one target only.
> Clang architecture (at least originally) assumes that all languages can map 
> to all targets which in practice is not true in some cases. This is why we 
> need to provide an address space map even for targets that have no memory 
> segmented language compiled to it. 

“HIPSPV” is not meant to be a new language. We are just adjusting the address 
space mapping from the HIP language (for device code) to SPIR-V that suits 
better than the default mapping where all the HIP address spaces would be 
mapped to target address space zero. We map the address spaces to the suitable 
ones in the OpenCL standard, which works both for HIPCL (which uses the 
OpenCL-based runtime and the OpenCL SPIR-V profile) and HIPLZ (which uses the 
LZ-based runtime and also the OpenCL SPIR-V profile).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108621

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

[PATCH] D108621: [HIPSPV] Add CUDA->SPIR-V address space mapping

[Henry Linjamäki via Phabricator via cfe-commits]

linjamaki updated this revision to Diff 387500.
linjamaki added a comment.

Rebase, add asserts and move address space map reset for HIP from 
SPIRVTargetInfo to BaseSPIRTargetInfo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108621

Files:
  clang/lib/Basic/Targets/SPIR.h
  clang/test/CodeGenHIP/hipspv-addr-spaces.cpp

Index: clang/test/CodeGenHIP/hipspv-addr-spaces.cpp
===
--- /dev/null
+++ clang/test/CodeGenHIP/hipspv-addr-spaces.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple spirv64 -x hip -emit-llvm -fcuda-is-device \
+// RUN:   -o - %s | FileCheck %s
+
+#define __device__ __attribute__((device))
+#define __shared__ __attribute__((shared))
+#define __constant__ __attribute__((constant))
+
+// CHECK: %struct.foo_t = type { i32, i32 addrspace(4)* }
+
+// CHECK: @d ={{.*}} addrspace(1) externally_initialized global
+__device__ int d;
+
+// CHECK: @c ={{.*}} addrspace(1) externally_initialized global
+__constant__ int c;
+
+// CHECK: @s ={{.*}} addrspace(3) global
+__shared__ int s;
+
+// CHECK: @foo ={{.*}} addrspace(1) externally_initialized global %struct.foo_t
+__device__ struct foo_t {
+  int i;
+  int* pi;
+} foo;
+
+// CHECK: define{{.*}} spir_func i32 addrspace(4)* @_Z3barPi(i32 addrspace(4)*
+__device__ int* bar(int *x) {
+  return x;
+}
+
+// CHECK: define{{.*}} spir_func i32 addrspace(4)* @_Z5baz_dv()
+__device__ int* baz_d() {
+  // CHECK: ret i32 addrspace(4)* addrspacecast (i32 addrspace(1)* @d to i32 addrspace(4)*
+  return 
+}
+
+// CHECK: define{{.*}} spir_func i32 addrspace(4)* @_Z5baz_cv()
+__device__ int* baz_c() {
+  // CHECK: ret i32 addrspace(4)* addrspacecast (i32 addrspace(1)* @c to i32 addrspace(4)*
+  return 
+}
+
+// CHECK: define{{.*}} spir_func i32 addrspace(4)* @_Z5baz_sv()
+__device__ int* baz_s() {
+  // CHECK: ret i32 addrspace(4)* addrspacecast (i32 addrspace(3)* @s to i32 addrspace(4)*
+  return 
+}
Index: clang/lib/Basic/Targets/SPIR.h
===
--- clang/lib/Basic/Targets/SPIR.h
+++ clang/lib/Basic/Targets/SPIR.h
@@ -56,9 +56,14 @@
 0, // opencl_generic
 0, // opencl_global_device
 0, // opencl_global_host
-0, // cuda_device
-0, // cuda_constant
-0, // cuda_shared
+// cuda_* address space mapping is intended for HIPSPV (HIP to SPIR-V
+// translation). This mapping is enabled when the language mode is HIP.
+1, // cuda_device
+// cuda_constant pointer can be casted to default/"flat" pointer, but in
+// SPIR-V casts between constant and generic pointers are not allowed. For
+// this reason cuda_constant is mapped to SPIR-V CrossWorkgroup.
+1, // cuda_constant
+3, // cuda_shared
 1, // sycl_global
 5, // sycl_global_device
 6, // sycl_global_host
@@ -74,6 +79,8 @@
 protected:
   BaseSPIRTargetInfo(const llvm::Triple , const TargetOptions &)
   : TargetInfo(Triple) {
+assert((Triple.isSPIR() || Triple.isSPIRV()) &&
+   "Invalid architecture for SPIR or SPIR-V.");
 assert(getTriple().getOS() == llvm::Triple::UnknownOS &&
"SPIR(-V) target must use unknown OS");
 assert(getTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
@@ -137,11 +144,16 @@
 // FIXME: SYCL specification considers unannotated pointers and references
 // to be pointing to the generic address space. See section 5.9.3 of
 // SYCL 2020 specification.
-// Currently, there is no way of representing SYCL's default address space
-// language semantic along with the semantics of embedded C's default
-// address space in the same address space map. Hence the map needs to be
-// reset to allow mapping to the desired value of 'Default' entry for SYCL.
-setAddressSpaceMap(/*DefaultIsGeneric=*/Opts.SYCLIsDevice);
+// Currently, there is no way of representing SYCL's and HIP's default
+// address space language semantic along with the semantics of embedded C's
+// default address space in the same address space map. Hence the map needs
+// to be reset to allow mapping to the desired value of 'Default' entry for
+// SYCL and HIP.
+setAddressSpaceMap(
+/*DefaultIsGeneric=*/Opts.SYCLIsDevice ||
+// The address mapping from HIP language for device code is only defined
+// for SPIR-V.
+(getTriple().isSPIRV() && Opts.HIP && Opts.CUDAIsDevice));
   }
 
   void setSupportedOpenCLOpts() override {
@@ -159,6 +171,7 @@
 public:
   SPIRTargetInfo(const llvm::Triple , const TargetOptions )
   : BaseSPIRTargetInfo(Triple, Opts) {
+assert(Triple.isSPIR() && "Invalid architecture for SPIR.");
 assert(getTriple().getOS() == llvm::Triple::UnknownOS &&
"SPIR target must use unknown OS");
 assert(getTriple().getEnvironment() == llvm::Triple::UnknownEnvironment &&
@@ -177,6 +190,8 @@
 public:
   

[PATCH] D112404: [SPIR-V] Add translator tool

[Henry Linjamäki via Phabricator via cfe-commits]

linjamaki updated this revision to Diff 387499.
linjamaki marked an inline comment as done.
linjamaki added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112404

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/ToolChains/SPIRV.cpp
  clang/lib/Driver/ToolChains/SPIRV.h

Index: clang/lib/Driver/ToolChains/SPIRV.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/SPIRV.h
@@ -0,0 +1,46 @@
+//===--- SPIRV.h - SPIR-V Tool Implementations --*- 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_LIB_DRIVER_TOOLCHAINS_SPIRV_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_SPIRV_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace tools {
+namespace SPIRV {
+
+void addTranslatorArgs(const llvm::opt::ArgList ,
+   llvm::opt::ArgStringList );
+
+void constructTranslateCommand(Compilation , const Tool ,
+   const JobAction , const InputInfo ,
+   const InputInfo ,
+   const llvm::opt::ArgStringList );
+
+class LLVM_LIBRARY_VISIBILITY Translator : public Tool {
+public:
+  Translator(const ToolChain )
+  : Tool("SPIR-V::Translator", "llvm-spirv", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool hasIntegratedAssembler() const override { return true; }
+
+  void ConstructJob(Compilation , const JobAction ,
+const InputInfo , const InputInfoList ,
+const llvm::opt::ArgList ,
+const char *LinkingOutput) const override;
+};
+
+} // namespace SPIRV
+} // namespace tools
+} // namespace driver
+} // namespace clang
+#endif
Index: clang/lib/Driver/ToolChains/SPIRV.cpp
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/SPIRV.cpp
@@ -0,0 +1,48 @@
+//===--- SPIRV.cpp - SPIR-V Tool Implementations *- 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
+//
+//===--===//
+#include "SPIRV.h"
+#include "CommonArgs.h"
+#include "clang/Driver/Compilation.h"
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/InputInfo.h"
+#include "clang/Driver/Options.h"
+
+using namespace clang::driver::tools;
+using namespace llvm::opt;
+
+void SPIRV::constructTranslateCommand(Compilation , const Tool ,
+  const JobAction ,
+  const InputInfo ,
+  const InputInfo ,
+  const llvm::opt::ArgStringList ) {
+  llvm::opt::ArgStringList CmdArgs(Args);
+  CmdArgs.push_back(Input.getFilename());
+
+  if (Input.getType() == types::TY_PP_Asm)
+CmdArgs.push_back("-to-binary");
+  if (Output.getType() == types::TY_PP_Asm)
+CmdArgs.push_back("-spirv-text");
+
+  CmdArgs.append({"-o", Output.getFilename()});
+
+  const char *Exec =
+  C.getArgs().MakeArgString(T.getToolChain().GetProgramPath("llvm-spirv"));
+  C.addCommand(std::make_unique(JA, T, ResponseFileSupport::None(),
+ Exec, CmdArgs, Input, Output));
+}
+
+void SPIRV::Translator::ConstructJob(Compilation , const JobAction ,
+ const InputInfo ,
+ const InputInfoList ,
+ const ArgList ,
+ const char *LinkingOutput) const {
+  claimNoWarnArgs(Args);
+  if (Inputs.size() != 1)
+llvm_unreachable("Invalid number of input files.");
+  constructTranslateCommand(C, *this, JA, Output, Inputs[0], {});
+}
Index: clang/lib/Driver/CMakeLists.txt
===
--- clang/lib/Driver/CMakeLists.txt
+++ clang/lib/Driver/CMakeLists.txt
@@ -69,6 +69,7 @@
   ToolChains/PS4CPU.cpp
   ToolChains/RISCVToolchain.cpp
   ToolChains/Solaris.cpp
+  ToolChains/SPIRV.cpp
   ToolChains/TCE.cpp
   ToolChains/VEToolchain.cpp
   ToolChains/WebAssembly.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113107: Support of expression granularity for _Float16.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315
+  if ((SrcType->isHalfType() || iSFloat16Allowed) &&
+  !CGF.getContext().getLangOpts().NativeHalfType) {
 // Cast to FP using the intrinsic if the half type itself isn't supported.

rjmccall wrote:
> pengfei wrote:
> > rjmccall wrote:
> > > Okay, this condition is pretty ridiculous to be repeating in three 
> > > different places across the compiler.  Especially since you're going to 
> > > change it when you implement the new option, right?
> > > 
> > > Can we state this condition more generally?  I'm not sure why this is so 
> > > narrowly restricted, and the variable name isn't telling me anything, 
> > > since `_Float16` must by definition be "allowed" if we have an expression 
> > > of `_Float16` type.
> > > since _Float16 must by definition be "allowed" if we have an expression 
> > > of _Float16 type.
> > 
> > _Float16 is allowed only on a few targets. 
> > https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
> > By the way, we should update for X86 since it's not limited to avx512fp16 
> > now.
> > _Float16 is allowed only on a few targets.
> 
> Yes, I know that.  But if `SrcType->isFloat16Type()` is true, we must be on 
> one of those targets, because the type doesn't otherwise exist.
I see your point now. The problem here is we want to allow the `_Float16` to be 
used more broadly. But the target doesn't really support it sometime. Currently 
full arithmatic operations are supported only on target with AVX512FP16.
We should cast for those targets without AVX512FP16 while avoid to do on 
AVX512FP16.


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

https://reviews.llvm.org/D113107

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

[PATCH] D113107: Support of expression granularity for _Float16.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315
+  if ((SrcType->isHalfType() || iSFloat16Allowed) &&
+  !CGF.getContext().getLangOpts().NativeHalfType) {
 // Cast to FP using the intrinsic if the half type itself isn't supported.

pengfei wrote:
> rjmccall wrote:
> > Okay, this condition is pretty ridiculous to be repeating in three 
> > different places across the compiler.  Especially since you're going to 
> > change it when you implement the new option, right?
> > 
> > Can we state this condition more generally?  I'm not sure why this is so 
> > narrowly restricted, and the variable name isn't telling me anything, since 
> > `_Float16` must by definition be "allowed" if we have an expression of 
> > `_Float16` type.
> > since _Float16 must by definition be "allowed" if we have an expression of 
> > _Float16 type.
> 
> _Float16 is allowed only on a few targets. 
> https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
> By the way, we should update for X86 since it's not limited to avx512fp16 now.
> _Float16 is allowed only on a few targets.

Yes, I know that.  But if `SrcType->isFloat16Type()` is true, we must be on one 
of those targets, because the type doesn't otherwise exist.


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

https://reviews.llvm.org/D113107

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

[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:1
 //===-- CalleeNamespaceCheck.cpp 
--===//
 //

You should add tests for this exception check. See 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/test/clang-tidy/checkers/llvmlibc-callee-namespace.cpp.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:13
 
+#include 
+

Eugene.Zelenko wrote:
> Should  be `` and without newline separation form rest of headers.
Looks like this is present only for the debug `printf`?



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39
+// intercepted.
+static const char *FUNCTIONS_TO_IGNORE_NAMESPACE[] = {
+"__errno_location", "malloc", "calloc", "realloc", "free"};

Eugene.Zelenko wrote:
> Why not `std::array` of appropriate LLVM container?
May be `static const std::uordered_set`? It would likely make 
the lookup below much neater.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:59
+llvm::StringRef(FUNCTIONS_TO_IGNORE_NAMESPACE[i]))) {
+  printf("String found %s\n", FuncDecl->getName().str().c_str());
+  return;

lntue wrote:
> Look like diag() is used to print messages in this module?
This looks like a debug `printf`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113946

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

[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:13
 
+#include 
+

Should  be `` and without newline separation form rest of headers.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39
+// intercepted.
+static const char *FUNCTIONS_TO_IGNORE_NAMESPACE[] = {
+"__errno_location", "malloc", "calloc", "realloc", "free"};

Why not `std::array` of appropriate LLVM container?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113946

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1313
+  !CGF.getContext().getTargetInfo().hasFeature("avx512fp16") &&
+  (TT.isX32() || TT.isX86());
+  if ((SrcType->isHalfType() || iSFloat16Allowed) &&

We don't need to check it. It is included by `TT.isX86()`.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315
+  if ((SrcType->isHalfType() || iSFloat16Allowed) &&
+  !CGF.getContext().getLangOpts().NativeHalfType) {
 // Cast to FP using the intrinsic if the half type itself isn't supported.

rjmccall wrote:
> Okay, this condition is pretty ridiculous to be repeating in three different 
> places across the compiler.  Especially since you're going to change it when 
> you implement the new option, right?
> 
> Can we state this condition more generally?  I'm not sure why this is so 
> narrowly restricted, and the variable name isn't telling me anything, since 
> `_Float16` must by definition be "allowed" if we have an expression of 
> `_Float16` type.
> since _Float16 must by definition be "allowed" if we have an expression of 
> _Float16 type.

_Float16 is allowed only on a few targets. 
https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
By the way, we should update for X86 since it's not limited to avx512fp16 now.



Comment at: clang/test/CodeGen/X86/Float16-aritmetic.c:8-9
+  // CHECK: alloca half
+  // CHECK: store half {{.*}}, half*
+  // CHECK: store half {{.*}}, half*
+  // CHECK: load half, half*

This isn't correct without the ABI code change. I did some work in D107082. I 
plan to refactor (if I have enough time)


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

https://reviews.llvm.org/D113107

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

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai updated this revision to Diff 387489.
HsiangKai added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

Files:
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h
  llvm/docs/CommandGuide/tblgen.rst

Index: llvm/docs/CommandGuide/tblgen.rst
===
--- llvm/docs/CommandGuide/tblgen.rst
+++ llvm/docs/CommandGuide/tblgen.rst
@@ -348,6 +348,10 @@
 
   Generate ``riscv_vector_builtin_cg.inc`` for Clang.
 
+.. option:: -gen-riscv-vector-builtin-sema
+
+  Generate ``riscv_vector_builtin_sema.inc`` for Clang.
+
 .. option:: -gen-attr-docs
 
   Generate attribute documentation.
Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -110,6 +110,7 @@
 void EmitRVVHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitRVVBuiltins(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitRVVBuiltinCG(llvm::RecordKeeper , llvm::raw_ostream );
+void EmitRVVBuiltinSema(llvm::RecordKeeper , llvm::raw_ostream );
 
 void EmitCdeHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitCdeBuiltinDef(llvm::RecordKeeper , llvm::raw_ostream );
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -88,6 +88,7 @@
   GenRISCVVectorHeader,
   GenRISCVVectorBuiltins,
   GenRISCVVectorBuiltinCG,
+  GenRISCVVectorBuiltinSema,
   GenAttrDocs,
   GenDiagDocs,
   GenOptDocs,
@@ -243,6 +244,8 @@
"Generate riscv_vector_builtins.inc for clang"),
 clEnumValN(GenRISCVVectorBuiltinCG, "gen-riscv-vector-builtin-codegen",
"Generate riscv_vector_builtin_cg.inc for clang"),
+clEnumValN(GenRISCVVectorBuiltinSema, "gen-riscv-vector-builtin-sema",
+   "Generate riscv_vector_builtin_sema.inc for clang"),
 clEnumValN(GenAttrDocs, "gen-attr-docs",
"Generate attribute documentation"),
 clEnumValN(GenDiagDocs, "gen-diag-docs",
@@ -458,6 +461,9 @@
   case GenRISCVVectorBuiltinCG:
 EmitRVVBuiltinCG(Records, OS);
 break;
+  case GenRISCVVectorBuiltinSema:
+EmitRVVBuiltinSema(Records, OS);
+break;
   case GenAttrDocs:
 EmitClangAttrDocs(Records, OS);
 break;
Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -15,6 +15,7 @@
 //===--===//
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
@@ -22,6 +23,8 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/StringMatcher.h"
+#include "llvm/TableGen/TableGenBackend.h"
 #include 
 
 using namespace llvm;
@@ -55,7 +58,6 @@
 Float,
 Invalid,
   };
-  BasicType BT;
   ScalarTypeKind ScalarType = Invalid;
   LMULType LMUL;
   bool IsPointer = false;
@@ -71,11 +73,26 @@
   std::string ClangBuiltinStr;
   std::string Str;
   std::string ShortStr;
+  std::string QualExpr;
 
 public:
   RVVType() : RVVType(BasicType(), 0, StringRef()) {}
   RVVType(BasicType BT, int Log2LMUL, StringRef prototype);
 
+  const std::string getMangledStr() const {
+if (!Valid)
+  return "";
+
+Twine ScalarTypeStr =
+Twine(Twine(ScalarType) + Twine(IsPointer) + Twine(IsImmediate) +
+  Twine(IsConstant) + Twine(ElementBitwidth));
+if (isScalar()) {
+  return ScalarTypeStr.str();
+} else {
+  return Twine(ScalarTypeStr + Twine(Scale.getValue()) + LMUL.str()).str();
+}
+  }
+
   // Return the string representation of a type, which is an encoded string for
   // passing to the BUILTIN() macro in Builtins.def.
   const std::string () const { return BuiltinStr; }
@@ -97,6 +114,8 @@
 return ShortStr;
   }
 
+  const std::string () { return QualExpr; }
+
   bool isValid() const { return Valid; }
   bool isScalar() const { return Scale.hasValue() && Scale.getValue() == 0; }
   bool isVector() const { return Scale.hasValue() && Scale.getValue() != 0; }
@@ -110,13 +129,15 @@
   bool isFloat(unsigned Width) const {
 return isFloat() && ElementBitwidth == Width;
   }
+  bool isPointer() const { return IsPointer; }
+  bool isConstant() const { return 

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai updated this revision to Diff 387488.
HsiangKai added a comment.

Check required extensions when adding the declarations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

Files:
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h
  llvm/docs/CommandGuide/tblgen.rst

Index: llvm/docs/CommandGuide/tblgen.rst
===
--- llvm/docs/CommandGuide/tblgen.rst
+++ llvm/docs/CommandGuide/tblgen.rst
@@ -348,6 +348,10 @@
 
   Generate ``riscv_vector_builtin_cg.inc`` for Clang.
 
+.. option:: -gen-riscv-vector-builtin-sema
+
+  Generate ``riscv_vector_builtin_sema.inc`` for Clang.
+
 .. option:: -gen-attr-docs
 
   Generate attribute documentation.
Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -110,6 +110,7 @@
 void EmitRVVHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitRVVBuiltins(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitRVVBuiltinCG(llvm::RecordKeeper , llvm::raw_ostream );
+void EmitRVVBuiltinSema(llvm::RecordKeeper , llvm::raw_ostream );
 
 void EmitCdeHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitCdeBuiltinDef(llvm::RecordKeeper , llvm::raw_ostream );
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -88,6 +88,7 @@
   GenRISCVVectorHeader,
   GenRISCVVectorBuiltins,
   GenRISCVVectorBuiltinCG,
+  GenRISCVVectorBuiltinSema,
   GenAttrDocs,
   GenDiagDocs,
   GenOptDocs,
@@ -243,6 +244,8 @@
"Generate riscv_vector_builtins.inc for clang"),
 clEnumValN(GenRISCVVectorBuiltinCG, "gen-riscv-vector-builtin-codegen",
"Generate riscv_vector_builtin_cg.inc for clang"),
+clEnumValN(GenRISCVVectorBuiltinSema, "gen-riscv-vector-builtin-sema",
+   "Generate riscv_vector_builtin_sema.inc for clang"),
 clEnumValN(GenAttrDocs, "gen-attr-docs",
"Generate attribute documentation"),
 clEnumValN(GenDiagDocs, "gen-diag-docs",
@@ -458,6 +461,9 @@
   case GenRISCVVectorBuiltinCG:
 EmitRVVBuiltinCG(Records, OS);
 break;
+  case GenRISCVVectorBuiltinSema:
+EmitRVVBuiltinSema(Records, OS);
+break;
   case GenAttrDocs:
 EmitClangAttrDocs(Records, OS);
 break;
Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -15,6 +15,7 @@
 //===--===//
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
@@ -22,6 +23,8 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/StringMatcher.h"
+#include "llvm/TableGen/TableGenBackend.h"
 #include 
 
 using namespace llvm;
@@ -55,7 +58,6 @@
 Float,
 Invalid,
   };
-  BasicType BT;
   ScalarTypeKind ScalarType = Invalid;
   LMULType LMUL;
   bool IsPointer = false;
@@ -71,11 +73,26 @@
   std::string ClangBuiltinStr;
   std::string Str;
   std::string ShortStr;
+  std::string QualExpr;
 
 public:
   RVVType() : RVVType(BasicType(), 0, StringRef()) {}
   RVVType(BasicType BT, int Log2LMUL, StringRef prototype);
 
+  const std::string getMangledStr() const {
+if (!Valid)
+  return "";
+
+Twine ScalarTypeStr =
+Twine(Twine(ScalarType) + Twine(IsPointer) + Twine(IsImmediate) +
+  Twine(IsConstant) + Twine(ElementBitwidth));
+if (isScalar()) {
+  return ScalarTypeStr.str();
+} else {
+  return Twine(ScalarTypeStr + Twine(Scale.getValue()) + LMUL.str()).str();
+}
+  }
+
   // Return the string representation of a type, which is an encoded string for
   // passing to the BUILTIN() macro in Builtins.def.
   const std::string () const { return BuiltinStr; }
@@ -97,6 +114,8 @@
 return ShortStr;
   }
 
+  const std::string () { return QualExpr; }
+
   bool isValid() const { return Valid; }
   bool isScalar() const { return Scale.hasValue() && Scale.getValue() == 0; }
   bool isVector() const { return Scale.hasValue() && Scale.getValue() != 0; }
@@ -110,13 +129,15 @@
   bool isFloat(unsigned Width) const {
 return isFloat() && ElementBitwidth == Width;
   }
+  bool isPointer() const { return 

[PATCH] D113830: [clang-tidy] Fix false positive in `readability-identifier-naming` check involving `override` attribute

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz accepted this revision.
salman-javed-nz added a comment.
This revision is now accepted and ready to land.

LGTM. Nothing more to suggest from my side. Can we allow a few days for the 
other reviewers to put in their 2c.

As for the Bugzilla ticket , I'd 
say you've done enough to satisfy the primary grievance expressed in the PR: 
warnings should be raised in the base class only.
This is in line with the check's state purpose in the documentation 
.

> The naming of virtual methods is reported where they occur in the base class, 
> but not where they are overridden, as it can’t be fixed locally there.

The comments below the PR talk about the need to propagate naming fixes to the 
derived classes and call sites. The last comment shows an example where fixes 
are not propagated at all, not even for the simple (non-template) case. But 
your tests for 
`COverriding` and `CTIOverriding` show that the replacements are indeed 
working. In any case, applying fixes outside the base class is going above and 
beyond the check's scope, and as you say, could lead to incorrect suggestions 
if the code is even slightly less trivial. More work could be done on this 
class, but doesn't have to be in this patch.


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

https://reviews.llvm.org/D113830

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

[PATCH] D90434: [CodeGen] Correct codegen for self-capturing __block var

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

@ille, are you still working on this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90434

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

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai updated this revision to Diff 387472.
HsiangKai added a comment.
Herald added subscribers: VincentWu, luke957, mgrang.

Restructure the data structure to reuse information between C intrinsics.
In this way, we can have a smaller binary size and speed up the lookup
for the C intrinsics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

Files:
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h
  llvm/docs/CommandGuide/tblgen.rst

Index: llvm/docs/CommandGuide/tblgen.rst
===
--- llvm/docs/CommandGuide/tblgen.rst
+++ llvm/docs/CommandGuide/tblgen.rst
@@ -348,6 +348,10 @@
 
   Generate ``riscv_vector_builtin_cg.inc`` for Clang.
 
+.. option:: -gen-riscv-vector-builtin-sema
+
+  Generate ``riscv_vector_builtin_sema.inc`` for Clang.
+
 .. option:: -gen-attr-docs
 
   Generate attribute documentation.
Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -110,6 +110,7 @@
 void EmitRVVHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitRVVBuiltins(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitRVVBuiltinCG(llvm::RecordKeeper , llvm::raw_ostream );
+void EmitRVVBuiltinSema(llvm::RecordKeeper , llvm::raw_ostream );
 
 void EmitCdeHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitCdeBuiltinDef(llvm::RecordKeeper , llvm::raw_ostream );
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -88,6 +88,7 @@
   GenRISCVVectorHeader,
   GenRISCVVectorBuiltins,
   GenRISCVVectorBuiltinCG,
+  GenRISCVVectorBuiltinSema,
   GenAttrDocs,
   GenDiagDocs,
   GenOptDocs,
@@ -243,6 +244,8 @@
"Generate riscv_vector_builtins.inc for clang"),
 clEnumValN(GenRISCVVectorBuiltinCG, "gen-riscv-vector-builtin-codegen",
"Generate riscv_vector_builtin_cg.inc for clang"),
+clEnumValN(GenRISCVVectorBuiltinSema, "gen-riscv-vector-builtin-sema",
+   "Generate riscv_vector_builtin_sema.inc for clang"),
 clEnumValN(GenAttrDocs, "gen-attr-docs",
"Generate attribute documentation"),
 clEnumValN(GenDiagDocs, "gen-diag-docs",
@@ -458,6 +461,9 @@
   case GenRISCVVectorBuiltinCG:
 EmitRVVBuiltinCG(Records, OS);
 break;
+  case GenRISCVVectorBuiltinSema:
+EmitRVVBuiltinSema(Records, OS);
+break;
   case GenAttrDocs:
 EmitClangAttrDocs(Records, OS);
 break;
Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -15,6 +15,7 @@
 //===--===//
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
@@ -22,6 +23,8 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/StringMatcher.h"
+#include "llvm/TableGen/TableGenBackend.h"
 #include 
 
 using namespace llvm;
@@ -55,7 +58,6 @@
 Float,
 Invalid,
   };
-  BasicType BT;
   ScalarTypeKind ScalarType = Invalid;
   LMULType LMUL;
   bool IsPointer = false;
@@ -71,11 +73,26 @@
   std::string ClangBuiltinStr;
   std::string Str;
   std::string ShortStr;
+  std::string QualExpr;
 
 public:
   RVVType() : RVVType(BasicType(), 0, StringRef()) {}
   RVVType(BasicType BT, int Log2LMUL, StringRef prototype);
 
+  const std::string getMangledStr() const {
+if (!Valid)
+  return "";
+
+Twine ScalarTypeStr =
+Twine(Twine(ScalarType) + Twine(IsPointer) + Twine(IsImmediate) +
+  Twine(IsConstant) + Twine(ElementBitwidth));
+if (isScalar()) {
+  return ScalarTypeStr.str();
+} else {
+  return Twine(ScalarTypeStr + Twine(Scale.getValue()) + LMUL.str()).str();
+}
+  }
+
   // Return the string representation of a type, which is an encoded string for
   // passing to the BUILTIN() macro in Builtins.def.
   const std::string () const { return BuiltinStr; }
@@ -97,6 +114,8 @@
 return ShortStr;
   }
 
+  const std::string () { return QualExpr; }
+
   bool isValid() const { return Valid; }
   bool isScalar() const { return Scale.hasValue() && Scale.getValue() == 0; }
   bool isVector() const { return Scale.hasValue() && 

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

If we do need to support constant expressions of this, I think we should have 
the constant evaluator produce an expression rather than an l-value, the way we 
do with some other builtin calls.  That should stop the comparison problem more 
generally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Do any of the proposed use cases actually require this to be a constant 
expression?  Some of these patterns can be cheaply implemented with code 
generation even if the target doesn't otherwise support constants; for example, 
we could just mask the THUMB bit off on 32-bit ARM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D113954: [clang] NFC: rename internal `IsPossiblyOpaquelyQualifiedType` overload

[Matheus Izvekov 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 rG21ed00bc1bfd: [clang] NFC: rename internal 
`IsPossiblyOpaquelyQualifiedType` overload (authored by mizvekov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113954

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp


Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -603,7 +603,7 @@
  /*NumberOfArgumentsMustMatch=*/true);
 }
 
-static bool IsPossiblyOpaquelyQualifiedType(const Type *T) {
+static bool IsPossiblyOpaquelyQualifiedTypeInternal(const Type *T) {
   assert(T->isCanonicalUnqualified());
 
   switch (T->getTypeClass()) {
@@ -619,7 +619,7 @@
   case Type::IncompleteArray:
   case Type::VariableArray:
   case Type::DependentSizedArray:
-return IsPossiblyOpaquelyQualifiedType(
+return IsPossiblyOpaquelyQualifiedTypeInternal(
 cast(T)->getElementType().getTypePtr());
 
   default:
@@ -630,7 +630,7 @@
 /// Determines whether the given type is an opaque type that
 /// might be more qualified when instantiated.
 static bool IsPossiblyOpaquelyQualifiedType(QualType T) {
-  return IsPossiblyOpaquelyQualifiedType(
+  return IsPossiblyOpaquelyQualifiedTypeInternal(
   T->getCanonicalTypeInternal().getTypePtr());
 }
 


Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -603,7 +603,7 @@
  /*NumberOfArgumentsMustMatch=*/true);
 }
 
-static bool IsPossiblyOpaquelyQualifiedType(const Type *T) {
+static bool IsPossiblyOpaquelyQualifiedTypeInternal(const Type *T) {
   assert(T->isCanonicalUnqualified());
 
   switch (T->getTypeClass()) {
@@ -619,7 +619,7 @@
   case Type::IncompleteArray:
   case Type::VariableArray:
   case Type::DependentSizedArray:
-return IsPossiblyOpaquelyQualifiedType(
+return IsPossiblyOpaquelyQualifiedTypeInternal(
 cast(T)->getElementType().getTypePtr());
 
   default:
@@ -630,7 +630,7 @@
 /// Determines whether the given type is an opaque type that
 /// might be more qualified when instantiated.
 static bool IsPossiblyOpaquelyQualifiedType(QualType T) {
-  return IsPossiblyOpaquelyQualifiedType(
+  return IsPossiblyOpaquelyQualifiedTypeInternal(
   T->getCanonicalTypeInternal().getTypePtr());
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 21ed00b - [clang] NFC: rename internal `IsPossiblyOpaquelyQualifiedType` overload

[Matheus Izvekov via cfe-commits]

Author: Matheus Izvekov
Date: 2021-11-16T03:09:50+01:00
New Revision: 21ed00bc1bfd2b0b81d288f7f096a31079c24c4a

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

LOG: [clang] NFC: rename internal `IsPossiblyOpaquelyQualifiedType` overload

Rename `IsPossiblyOpaquelyQualifiedType` overload taking a Type*
as `IsPossiblyOpaquelyQualifiedTypeInternal` instead.

Signed-off-by: Matheus Izvekov 

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

Added: 


Modified: 
clang/lib/Sema/SemaTemplateDeduction.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index a1722c45b632..3c67b5b5072e 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -603,7 +603,7 @@ DeduceTemplateSpecArguments(Sema , TemplateParameterList 
*TemplateParams,
  /*NumberOfArgumentsMustMatch=*/true);
 }
 
-static bool IsPossiblyOpaquelyQualifiedType(const Type *T) {
+static bool IsPossiblyOpaquelyQualifiedTypeInternal(const Type *T) {
   assert(T->isCanonicalUnqualified());
 
   switch (T->getTypeClass()) {
@@ -619,7 +619,7 @@ static bool IsPossiblyOpaquelyQualifiedType(const Type *T) {
   case Type::IncompleteArray:
   case Type::VariableArray:
   case Type::DependentSizedArray:
-return IsPossiblyOpaquelyQualifiedType(
+return IsPossiblyOpaquelyQualifiedTypeInternal(
 cast(T)->getElementType().getTypePtr());
 
   default:
@@ -630,7 +630,7 @@ static bool IsPossiblyOpaquelyQualifiedType(const Type *T) {
 /// Determines whether the given type is an opaque type that
 /// might be more qualified when instantiated.
 static bool IsPossiblyOpaquelyQualifiedType(QualType T) {
-  return IsPossiblyOpaquelyQualifiedType(
+  return IsPossiblyOpaquelyQualifiedTypeInternal(
   T->getCanonicalTypeInternal().getTypePtr());
 }
 



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

[PATCH] D113107: Support of expression granularity for _Float16.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Can we split the patch which changes what types are enabled for various x86 
sub-targets out from the patch that changes the semantics of operations on 
`_Float16`?  Or is there a good reason it's disabled currently, namely because 
the semantics are wrong?




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315
+  if ((SrcType->isHalfType() || iSFloat16Allowed) &&
+  !CGF.getContext().getLangOpts().NativeHalfType) {
 // Cast to FP using the intrinsic if the half type itself isn't supported.

Okay, this condition is pretty ridiculous to be repeating in three different 
places across the compiler.  Especially since you're going to change it when 
you implement the new option, right?

Can we state this condition more generally?  I'm not sure why this is so 
narrowly restricted, and the variable name isn't telling me anything, since 
`_Float16` must by definition be "allowed" if we have an expression of 
`_Float16` type.


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

https://reviews.llvm.org/D113107

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

[PATCH] D113707: [clang] Make -masm=intel affect inline asm style

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(Here are a few revisions I read through while researching this patch:

da081a8e1f295ffdd670f87dba19f470a9a5acaa
and fc416faaa07c5
(and 4f2791617e2fa)
and aa23fa9f435d2

1778831a3d1d2 split the "ms" inline asm out

isSimple added in 19fe116fc0b35, meaningfully used in b737b6247f051)


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

https://reviews.llvm.org/D113707

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

[PATCH] D113954: [clang] NFC: rename internal `IsPossiblyOpaquelyQualifiedType` overload

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 387449.
mizvekov added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113954

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp


Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -603,7 +603,7 @@
  /*NumberOfArgumentsMustMatch=*/true);
 }
 
-static bool IsPossiblyOpaquelyQualifiedType(const Type *T) {
+static bool IsPossiblyOpaquelyQualifiedTypeInternal(const Type *T) {
   assert(T->isCanonicalUnqualified());
 
   switch (T->getTypeClass()) {
@@ -619,7 +619,7 @@
   case Type::IncompleteArray:
   case Type::VariableArray:
   case Type::DependentSizedArray:
-return IsPossiblyOpaquelyQualifiedType(
+return IsPossiblyOpaquelyQualifiedTypeInternal(
 cast(T)->getElementType().getTypePtr());
 
   default:
@@ -630,7 +630,7 @@
 /// Determines whether the given type is an opaque type that
 /// might be more qualified when instantiated.
 static bool IsPossiblyOpaquelyQualifiedType(QualType T) {
-  return IsPossiblyOpaquelyQualifiedType(
+  return IsPossiblyOpaquelyQualifiedTypeInternal(
   T->getCanonicalTypeInternal().getTypePtr());
 }
 


Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -603,7 +603,7 @@
  /*NumberOfArgumentsMustMatch=*/true);
 }
 
-static bool IsPossiblyOpaquelyQualifiedType(const Type *T) {
+static bool IsPossiblyOpaquelyQualifiedTypeInternal(const Type *T) {
   assert(T->isCanonicalUnqualified());
 
   switch (T->getTypeClass()) {
@@ -619,7 +619,7 @@
   case Type::IncompleteArray:
   case Type::VariableArray:
   case Type::DependentSizedArray:
-return IsPossiblyOpaquelyQualifiedType(
+return IsPossiblyOpaquelyQualifiedTypeInternal(
 cast(T)->getElementType().getTypePtr());
 
   default:
@@ -630,7 +630,7 @@
 /// Determines whether the given type is an opaque type that
 /// might be more qualified when instantiated.
 static bool IsPossiblyOpaquelyQualifiedType(QualType T) {
-  return IsPossiblyOpaquelyQualifiedType(
+  return IsPossiblyOpaquelyQualifiedTypeInternal(
   T->getCanonicalTypeInternal().getTypePtr());
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113954: [clang] NFC: rename internal `IsPossiblyOpaquelyQualifiedType` overload

[Matheus Izvekov via Phabricator via cfe-commits]

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

Rename `IsPossiblyOpaquelyQualifiedType` overload taking a Type*
as `IsPossiblyOpaquelyQualifiedTypeInternal` instead.

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113954

Files:
  clang/lib/Sema/SemaTemplateDeduction.cpp


Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -603,7 +603,7 @@
  /*NumberOfArgumentsMustMatch=*/true);
 }
 
-static bool IsPossiblyOpaquelyQualifiedType(const Type *T) {
+static bool IsPossiblyOpaquelyQualifiedTypeInternal(const Type *T) {
   assert(T->isCanonicalUnqualified());
 
   switch (T->getTypeClass()) {
@@ -630,7 +630,7 @@
 /// Determines whether the given type is an opaque type that
 /// might be more qualified when instantiated.
 static bool IsPossiblyOpaquelyQualifiedType(QualType T) {
-  return IsPossiblyOpaquelyQualifiedType(
+  return IsPossiblyOpaquelyQualifiedTypeInternal(
   T->getCanonicalTypeInternal().getTypePtr());
 }
 


Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -603,7 +603,7 @@
  /*NumberOfArgumentsMustMatch=*/true);
 }
 
-static bool IsPossiblyOpaquelyQualifiedType(const Type *T) {
+static bool IsPossiblyOpaquelyQualifiedTypeInternal(const Type *T) {
   assert(T->isCanonicalUnqualified());
 
   switch (T->getTypeClass()) {
@@ -630,7 +630,7 @@
 /// Determines whether the given type is an opaque type that
 /// might be more qualified when instantiated.
 static bool IsPossiblyOpaquelyQualifiedType(QualType T) {
-  return IsPossiblyOpaquelyQualifiedType(
+  return IsPossiblyOpaquelyQualifiedTypeInternal(
   T->getCanonicalTypeInternal().getTypePtr());
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals

[Tue Ly via Phabricator via cfe-commits]

lntue added inline comments.



Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:59
+llvm::StringRef(FUNCTIONS_TO_IGNORE_NAMESPACE[i]))) {
+  printf("String found %s\n", FuncDecl->getName().str().c_str());
+  return;

Look like diag() is used to print messages in this module?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113946

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

[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals

[Michael Jones via Phabricator via cfe-commits]

michaelrj created this revision.
michaelrj added reviewers: sivachandra, lntue.
Herald added subscribers: libc-commits, carlosgalvezp, ecnelises, tschuett, 
xazax.hun.
Herald added a project: libc-project.
michaelrj requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Up until now, all references to `errno` were marked with `NOLINT`, since
it was technically calling an external function. This fixes the lint
rules so that `errno`, as well as `malloc`, `calloc`, `realloc`, and
`free` are all allowed to be called as external functions. All of the
relevant `NOLINT` comments have been removed, and the documentation has
been updated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113946

Files:
  clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp
  libc/docs/clang_tidy_checks.rst
  libc/src/__support/FPUtil/NearestIntegerOperations.h
  libc/src/__support/str_to_float.h
  libc/src/__support/str_to_integer.h
  libc/src/math/generic/math_utils.h
  libc/src/string/strdup.cpp
  libc/src/string/strndup.cpp

Index: libc/src/string/strndup.cpp
===
--- libc/src/string/strndup.cpp
+++ libc/src/string/strndup.cpp
@@ -23,7 +23,7 @@
   size_t len = internal::string_length(src);
   if (len > size)
 len = size;
-  char *dest = reinterpret_cast(::malloc(len + 1)); // NOLINT
+  char *dest = reinterpret_cast(::malloc(len + 1));
   if (dest == nullptr)
 return nullptr;
   char *result =
Index: libc/src/string/strdup.cpp
===
--- libc/src/string/strdup.cpp
+++ libc/src/string/strdup.cpp
@@ -21,7 +21,7 @@
 return nullptr;
   }
   size_t len = internal::string_length(src) + 1;
-  char *dest = reinterpret_cast(::malloc(len)); // NOLINT
+  char *dest = reinterpret_cast(::malloc(len));
   if (dest == nullptr) {
 return nullptr;
   }
Index: libc/src/math/generic/math_utils.h
===
--- libc/src/math/generic/math_utils.h
+++ libc/src/math/generic/math_utils.h
@@ -55,7 +55,7 @@
 
 template  static inline T with_errno(T x, int err) {
   if (math_errhandling & MATH_ERRNO)
-errno = err; // NOLINT
+errno = err;
   return x;
 }
 
Index: libc/src/__support/str_to_integer.h
===
--- libc/src/__support/str_to_integer.h
+++ libc/src/__support/str_to_integer.h
@@ -74,7 +74,7 @@
   const char *original_src = src;
 
   if (base < 0 || base == 1 || base > 36) {
-errno = EINVAL; // NOLINT
+errno = EINVAL;
 return 0;
   }
 
@@ -114,19 +114,19 @@
 // the result cannot change, but we still need to advance src to the end of
 // the number.
 if (result == ABS_MAX) {
-  errno = ERANGE; // NOLINT
+  errno = ERANGE;
   continue;
 }
 
 if (result > ABS_MAX_DIV_BY_BASE) {
   result = ABS_MAX;
-  errno = ERANGE; // NOLINT
+  errno = ERANGE;
 } else {
   result = result * base;
 }
 if (result > ABS_MAX - cur_digit) {
   result = ABS_MAX;
-  errno = ERANGE; // NOLINT
+  errno = ERANGE;
 } else {
   result = result + cur_digit;
 }
Index: libc/src/__support/str_to_float.h
===
--- libc/src/__support/str_to_float.h
+++ libc/src/__support/str_to_float.h
@@ -220,7 +220,7 @@
   static_cast(fputil::FloatProperties::exponentBias)) {
 *outputMantissa = 0;
 *outputExp2 = fputil::FPBits::maxExponent;
-errno = ERANGE; // NOLINT
+errno = ERANGE;
 return;
   }
   // If the exponent is too small even for a subnormal, return 0.
@@ -230,7 +230,7 @@
fputil::FloatProperties::mantissaWidth)) {
 *outputMantissa = 0;
 *outputExp2 = 0;
-errno = ERANGE; // NOLINT
+errno = ERANGE;
 return;
   }
 
@@ -273,7 +273,7 @@
   if (exp2 >= fputil::FPBits::maxExponent) {
 *outputMantissa = 0;
 *outputExp2 = fputil::FPBits::maxExponent;
-errno = ERANGE; // NOLINT
+errno = ERANGE;
 return;
   }
 
@@ -309,7 +309,7 @@
   }
 
   if (exp2 == 0) {
-errno = ERANGE; // NOLINT
+errno = ERANGE;
   }
 
   *outputMantissa = finalMantissa;
@@ -411,7 +411,7 @@
   static_cast(fputil::FloatProperties::exponentBias) / 3) {
 *outputMantissa = 0;
 *outputExp2 = fputil::FPBits::maxExponent;
-errno = ERANGE; // NOLINT
+errno = ERANGE;
 return;
   }
   // If the exponent is too small even for a subnormal, return 0.
@@ -422,7 +422,7 @@
   2) {
 *outputMantissa = 0;
 *outputExp2 = 0;
-errno = ERANGE; // NOLINT
+errno = ERANGE;
 return;
   }
 
@@ -508,7 +508,7 @@
 // If we cut off any bits to fit this number into a subnormal, then it's
 // out of range for this size of float.
 if ((mantissa & ((1 << amountToShift) - 1)) > 0) {
-   

[PATCH] D113530: [wip] [analyzer] support use-after-free checking with parameter annotation

[Chris D'Angelo via Phabricator via cfe-commits]

chrisdangelo added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2383
+def OwnershipParam : InheritableParamAttr {
+  let Spellings = [Clang<"ownership_takes_param">];
+  let Subjects = SubjectList<[ParmVar]>;

I've discussed this a bit with Artem in person today. Artem would like to see 
if the existing "ownership_takes" attribute can be adjusted to include the 
ability to be used against a parameter. I'm looking into this now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113530

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

[PATCH] D113575: Add `isInitCapture` and `forEachLambdaCapture` matchers.

[Yitzhak Mandelbaum 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 rG9809c6c61ceb: Add `isInitCapture` and `forEachLambdaCapture` 
matchers. (authored by jcking1034, committed by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113575

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -4769,6 +4769,66 @@
 cxxConstructorDecl(forEachConstructorInitializer(cxxCtorInitializer();
 }
 
+TEST(ForEachLambdaCapture, MatchesCaptures) {
+  EXPECT_TRUE(matches(
+  "int main() { int x, y; auto f = [x, y]() { return x + y; }; }",
+  lambdaExpr(forEachLambdaCapture(lambdaCapture())), langCxx11OrLater()));
+  auto matcher = lambdaExpr(forEachLambdaCapture(
+  lambdaCapture(capturesVar(varDecl(hasType(isInteger().bind("LC")));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "int main() { int x, y; float z; auto f = [=]() { return x + y + z; }; }",
+  matcher, std::make_unique>("LC", 2)));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "int main() { int x, y; float z; auto f = [x, y, z]() { return x + y + "
+  "z; }; }",
+  matcher, std::make_unique>("LC", 2)));
+}
+
+TEST(ForEachLambdaCapture, IgnoreUnlessSpelledInSource) {
+  auto matcher =
+  traverse(TK_IgnoreUnlessSpelledInSource,
+   lambdaExpr(forEachLambdaCapture(
+   lambdaCapture(capturesVar(varDecl(hasType(isInteger()
+   .bind("LC";
+  EXPECT_TRUE(
+  notMatches("int main() { int x, y; auto f = [=]() { return x + y; }; }",
+ matcher, langCxx11OrLater()));
+  EXPECT_TRUE(
+  notMatches("int main() { int x, y; auto f = [&]() { return x + y; }; }",
+ matcher, langCxx11OrLater()));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  R"cc(
+  int main() {
+int x, y;
+float z;
+auto f = [=, ]() { return x + y + z; };
+  }
+  )cc",
+  matcher, std::make_unique>("LC", 1)));
+}
+
+TEST(ForEachLambdaCapture, MatchImplicitCapturesOnly) {
+  auto matcher =
+  lambdaExpr(forEachLambdaCapture(lambdaCapture(isImplicit()).bind("LC")));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "int main() { int x, y, z; auto f = [=, ]() { return x + y + z; }; }",
+  matcher, std::make_unique>("LC", 2)));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "int main() { int x, y, z; auto f = [&, z]() { return x + y + z; }; }",
+  matcher, std::make_unique>("LC", 2)));
+}
+
+TEST(ForEachLambdaCapture, MatchExplicitCapturesOnly) {
+  auto matcher = lambdaExpr(
+  forEachLambdaCapture(lambdaCapture(unless(isImplicit())).bind("LC")));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "int main() { int x, y, z; auto f = [=, ]() { return x + y + z; }; }",
+  matcher, std::make_unique>("LC", 1)));
+  EXPECT_TRUE(matchAndVerifyResultTrue(
+  "int main() { int x, y, z; auto f = [&, z]() { return x + y + z; }; }",
+  matcher, std::make_unique>("LC", 1)));
+}
+
 TEST(HasConditionVariableStatement, DoesNotMatchCondition) {
   EXPECT_TRUE(notMatches(
 "void x() { if(true) {} }",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1439,6 +1439,22 @@
   EXPECT_TRUE(notMatches("int X;", M));
 }
 
+TEST_P(ASTMatchersTest, IsInitCapture) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto M = varDecl(hasName("vd"), isInitCapture());
+  EXPECT_TRUE(notMatches(
+  "int main() { int vd = 3; auto f = [vd]() { return vd; }; }", M));
+
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { auto f = [vd=3]() { return vd; }; }", M));
+  EXPECT_TRUE(matches(
+  "int main() { int x = 3; auto f = [vd=x]() { return vd; }; }", M));
+}
+
 TEST_P(ASTMatchersTest, StorageDuration) {
   StringRef T =
   "void f() { int x; static int y; } int a;static int b;extern int c;";
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -246,6 +246,7 @@
   REGISTER_MATCHER(forEachArgumentWithParamType);
   REGISTER_MATCHER(forEachConstructorInitializer);
   

[clang] 9809c6c - Add `isInitCapture` and `forEachLambdaCapture` matchers.

[Yitzhak Mandelbaum via cfe-commits]

Author: James King
Date: 2021-11-15T22:55:28Z
New Revision: 9809c6c61cebbfcd100a3afd30fc9009f68d4678

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

LOG: Add `isInitCapture` and `forEachLambdaCapture` matchers.

This contributes follow-up work from https://reviews.llvm.org/D112491, which
allows for increased control over the matching of lambda captures. This also
updates the documentation for the `lambdaCapture` matcher.

Reviewed By: ymandel, aaron.ballman

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

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/lib/ASTMatchers/Dynamic/Registry.cpp
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index 9d2398e10..8f310d42be1c9 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -1191,7 +1191,17 @@ Node Matchers
 
 
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1LambdaCapture.html;>LambdaCapturelambdaCaptureMatcherhttps://clang.llvm.org/doxygen/classclang_1_1LambdaCapture.html;>LambdaCapture...
-
+Matches lambda 
captures.
+
+Given
+  int main() {
+int x;
+auto f = [x](){};
+auto g = [x = 1](){};
+  }
+In the matcher `lambdaExpr(hasAnyCapture(lambdaCapture()))`,
+`lambdaCapture()` matches `x` and `x=1`.
+
 
 
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1NestedNameSpecifierLoc.html;>NestedNameSpecifierLocnestedNameSpecifierLocMatcherhttps://clang.llvm.org/doxygen/classclang_1_1NestedNameSpecifierLoc.html;>NestedNameSpecifierLoc...
@@ -4540,6 +4550,21 @@ Narrowing Matchers
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1LambdaExpr.html;>LambdaExprforEachLambdaCaptureLambdaCaptureMatcher
 InnerMatcher
+Matches each 
lambda capture in a lambda expression.
+
+Given
+  int main() {
+int x, y;
+float z;
+auto f = [=]() { return x + y + z; };
+  }
+lambdaExpr(forEachLambdaCapture(
+lambdaCapture(capturesVar(varDecl(hasType(isInteger()))
+will trigger two matches, binding for 'x' and 'y' respectively.
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1LambdaExpr.html;>LambdaExprhasAnyCaptureLambdaCaptureMatcher 
InnerMatcher
 Matches any capture 
in a lambda expression.
 
@@ -5666,6 +5691,15 @@ Narrowing Matchers
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1VarDecl.html;>VarDeclisInitCapture
+Matches a variable 
serving as the implicit variable for a lambda init-
+capture.
+
+Example matches x (matcher = varDecl(isInitCapture()))
+auto f = [x=3]() { return x; };
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1VarDecl.html;>VarDeclisStaticLocal
 Matches a static 
variable with local scope.
 

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index aa49d4cf5e962..d6e5b215462b2 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4205,6 +4205,45 @@ AST_MATCHER_P(
   InnerMatcher.matches(*Initializer, Finder, Builder));
 }
 
+/// Matches a variable serving as the implicit variable for a lambda init-
+/// capture.
+///
+/// Example matches x (matcher = varDecl(isInitCapture()))
+/// \code
+/// auto f = [x=3]() { return x; };
+/// \endcode
+AST_MATCHER(VarDecl, isInitCapture) { return Node.isInitCapture(); }
+
+/// Matches each lambda capture in a lambda expression.
+///
+/// Given
+/// \code
+///   int main() {
+/// int x, y;
+/// float z;
+/// auto f = [=]() { return x + y + z; };
+///   }
+/// \endcode
+/// lambdaExpr(forEachLambdaCapture(
+/// lambdaCapture(capturesVar(varDecl(hasType(isInteger()))
+/// will trigger two matches, binding for 'x' and 'y' respectively.
+AST_MATCHER_P(LambdaExpr, forEachLambdaCapture, LambdaCaptureMatcher,
+  InnerMatcher) {
+  BoundNodesTreeBuilder Result;
+  bool Matched = false;
+  for (const auto  : Node.captures()) {
+if (Finder->isTraversalIgnoringImplicitNodes() && Capture.isImplicit())
+  continue;
+BoundNodesTreeBuilder CaptureBuilder(*Builder);
+if (InnerMatcher.matches(Capture, Finder, )) {
+  Matched = true;
+  Result.addMatch(CaptureBuilder);
+}
+  }
+  *Builder = std::move(Result);
+  return Matched;
+}
+
 /// \brief Matches a static variable with local scope.
 ///
 /// Example matches y (matcher = varDecl(isStaticLocal()))
@@ -4590,6 +4629,18 @@ AST_POLYMORPHIC_MATCHER_P(hasAnyArgument,
   return false;
 }
 
+/// Matches lambda captures.
+///
+/// Given
+/// \code
+///   int main() {
+/// int x;
+/// auto 

[PATCH] D113944: [CodeGenAction][NFC] Remove empty comment

[Youngsuk Kim via Phabricator via cfe-commits]

JOE1994 created this revision.
JOE1994 requested review of this revision.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113944

Files:
  clang/lib/CodeGen/CodeGenAction.cpp


Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1180,8 +1180,6 @@
 OptRecordFile->keep();
 }
 
-//
-
 void EmitAssemblyAction::anchor() { }
 EmitAssemblyAction::EmitAssemblyAction(llvm::LLVMContext *_VMContext)
   : CodeGenAction(Backend_EmitAssembly, _VMContext) {}


Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1180,8 +1180,6 @@
 OptRecordFile->keep();
 }
 
-//
-
 void EmitAssemblyAction::anchor() { }
 EmitAssemblyAction::EmitAssemblyAction(llvm::LLVMContext *_VMContext)
   : CodeGenAction(Backend_EmitAssembly, _VMContext) {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

[Arthur Eubanks via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG19867de9e793: [NewPM] Only invalidate modified 
functions analyses in CGSCC passes + turn on… (authored by aeubanks).

Changed prior to commit:
  https://reviews.llvm.org/D113304?vs=385642=387410#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113304

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-eager-invalidate.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-pgo-preinline.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/FunctionAttrs/invalidate.ll
  llvm/test/Transforms/Inline/analysis-invalidation.ll
  llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll

Index: llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
===
--- llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
+++ llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll
@@ -8,11 +8,11 @@
 ;
 ; CHECK: Running pass: InlinerPass on (test1_f, test1_g, test1_h)
 ; CHECK: Running analysis: DominatorTreeAnalysis on test1_f
-; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_f
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_f
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_f
 ; CHECK: Invalidating analysis: BlockFrequencyAnalysis on test1_f
+; CHECK: Running analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: DominatorTreeAnalysis on test1_g
 ; CHECK: Invalidating analysis: LoopAnalysis on test1_g
 ; CHECK: Invalidating analysis: BranchProbabilityAnalysis on test1_g
@@ -29,7 +29,6 @@
 ; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_h
 ; CHECK-NOT: Invalidating analysis:
 ; CHECK: Running pass: DominatorTreeVerifierPass on test1_f
-; CHECK-NEXT: Running analysis: DominatorTreeAnalysis on test1_f
 
 ; An external function used to control branches.
 declare i1 @flag()
Index: llvm/test/Transforms/Inline/analysis-invalidation.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/analysis-invalidation.ll
@@ -0,0 +1,17 @@
+; RUN: opt -passes=inline < %s -disable-output -debug-pass-manager 2>&1 | FileCheck %s
+
+; We shouldn't invalidate any function analyses on g since it's never modified.
+
+; CHECK-NOT: Invalidating{{.*}} on g
+; CHECK: Invalidating{{.*}} on f
+; CHECK-NOT: Invalidating{{.*}} on g
+
+define void @f() noinline {
+  call void @g()
+  ret void
+}
+
+define void @g() alwaysinline {
+  call void @f()
+  ret void
+}
Index: llvm/test/Transforms/FunctionAttrs/invalidate.ll
===
--- /dev/null
+++ llvm/test/Transforms/FunctionAttrs/invalidate.ll
@@ -0,0 +1,25 @@
+; RUN: opt -passes='function(require),cgscc(function-attrs)' -disable-output < %s -debug-pass-manager 2>&1 | FileCheck %s
+
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (f)
+; CHECK: Invalidating analysis: NoOpFunctionAnalysis on f
+; CHECK-NOT: Invalidating analysis: NoOpFunctionAnalysis on h
+; CHECK: Invalidating analysis: NoOpFunctionAnalysis on g
+; CHECK-NOT: Invalidating analysis: NoOpFunctionAnalysis on h
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (g)
+; CHECK: Running pass: PostOrderFunctionAttrsPass on (h)
+
+declare i32 @e(i32(i32)*)
+
+define i32 @f(i32 %a) {
+  ret i32 %a
+}
+
+define i32 @g(i32 %b) {
+  %c = call i32 @f(i32 %b)
+  ret i32 %c
+}
+
+define i32 @h(i32 %b) {
+  %c = call i32 @e(i32(i32)* @f)
+  ret i32 %c
+}
Index: llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
@@ -1,26 +1,26 @@
 ; Validate ThinLTO prelink pipeline when we have Sample PGO
 ;
-; RUN: opt -disable-verify -verify-cfg-preserved=0 -debug-pass-manager \
+; RUN: opt -disable-verify -verify-cfg-preserved=0 -eagerly-invalidate-analyses=0 -debug-pass-manager \
 ; RUN: -pgo-kind=pgo-sample-use-pipeline -profile-file='%S/Inputs/new-pm-thinlto-samplepgo-defaults.prof' \
 ; RUN: -passes='thinlto-pre-link' -S %s 2>&1 \
 ; RUN: 

[clang] 19867de - [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2021-11-15T14:44:53-08:00
New Revision: 19867de9e79327207796a16c1c24ac5d2cafecf9

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

LOG: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + 
turn on eagerly invalidate analyses

Previously, any change in any function in an SCC would cause all
analyses for all functions in the SCC to be invalidated. With this
change, we now manually invalidate analyses for functions we modify,
then let the pass manager know that all function analyses should be
preserved since we've already handled function analysis invalidation.

So far this only touches the inliner, argpromotion, function-attrs, and
updateCGAndAnalysisManager(), since they are the most used.

This is part of an effort to investigate running the function
simplification pipeline less on functions we visit multiple times in the
inliner pipeline.

However, this causes major memory regressions especially on larger IR.
To counteract this, turn on the option to eagerly invalidate function
analyses. This invalidates analyses on functions immediately after
they're processed in a module or scc to function adaptor for specific
parts of the pipeline.

Within an SCC, if a pass only modifies one function, other functions in
the SCC do not have their analyses invalidated, so in later function
passes in the SCC pass manager the analyses may still be cached. It is
only after the function passes that the eager invalidation takes effect.
For the default pipelines this makes sense because the inliner pipeline
runs the function simplification pipeline after all other SCC passes
(except CoroSplit which doesn't request any analyses).

Overall this has mostly positive effects on compile time and positive effects 
on memory usage.
https://llvm-compile-time-tracker.com/compare.php?from=7f627596977624730f9298a1b69883af1555765e=39e824e0d3ca8a517502f13032dfa67304841c90=instructions
https://llvm-compile-time-tracker.com/compare.php?from=7f627596977624730f9298a1b69883af1555765e=39e824e0d3ca8a517502f13032dfa67304841c90=max-rss

D113196 shows that we slightly regressed compile times in exchange for
some memory improvements when turning on eager invalidation.  D100917
shows that we slightly improved compile times in exchange for major
memory regressions in some cases when invalidating less in SCC passes.
Turning these on at the same time keeps the memory improvements while
keeping compile times neutral/slightly positive.

Reviewed By: asbirlea, nikic

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

Added: 
llvm/test/Transforms/FunctionAttrs/invalidate.ll
llvm/test/Transforms/Inline/analysis-invalidation.ll

Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll
llvm/lib/Analysis/CGSCCPassManager.cpp
llvm/lib/Passes/PassBuilderPipelines.cpp
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
llvm/lib/Transforms/IPO/FunctionAttrs.cpp
llvm/lib/Transforms/IPO/Inliner.cpp
llvm/test/Other/new-pm-defaults.ll
llvm/test/Other/new-pm-eager-invalidate.ll
llvm/test/Other/new-pm-lto-defaults.ll
llvm/test/Other/new-pm-pgo-preinline.ll
llvm/test/Other/new-pm-thinlto-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
llvm/test/Transforms/Inline/cgscc-incremental-invalidate.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index 67a86c3b45049..8ec8b7c5e4aff 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -20,67 +20,31 @@
 ; RUN:   -o %t.native.o -x ir %t.o 2>&1 | FileCheck 
-check-prefixes=CHECK-O,CHECK-O3 %s --dump-input=fail
 
 ; CHECK-O: Running pass: WholeProgramDevirtPass
-; CHECK-O: Running analysis: InnerAnalysisManagerProxy
 ; CHECK-O: Running pass: LowerTypeTestsPass
-; CHECK-O: Invalidating analysis: InnerAnalysisManagerProxy
 ; CHECK-O: Running pass: ForceFunctionAttrsPass
 ; CHECK-O: Running pass: PGOIndirectCallPromotion
-; CHECK-O: Running analysis: ProfileSummaryAnalysis
-; CHECK-O: Running analysis: InnerAnalysisManagerProxy
-; CHECK-O: Running analysis: OptimizationRemarkEmitterAnalysis on main
 ; CHECK-O: Running pass: InferFunctionAttrsPass
 ; CHECK-O: Running pass: LowerExpectIntrinsicPass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main
-; CHECK-O: Running analysis: TargetIRAnalysis on main
-; CHECK-O: Running analysis: AssumptionAnalysis on main
 ; CHECK-O: Running pass: SROAPass on main
-; CHECK-O: Running analysis: DominatorTreeAnalysis on 

[PATCH] D113943: Add withTag matcher.

[James King via Phabricator via cfe-commits]

jcking1034 created this revision.
jcking1034 added reviewers: ymandel, tdl-g, aaron.ballman.
jcking1034 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adds a `withTag` matcher which outputs contextual information for
better debugging. This relies on changes made in
https://reviews.llvm.org/D113917 to access the names of matchers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113943

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h


Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -43,6 +43,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -1042,6 +1043,44 @@
   std::vector Names;
 };
 
+template  class WithTagMatcher : public MatcherInterface {
+public:
+  explicit WithTagMatcher(std::string _BeforeTag, std::string _AfterTag,
+  internal::Matcher _InnerMatcher)
+  : BeforeTag(std::move(_BeforeTag)), AfterTag(std::move(_AfterTag)),
+InnerMatcher(_InnerMatcher) {}
+  explicit WithTagMatcher(internal::Matcher _InnerMatcher)
+  : BeforeTag("⭐ Attempting new match"),
+AfterTag("✔️ Concluding attempt"), InnerMatcher(_InnerMatcher) {}
+  bool matches(const T , ASTMatchFinder *Finder,
+   BoundNodesTreeBuilder *Builder) const override {
+DynTypedNode DTN = DynTypedNode::create(Node);
+
+llvm::errs() << BeforeTag << "\n";
+
+llvm::errs() << "Matcher Name: " << InnerMatcher.getMatcherName() << "\n";
+
+llvm::errs() << "Node Kind: " << DTN.getNodeKind().asStringRef() << "\n"
+ << "Node Value:\n```\n";
+DTN.print(llvm::errs(), PrintingPolicy(LangOptions()));
+llvm::errs() << "\n```\n"
+ << "Node AST:\n";
+DTN.dump(llvm::errs(), Finder->getASTContext());
+
+bool result = InnerMatcher.matches(Node, Finder, Builder);
+llvm::errs() << "Result: " << (result ? "Successful\n" : "Unsuccessful\n");
+
+llvm::errs() << AfterTag << "\n\n";
+
+return result;
+  }
+
+private:
+  std::string BeforeTag;
+  std::string AfterTag;
+  internal::Matcher InnerMatcher;
+};
+
 /// Matches named declarations with a specific name.
 ///
 /// See \c hasName() and \c hasAnyName() in ASTMatchers.h for details.
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2993,6 +2993,37 @@
   new internal::HasNameMatcher({std::string(Name)}));
 }
 
+template 
+inline internal::Matcher withTag(std::string BeforeTag, std::string 
AfterTag,
+const internal::Matcher ) {
+  return internal::Matcher(
+  new internal::WithTagMatcher(BeforeTag, AfterTag, InnerMatcher));
+}
+
+template  class MatcherT,
+  typename ReturnTypesF, typename... ParamTypes>
+inline MatcherT
+withTag(std::string BeforeTag, std::string AfterTag,
+const internal::PolymorphicMatcher ) {
+  return internal::Matcher(new internal::WithTagMatcher(
+  BeforeTag, AfterTag, internal::Matcher(InnerMatcher)));
+}
+
+template 
+inline internal::Matcher withTag(const internal::Matcher ) {
+  return internal::Matcher(new internal::WithTagMatcher(InnerMatcher));
+}
+
+template  class MatcherT,
+  typename ReturnTypesF, typename... ParamTypes>
+inline MatcherT
+withTag(const internal::PolymorphicMatcher ) {
+  return internal::Matcher(
+  new internal::WithTagMatcher(internal::Matcher(InnerMatcher)));
+}
+
 /// Matches NamedDecl nodes that have any of the specified names.
 ///
 /// This matcher is only provided as a performance optimization of hasName.


Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -43,6 +43,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -1042,6 +1043,44 @@
   std::vector Names;
 };
 
+template  class WithTagMatcher : public MatcherInterface {
+public:
+  explicit WithTagMatcher(std::string _BeforeTag, std::string _AfterTag,
+  internal::Matcher _InnerMatcher)
+  : BeforeTag(std::move(_BeforeTag)), AfterTag(std::move(_AfterTag)),
+

[PATCH] D113707: [clang] Make -masm=intel affect inline asm style

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D113707#3132812 , @thakis wrote:

> Fairly big update. I'd like to punt on the pragma for now since this became a 
> lot more involved than expected already (see dependent patches; several of 
> them have already landed).

No problem.

This seems very reasonable.


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

https://reviews.llvm.org/D113707

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

[PATCH] D113942: [NFC] Inclusive language: replace master with main in convert_arm_neon.py

[Quinn Pham via Phabricator via cfe-commits]

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

[NFC] As part of using inclusive language within the llvm project and to
match the renamed master branch, this patch replaces master with main in
`convert_arm_neon.py`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113942

Files:
  clang/utils/convert_arm_neon.py


Index: clang/utils/convert_arm_neon.py
===
--- clang/utils/convert_arm_neon.py
+++ clang/utils/convert_arm_neon.py
@@ -7,7 +7,7 @@
 # using the old single-char type modifiers to an equivalent new-style form 
where
 # each modifier is orthogonal and they can be composed.
 #
-# It was used to directly generate the .td files on master, so if you have any
+# It was used to directly generate the .td files on main, so if you have any
 # local additions I would suggest implementing any modifiers here, and running
 # it over your entire pre-merge .td files rather than trying to resolve any
 # conflicts manually.


Index: clang/utils/convert_arm_neon.py
===
--- clang/utils/convert_arm_neon.py
+++ clang/utils/convert_arm_neon.py
@@ -7,7 +7,7 @@
 # using the old single-char type modifiers to an equivalent new-style form where
 # each modifier is orthogonal and they can be composed.
 #
-# It was used to directly generate the .td files on master, so if you have any
+# It was used to directly generate the .td files on main, so if you have any
 # local additions I would suggest implementing any modifiers here, and running
 # it over your entire pre-merge .td files rather than trying to resolve any
 # conflicts manually.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 387404.
mizvekov added a comment.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

- Add `libcxx/DELETE.ME` to trigger libcxx CI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  libcxx/DELETE.ME

Index: libcxx/DELETE.ME
===
--- /dev/null
+++ libcxx/DELETE.ME
@@ -0,0 +1 @@
+D111283
Index: clang/test/SemaCXX/sugared-auto.cpp
===
--- clang/test/SemaCXX/sugared-auto.cpp
+++ clang/test/SemaCXX/sugared-auto.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -fblocks -fenable-matrix -Wno-dynamic-exception-spec
 
 enum class N {};
 
@@ -9,6 +9,26 @@
 using Man = Animal;
 using Dog = Animal;
 
+using ManPtr = Man *;
+using DogPtr = Dog *;
+
+using SocratesPtr = ManPtr;
+
+using ConstMan = const Man;
+using ConstDog = const Dog;
+
+using Virus = void;
+using SARS = Virus;
+using Ebola = Virus;
+
+using Bacteria = float;
+using Bacilli = Bacteria;
+using Vibrio = Bacteria;
+
+struct Plant;
+using Gymnosperm = Plant;
+using Angiosperm = Plant;
+
 namespace variable {
 
 auto x1 = Animal();
@@ -41,3 +61,115 @@
 N t3 = x3; // expected-error {{lvalue of type 'Animal' (aka 'int')}}
 
 } // namespace function_basic
+
+namespace function_multiple_basic {
+
+N t1 = [] { // expected-error {{rvalue of type 'Animal' (aka 'int')}}
+  if (true)
+return Man();
+  return Dog();
+}();
+
+N t2 = []() -> decltype(auto) { // expected-error {{rvalue of type 'Animal' (aka 'int')}}
+  if (true)
+return Man();
+  return Dog();
+}();
+
+N t3 = [] { // expected-error {{rvalue of type 'Animal' (aka 'int')}}
+  if (true)
+return Dog();
+  auto x = Man();
+  return x;
+}();
+
+N t4 = [] { // expected-error {{rvalue of type 'int'}}
+  if (true)
+return Dog();
+  return 1;
+}();
+
+N t5 = [] { // expected-error {{rvalue of type 'Virus' (aka 'void')}}
+  if (true)
+return Ebola();
+  return SARS();
+}();
+
+N t6 = [] { // expected-error {{rvalue of type 'void'}}
+  if (true)
+return SARS();
+  return;
+}();
+
+} // namespace function_multiple_basic
+
+#define TEST_AUTO(X, A, B) \
+  auto X(A a, B b) {   \
+if (0) \
+  return a;\
+if (0) \
+  return b;\
+return N();\
+  }
+#define TEST_DAUTO(X, A, B) \
+  decltype(auto) X(A a, B b) {  \
+if (0)  \
+  return static_cast(a); \
+if (0)  \
+  return static_cast(b); \
+return N(); \
+  }
+
+namespace misc {
+
+TEST_AUTO(t1, ManPtr, DogPtr)  // expected-error {{but deduced as 'Animal *' (aka 'int *')}}
+TEST_AUTO(t2, ManPtr, int *)   // expected-error {{but deduced as 'int *'}}
+TEST_AUTO(t3, SocratesPtr, ManPtr) // expected-error {{but deduced as 'ManPtr' (aka 'int *')}}
+
+TEST_AUTO(t4, _Atomic(Man), _Atomic(Dog)) // expected-error {{but deduced as '_Atomic(Animal)'}}
+
+using block_man = void (^)(Man);
+using block_dog = void (^)(Dog);
+TEST_AUTO(t5, block_man, block_dog) // expected-error {{but deduced as 'void (^)(Animal)'}}
+
+using fp1 = SARS (*)(Man, DogPtr) throw(Vibrio);
+using fp2 = Ebola (*)(Dog, ManPtr) throw(Bacilli);
+TEST_AUTO(t6, fp1, fp2); // expected-error {{but deduced as 'Virus (*)(Animal, Animal *) throw(Bacteria)' (aka 'void (*)(int, int *) throw(Bacteria)')}}
+
+using fp3 = SARS (*)() throw(Man);
+using fp4 = Ebola (*)() throw(Vibrio);
+auto t7(fp3 a, fp4 b) {
+  if (false)
+return true ? a : b;
+  if (false)
+return a;
+  return N(); // expected-error {{but deduced as 'SARS (*)() throw(Man, Vibrio)' (aka 'void (*)() throw(Man, Vibrio)')}}
+}
+
+using fp5 = void (*)(const Man);
+using fp6 = void (*)(Dog);
+TEST_AUTO(t8, fp5, fp6); // expected-error {{but deduced as 'void (*)(const Animal)' (aka 'void (*)(const int)')}}
+
+using fp6 = void (*)(ConstMan);
+using fp7 = void (*)(ConstDog);
+TEST_AUTO(t10, fp6, fp7); // expected-error {{but deduced as 'void (*)(const Animal)' (aka 'void (*)(const int)')}}
+
+TEST_AUTO(t11, Man Angiosperm::*, Dog Gymnosperm::*) // expected-error {{but deduced as 'Animal Plant::*'}}
+
+TEST_DAUTO(t12, const Man &, const Dog &) // expected-error {{but deduced as 'const Animal &' (aka 'const int &')}}
+
+TEST_DAUTO(t13, Man &&, Dog &&) // expected-error {{but deduced as 

[PATCH] D113830: [clang-tidy] Fix false positive in `readability-identifier-naming` check involving `override` attribute

[Fabian Wolff via Phabricator via cfe-commits]

fwolff updated this revision to Diff 387401.
fwolff added a comment.

Thanks again for your feedback @salman-javed-nz! I think I've addressed all of 
your comments now.


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

https://reviews.llvm.org/D113830

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp
@@ -285,6 +285,10 @@
   virtual void BadBaseMethod() = 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
   // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
+
+  virtual void BadBaseMethodNoAttr() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethodNoAttr'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method_No_Attr() = 0;
 };
 
 class COverriding : public AOverridden {
@@ -293,6 +297,9 @@
   void BadBaseMethod() override {}
   // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override {}
 
+  void BadBaseMethodNoAttr() /* override */ {}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method_No_Attr() /* override */ {}
+
   void foo() {
 BadBaseMethod();
 // CHECK-FIXES: {{^}}v_Bad_Base_Method();
@@ -302,12 +309,79 @@
 // CHECK-FIXES: {{^}}AOverridden::v_Bad_Base_Method();
 COverriding::BadBaseMethod();
 // CHECK-FIXES: {{^}}COverriding::v_Bad_Base_Method();
+
+BadBaseMethodNoAttr();
+// CHECK-FIXES: {{^}}v_Bad_Base_Method_No_Attr();
+this->BadBaseMethodNoAttr();
+// CHECK-FIXES: {{^}}this->v_Bad_Base_Method_No_Attr();
+AOverridden::BadBaseMethodNoAttr();
+// CHECK-FIXES: {{^}}AOverridden::v_Bad_Base_Method_No_Attr();
+COverriding::BadBaseMethodNoAttr();
+// CHECK-FIXES: {{^}}COverriding::v_Bad_Base_Method_No_Attr();
   }
 };
 
-void VirtualCall(AOverridden _vItem) {
+// Same test as above, now with a dependent base class.
+template
+class ATOverridden {
+public:
+  virtual void BadBaseMethod() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
+
+  virtual void BadBaseMethodNoAttr() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethodNoAttr'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method_No_Attr() = 0;
+};
+
+template
+class CTOverriding : public ATOverridden {
+  // Overriding a badly-named base isn't a new violation.
+  // FIXME: The fixes from the base class should be propagated to the derived class here
+  //(note that there could be specializations of the template base class, though)
+  void BadBaseMethod() override {}
+
+  // Without the "override" attribute, and due to the dependent base class, it is not
+  // known whether this method overrides anything, so we get the warning here.
+  virtual void BadBaseMethodNoAttr() {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethodNoAttr'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method_No_Attr() {};
+};
+
+template
+void VirtualCall(AOverridden _vItem, ATOverridden _vTitem) {
+  a_vItem.BadBaseMethod();
+  // CHECK-FIXES: {{^}}  a_vItem.v_Bad_Base_Method();
+
+  // FIXME: The fixes from ATOverridden should be propagated to the following call
+  a_vTitem.BadBaseMethod();
+}
+
+// Same test as above, now with a dependent base class that is instantiated below.
+template
+class ATIOverridden {
+public:
+  virtual void BadBaseMethod() = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for virtual method 'BadBaseMethod'
+  // CHECK-FIXES: {{^}}  virtual void v_Bad_Base_Method() = 0;
+};
+
+template
+class CTIOverriding : public ATIOverridden {
+public:
+  // Overriding a badly-named base isn't a new violation.
+  void BadBaseMethod() override {}
+  // CHECK-FIXES: {{^}}  void v_Bad_Base_Method() override {}
+};
+
+template class CTIOverriding;
+
+void VirtualCallI(ATIOverridden& a_vItem, CTIOverriding& a_vCitem) {
   a_vItem.BadBaseMethod();
   // CHECK-FIXES: {{^}}  a_vItem.v_Bad_Base_Method();
+
+  a_vCitem.BadBaseMethod();
+  // CHECK-FIXES: {{^}}  a_vCitem.v_Bad_Base_Method();
 }
 
 template 
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -1257,7 +1257,7 @@
 
   if (const auto *Decl = dyn_cast(D)) {
 if (Decl->isMain() || 

[PATCH] D113707: [clang] Make -masm=intel affect inline asm style

[Nico Weber via Phabricator via cfe-commits]

thakis updated this revision to Diff 387399.
thakis added a comment.

minor test expectations update


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

https://reviews.llvm.org/D113707

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Headers/immintrin.h
  clang/lib/Headers/intrin.h
  clang/lib/Headers/x86gprintrin.h
  clang/test/CodeGen/inline-asm-intel.c
  clang/test/CodeGen/inline-asm-mixed-style.c
  clang/test/CodeGen/ms-intrinsics-cpuid.c
  clang/test/CodeGen/ms-intrinsics.c
  clang/test/Driver/masm.c

Index: clang/test/Driver/masm.c
===
--- clang/test/Driver/masm.c
+++ clang/test/Driver/masm.c
@@ -6,9 +6,12 @@
 
 int f() {
 // CHECK-INTEL: -x86-asm-syntax=intel
+// CHECK-INTEL: -inline-asm=intel
 // CHECK-ATT: -x86-asm-syntax=att
+// CHECK-ATT: -inline-asm=att
 // CHECK-SOMEREQUIRED: error: unsupported argument 'somerequired' to option 'masm='
 // CHECK-ARM: warning: argument unused during compilation: '-masm=intel'
 // CHECK-CL: -x86-asm-syntax=intel
+// CHECK-CL-NOT: -inline-asm=intel
   return 0;
 }
Index: clang/test/CodeGen/ms-intrinsics.c
===
--- clang/test/CodeGen/ms-intrinsics.c
+++ clang/test/CodeGen/ms-intrinsics.c
@@ -36,7 +36,7 @@
   return __movsb(Dest, Src, Count);
 }
 // CHECK-I386-LABEL: define{{.*}} void @test__movsb
-// CHECK-I386:   tail call { i8*, i8*, i32 } asm sideeffect "xchg %esi, $1\0Arep movsb\0Axchg %esi, $1", "={di},=r,={cx},0,1,2,~{memory},~{dirflag},~{fpsr},~{flags}"(i8* %Dest, i8* %Src, i32 %Count)
+// CHECK-I386:   tail call { i8*, i8*, i32 } asm sideeffect "xchg $(%esi, $1$|$1, esi$)\0Arep movsb\0Axchg $(%esi, $1$|$1, esi$)", "={di},=r,={cx},0,1,2,~{memory},~{dirflag},~{fpsr},~{flags}"(i8* %Dest, i8* %Src, i32 %Count)
 // CHECK-I386:   ret void
 // CHECK-I386: }
 
@@ -62,7 +62,7 @@
   return __movsw(Dest, Src, Count);
 }
 // CHECK-I386-LABEL: define{{.*}} void @test__movsw
-// CHECK-I386:   tail call { i16*, i16*, i32 } asm sideeffect "xchg %esi, $1\0Arep movsw\0Axchg %esi, $1", "={di},=r,={cx},0,1,2,~{memory},~{dirflag},~{fpsr},~{flags}"(i16* %Dest, i16* %Src, i32 %Count)
+// CHECK-I386:   tail call { i16*, i16*, i32 } asm sideeffect "xchg $(%esi, $1$|$1, esi$)\0Arep movsw\0Axchg $(%esi, $1$|$1, esi$)", "={di},=r,={cx},0,1,2,~{memory},~{dirflag},~{fpsr},~{flags}"(i16* %Dest, i16* %Src, i32 %Count)
 // CHECK-I386:   ret void
 // CHECK-I386: }
 
@@ -75,12 +75,12 @@
   return __stosd(Dest, Data, Count);
 }
 // CHECK-I386-LABEL: define{{.*}} void @test__stosd
-// CHECK-I386:   call { i32*, i32 } asm sideeffect "rep stosl", "={di},={cx},{ax},0,1,~{memory},~{dirflag},~{fpsr},~{flags}"(i32 %Data, i32* %Dest, i32 %Count)
+// CHECK-I386:   call { i32*, i32 } asm sideeffect "rep stos$(l$|d$)", "={di},={cx},{ax},0,1,~{memory},~{dirflag},~{fpsr},~{flags}"(i32 %Data, i32* %Dest, i32 %Count)
 // CHECK-I386:   ret void
 // CHECK-I386: }
 
 // CHECK-X64-LABEL: define{{.*}} void @test__stosd
-// CHECK-X64:   call { i32*, i64 } asm sideeffect "rep stosl", "={di},={cx},{ax},0,1,~{memory},~{dirflag},~{fpsr},~{flags}"(i32 %Data, i32* %Dest, i64 %Count)
+// CHECK-X64:   call { i32*, i64 } asm sideeffect "rep stos$(l$|d$)", "={di},={cx},{ax},0,1,~{memory},~{dirflag},~{fpsr},~{flags}"(i32 %Data, i32* %Dest, i64 %Count)
 // CHECK-X64:   ret void
 // CHECK-X64: }
 
@@ -88,12 +88,12 @@
   return __movsd(Dest, Src, Count);
 }
 // CHECK-I386-LABEL: define{{.*}} void @test__movsd
-// CHECK-I386:   tail call { i32*, i32*, i32 } asm sideeffect "xchg %esi, $1\0Arep movsl\0Axchg %esi, $1", "={di},=r,={cx},0,1,2,~{memory},~{dirflag},~{fpsr},~{flags}"(i32* %Dest, i32* %Src, i32 %Count)
+// CHECK-I386:   tail call { i32*, i32*, i32 } asm sideeffect "xchg $(%esi, $1$|$1, esi$)\0Arep movs$(l$|d$)\0Axchg $(%esi, $1$|$1, esi$)", "={di},=r,={cx},0,1,2,~{memory},~{dirflag},~{fpsr},~{flags}"(i32* %Dest, i32* %Src, i32 %Count)
 // CHECK-I386:   ret void
 // CHECK-I386: }
 
 // CHECK-X64-LABEL: define{{.*}} void @test__movsd
-// CHECK-X64:   call { i32*, i32*, i64 } asm sideeffect "rep movsl", "={di},={si},={cx},0,1,2,~{memory},~{dirflag},~{fpsr},~{flags}"(i32* %Dest, i32* %Src, i64 %Count)
+// CHECK-X64:   call { i32*, i32*, i64 } asm sideeffect "rep movs$(l$|d$)", "={di},={si},={cx},0,1,2,~{memory},~{dirflag},~{fpsr},~{flags}"(i32* %Dest, i32* %Src, i64 %Count)
 // CHECK-X64:   ret void
 // CHECK-X64: }
 
@@ -626,48 +626,48 @@
 #if defined(__i386__) || defined(__x86_64__)
 long test_InterlockedExchange_HLEAcquire(long volatile *Target, long Value) {
 // CHECK-INTEL: define{{.*}} i32 @test_InterlockedExchange_HLEAcquire(i32*{{[a-z_ ]*}}%Target, i32{{[a-z_ ]*}}%Value)
-// CHECK-INTEL: call i32 asm sideeffect ".byte 0xf2 ; lock ; xchg $0, $1", 

[PATCH] D113211: [NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/

[Jan Korous via Phabricator via cfe-commits]

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

LGTM




Comment at: clang/test/CodeGen/address-safety-attr.cpp:9
 
-// RUN: echo "fun:*BlacklistedFunction*" > %t.func.blacklist
-// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin -disable-O0-optnone 
-emit-llvm -o - %s -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-blacklist=%t.func.blacklist | FileCheck -check-prefix=BLFUNC %s
+// RUN: echo "fun:*IgnorelistedFunction*" > %t.func.ignorelist
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin -disable-O0-optnone 
-emit-llvm -o - %s -include %t.extra-source.cpp -fsanitize=address 
-fsanitize-ignorelist=%t.func.ignorelist | FileCheck -check-prefix=BLFUNC %s

jkorous wrote:
> Shouldn't we teach ASan about the new syntax of ignorelist as well 
> `fun:*IgnorelistedFunction*`?
Nvm - that's just name of the test function below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113211

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

[PATCH] D113925: [HIP] Add HIP scope atomic operations

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 387396.
gandhi21299 added a comment.

reapplied clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113925

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/SyncScope.h
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCUDA/atomic-ops.cu

Index: clang/test/CodeGenCUDA/atomic-ops.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/atomic-ops.cu
@@ -0,0 +1,302 @@
+// RUN: %clang_cc1 -x hip -std=c++11 -triple amdgcn -fcuda-is-device -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+
+// CHECK-LABEL: @_Z24atomic32_op_singlethreadPiii
+// CHECK: cmpxchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw xchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw add i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw and i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw or i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw xor i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw min i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw max i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+__device__ int atomic32_op_singlethread(int *ptr, int val, int desired) {
+  bool flag = __hip_atomic_compare_exchange_strong(ptr, , desired, __ATOMIC_RELAXED, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_exchange(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_add(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_and(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_or(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_xor(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_min(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_max(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  return flag ? val : desired;
+}
+
+// CHECK-LABEL: @_Z25atomicu32_op_singlethreadPjjj
+// CHECK: atomicrmw umin i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw umax i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+__device__ unsigned int atomicu32_op_singlethread(unsigned int *ptr, unsigned int val, unsigned int desired) {
+  val = __hip_atomic_fetch_min(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_max(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  return val;
+}
+
+// CHECK-LABEL: @_Z21atomic32_op_wavefrontPiii
+// CHECK: cmpxchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw xchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw add i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw and i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw or i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw xor i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw min i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw max i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+__device__ int atomic32_op_wavefront(int *ptr, int val, int desired) {
+  bool flag = __hip_atomic_compare_exchange_strong(ptr, , desired, __ATOMIC_RELAXED, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_exchange(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_add(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_and(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_or(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_xor(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_min(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_max(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  return flag ? val : desired;
+}
+
+// CHECK-LABEL: @_Z22atomicu32_op_wavefrontPjjj
+// CHECK: atomicrmw umin i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw umax i32* {{%[0-9]+}}, i32 

[PATCH] D113925: [HIP] Add HIP scope atomic operations

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 387395.
gandhi21299 added a comment.

clang-formatted code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113925

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/SyncScope.h
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCUDA/atomic-ops.cu

Index: clang/test/CodeGenCUDA/atomic-ops.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/atomic-ops.cu
@@ -0,0 +1,302 @@
+// RUN: %clang_cc1 -x hip -std=c++11 -triple amdgcn -fcuda-is-device -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+
+// CHECK-LABEL: @_Z24atomic32_op_singlethreadPiii
+// CHECK: cmpxchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw xchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw add i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw and i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw or i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw xor i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw min i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw max i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+__device__ int atomic32_op_singlethread(int *ptr, int val, int desired) {
+  bool flag = __hip_atomic_compare_exchange_strong(ptr, , desired, __ATOMIC_RELAXED, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_exchange(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_add(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_and(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_or(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_xor(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_min(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_max(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  return flag ? val : desired;
+}
+
+// CHECK-LABEL: @_Z25atomicu32_op_singlethreadPjjj
+// CHECK: atomicrmw umin i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw umax i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+__device__ unsigned int atomicu32_op_singlethread(unsigned int *ptr, unsigned int val, unsigned int desired) {
+  val = __hip_atomic_fetch_min(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_max(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  return val;
+}
+
+// CHECK-LABEL: @_Z21atomic32_op_wavefrontPiii
+// CHECK: cmpxchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw xchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw add i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw and i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw or i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw xor i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw min i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw max i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+__device__ int atomic32_op_wavefront(int *ptr, int val, int desired) {
+  bool flag = __hip_atomic_compare_exchange_strong(ptr, , desired, __ATOMIC_RELAXED, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_exchange(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_add(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_and(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_or(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_xor(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_min(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_max(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  return flag ? val : desired;
+}
+
+// CHECK-LABEL: @_Z22atomicu32_op_wavefrontPjjj
+// CHECK: atomicrmw umin i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw umax i32* {{%[0-9]+}}, i32 

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added inline comments.



Comment at: clang/test/CodeGen/X86/avx512fp16-complex.c:123
   // X86-NOT: fdiv
-  // X86: call {{.*}} @__divhc3(
+  // X86: call {{.*}} @__divsc3(
   // X86: ret

zahiraam wrote:
> pengfei wrote:
> > andrew.w.kaylor wrote:
> > > Why did this change happen?
> > This shouldn't be affect either.
> Still working on this.  
Still not sure what the reason but I think it is because of the coerce.
The IR generated for div_half_rc is :
%retval = alloca { half, half }, align 2
  %b = alloca { half, half }, align 2
  %a.addr = alloca half, align 2
  **%coerce = alloca { float, float }, align 4**
  %0 = bitcast { half, half }* %b to <2 x half>*

I would have expected to have %coerce = alloca { half, half }, align 2 
Still not sure what the issue is.


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

https://reviews.llvm.org/D113107

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 387387.

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

https://reviews.llvm.org/D113107

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGen/X86/Float16-aritmetic.c
  clang/test/CodeGen/X86/Float16-complex.c
  clang/test/CodeGen/X86/avx512fp16-complex.c
  clang/test/Sema/Float16.c
  clang/test/Sema/conversion-target-dep.c
  clang/test/SemaCXX/Float16.cpp

Index: clang/test/SemaCXX/Float16.cpp
===
--- clang/test/SemaCXX/Float16.cpp
+++ clang/test/SemaCXX/Float16.cpp
@@ -1,18 +1,8 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifdef HAVE
 // expected-no-diagnostics
-#endif // HAVE
-
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // !HAVE
 _Float16 f;
-
-#ifndef HAVE
-// expected-error@+2{{invalid suffix 'F16' on floating constant}}
-#endif // !HAVE
 const auto g = 1.1F16;
Index: clang/test/Sema/conversion-target-dep.c
===
--- clang/test/Sema/conversion-target-dep.c
+++ clang/test/Sema/conversion-target-dep.c
@@ -6,7 +6,7 @@
 
 long double ld;
 double d;
-_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+_Float16 f16;
 
 int main() {
   ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
Index: clang/test/Sema/Float16.c
===
--- clang/test/Sema/Float16.c
+++ clang/test/Sema/Float16.c
@@ -1,18 +1,11 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // HAVE
-_Float16 f;
-
-#ifdef HAVE
 _Complex _Float16 a;
 void builtin_complex() {
   _Float16 a = 0;
   (void)__builtin_complex(a, a); // expected-error {{'_Complex _Float16' is invalid}}
 }
-#endif
Index: clang/test/CodeGen/X86/Float16-complex.c
===
--- clang/test/CodeGen/X86/Float16-complex.c
+++ clang/test/CodeGen/X86/Float16-complex.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -target-feature +avx512fp16 -o - | FileCheck %s --check-prefix=X86
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s --check-prefixes=X86
 
 _Float16 _Complex add_half_rr(_Float16 a, _Float16 b) {
   // X86-LABEL: @add_half_rr(
Index: clang/test/CodeGen/X86/Float16-aritmetic.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/Float16-aritmetic.c
@@ -0,0 +1,69 @@
+// RUN: %clang_cc1 -triple  x86_64-unknown-unknown -emit-llvm  \
+// RUN: < %s  | FileCheck %s --check-prefixes=CHECK
+
+_Float16 add1(_Float16 a, _Float16 b) {
+  // CHECK-LABEL: define {{.*}} half @add1
+  // CHECK: alloca half
+  // CHECK: alloca half
+  // CHECK: store half {{.*}}, half*
+  // CHECK: store half {{.*}}, half*
+  // CHECK: load half, half*
+  // CHECK: fpext half {{.*}} to float
+  // CHECK: load half, half* {{.*}}
+  // CHECK: fpext half {{.*}} to float
+  // CHECK: fadd float {{.*}}, {{.*}}
+  // CHECK: fptrunc float {{.*}} to half
+  // CHECK: ret half
+  return a + b;
+}
+
+_Float16 add2(_Float16 a, _Float16 b, _Float16 c) {
+  // CHECK-LABEL: define dso_local half @add2
+  // CHECK: alloca half
+  // CHECK: alloca half
+  // CHECK: alloca half
+  // CHECK: load half, half* {{.*}}
+  // CHECK: fpext half {{.*}} to float
+  // CHECK: load half, half* {{.*}}
+  // CHECK: fpext half {{.*}} to float
+  // CHECK: fadd float {{.*}}, {{.*}}
+  // CHECK: load half, half* {{.*}}
+  // CHECK: fpext half {{.*}} to float
+  // CHECK: fadd float {{.*}}, 

[PATCH] D113585: [clang-tidy] Fix false positive in `bugprone-throw-keyword-missing` check

[Fabian Wolff via Phabricator via cfe-commits]

fwolff added a comment.

In D113585#3132810 , @aaron.ballman 
wrote:

> In D113585#3132793 , @fwolff wrote:
>
>> Thanks for reviewing this! Can you also commit it for me?
>
> Sure can! What name and email address would you like used for patch 
> attribution? I'm in C standards meetings all this week so if someone else is 
> able to land this before I get to it, I will not be hurt. :-)

Thanks! You can use the same name and email as in commit 
`c3e3c762098e8d425731bb40f3b8b04dac1013f3`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113585

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

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

[Michael Benfield via Phabricator via cfe-commits]

mbenfield updated this revision to Diff 387389.
mbenfield added a comment.

s/requires/takes in diagnostic message.

Test case with a non-template overloaded function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112024

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/attr-diagnose-as-builtin.c

Index: clang/test/Sema/attr-diagnose-as-builtin.c
===
--- /dev/null
+++ clang/test/Sema/attr-diagnose-as-builtin.c
@@ -0,0 +1,77 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
+
+typedef unsigned long size_t;
+
+void *test_memcpy(const void *src, size_t c, void *dst) __attribute__((diagnose_as_builtin(__builtin_memcpy, 3, 1, 2))) {
+  return __builtin_memcpy(dst, src, c);
+}
+
+void call_test_memcpy() {
+  char bufferA[10];
+  char bufferB[11];
+  test_memcpy(bufferB, 10, bufferA);
+  test_memcpy(bufferB, 11, bufferA); // expected-warning {{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
+
+void failure_function_doesnt_exist() __attribute__((diagnose_as_builtin(__function_that_doesnt_exist))) {} // expected-error {{use of undeclared identifier '__function_that_doesnt_exist'}}
+
+void not_a_builtin() {}
+
+void failure_not_a_builtin() __attribute__((diagnose_as_builtin(not_a_builtin))) {} // expected-error {{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_too_many_parameters(void *dst, const void *src, size_t count, size_t nothing) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3, 4))) {} // expected-error {{'diagnose_as_builtin' attribute references function __builtin_memcpy, which takes exactly 3 arguments}}
+
+void failure_too_few_parameters(void *dst, const void *src) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2))) {} // expected-error{{'diagnose_as_builtin' attribute references function __builtin_memcpy, which takes exactly 3 arguments}}
+
+void failure_not_an_integer(void *dst, const void *src, size_t count) __attribute__((diagnose_as_builtin(__builtin_memcpy, "abc", 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 2 to be an integer constant}}
+
+void failure_not_a_builtin2() __attribute__((diagnose_as_builtin("abc"))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_parameter_index_bounds(void *dst, const void *src) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute references parameter 3, but the function failure_parameter_index_bounds has only 2 parameters}}
+
+void failure_parameter_types(double dst, const void *src, size_t count) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute parameter types do not match: parameter 1 of function 'failure_parameter_types' has type 'double', but parameter 1 of function '__builtin_memcpy' has type 'void *'}}
+
+void to_redeclare(void *dst, const void *src, size_t count);
+
+void use_to_redeclare() {
+  char src[10];
+  char dst[9];
+  // We shouldn't get an error yet.
+  to_redeclare(dst, src, 10);
+}
+
+void to_redeclare(void *dst, const void *src, size_t count) __attribute((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3)));
+
+void error_to_redeclare() {
+  char src[10];
+  char dst[9];
+  // Now we get an error.
+  to_redeclare(dst, src, 10); // expected-warning {{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}}
+}
+
+// Make sure this works even with extra qualifiers and the pass_object_size attribute.
+void *memcpy2(void *const dst __attribute__((pass_object_size(0))), const void *src, size_t copy_amount) __attribute((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3)));
+
+void call_memcpy2() {
+  char buf1[10];
+  char buf2[11];
+  memcpy2(buf1, buf2, 11); // expected-warning {{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
+
+#ifdef __cplusplus
+template 
+void some_templated_function(int x) {
+  return;
+}
+
+void failure_template(int x) __attribute__((diagnose_as_builtin(some_templated_function, 1))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_template_instantiation(int x) __attribute__((diagnose_as_builtin(some_templated_function, 1))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void overloaded_function(unsigned);
+
+void overloaded_function(int);
+

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: alexfh, whisperity.
aaron.ballman added a comment.

In D112646#3132141 , @avogelsgesang 
wrote:

> @xazax.hun / @whisperity could you review this change for me, please :) 
> Or, alternatively: can you direct me to somebody who could review?

I've added a few more folks to the reviewer list just in case they're able to 
get to this before I am. FWIW, I was initially surprised that this is not in 
`modernize-` but the explanation about why the author wants it in 
`readability-` was compelling enough for me to think it's fine there. I haven't 
had the chance to give this a thorough review yet, but I like the general 
direction and appreciate the new check!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

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

[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

[Fabian Wolff via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:20-23
+static const char ContainerExprName[] = "container-expr";
+static const char DerefContainerExprName[] = "deref-container-expr";
+static const char AddrofContainerExprName[] = "addrof-container-expr";
+static const char AddressOfName[] = "address-of";

whisperity wrote:
> Perhaps we could use `constexpr llvm::StringLiteral`?
Fixed.


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

https://reviews.llvm.org/D113863

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

[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

[Fabian Wolff via Phabricator via cfe-commits]

fwolff updated this revision to Diff 387384.

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

https://reviews.llvm.org/D113863

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -109,3 +109,38 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
   // CHECK-FIXES: {{^  }}return v.data();{{$}}
 }
+
+template 
+struct container_without_data {
+  using size_type = size_t;
+  T [](size_type);
+  const T [](size_type) const;
+};
+
+template 
+const T *n(const container_without_data ) {
+  // c has no "data" member function, so there should not be a warning here:
+  return [0];
+}
+
+const int *o(const std::vector>> , const size_t idx1, const size_t idx2) {
+  return [idx1][idx2][0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return v[idx1][idx2].data();{{$}}
+}
+
+std::vector (std::vector , std::vector ) {
+  return v;
+}
+
+int *p(std::vector , std::vector ) {
+  return (*, v2)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return select(*, v2).data();{{$}}
+}
+
+int *q(std::vector ***v) {
+  return &(***v)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return (**v)->data();{{$}}
+}
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -16,6 +16,12 @@
 namespace clang {
 namespace tidy {
 namespace readability {
+
+constexpr llvm::StringLiteral ContainerExprName = "container-expr";
+constexpr llvm::StringLiteral DerefContainerExprName = "deref-container-expr";
+constexpr llvm::StringLiteral AddrofContainerExprName = "addrof-container-expr";
+constexpr llvm::StringLiteral AddressOfName = "address-of";
+
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context) {}
@@ -38,69 +44,65 @@
   const auto Container =
   qualType(anyOf(NonTemplateContainerType, TemplateContainerType));
 
+  const auto ContainerExpr = anyOf(
+  unaryOperator(
+  hasOperatorName("*"),
+  hasUnaryOperand(
+  expr(hasType(pointsTo(Container))).bind(DerefContainerExprName)))
+  .bind(ContainerExprName),
+  unaryOperator(hasOperatorName("&"),
+hasUnaryOperand(expr(anyOf(hasType(Container),
+   hasType(references(Container
+.bind(AddrofContainerExprName)))
+  .bind(ContainerExprName),
+  expr(anyOf(hasType(Container), hasType(pointsTo(Container)),
+ hasType(references(Container
+  .bind(ContainerExprName));
+
+  const auto Zero = integerLiteral(equals(0));
+
+  const auto SubscriptOperator = callee(cxxMethodDecl(hasName("operator[]")));
+
   Finder->addMatcher(
   unaryOperator(
   unless(isExpansionInSystemHeader()), hasOperatorName("&"),
-  hasUnaryOperand(anyOf(
-  ignoringParenImpCasts(
-  cxxOperatorCallExpr(
-  callee(cxxMethodDecl(hasName("operator[]"))
- .bind("operator[]")),
-  argumentCountIs(2),
-  hasArgument(
-  0,
-  anyOf(ignoringParenImpCasts(
-declRefExpr(
-to(varDecl(anyOf(
-hasType(Container),
-hasType(references(Container))
-.bind("var")),
-ignoringParenImpCasts(hasDescendant(
-declRefExpr(
-  

[PATCH] D112648: [clang-tidy] Improve fix-its for `modernize-pass-by-value` check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: aaron.ballman, whisperity, hokein.
aaron.ballman added a comment.
Herald added a subscriber: rnkovacs.

In D112648#3132157 , @avogelsgesang 
wrote:

> @ymandel,  @whisperity, @aaron.ballman could one of you review this/point me 
> in the direction of a good reviewer for this change?
> (Sorry for the spam - I am new to the LLVM project, and I guess I still have 
> to learn how to navigate Phabricator/find the correct reviewers for my 
> changes)

No worries; this is not spam, it was super handy for raising awareness!

I've added a few more reviewers who can hopefully help. I'll try to take a look 
once I get the chance, but I'm pretty swamped currently with C standards 
meetings this week. Also, the LLVM dev conference is this week, so reviewers 
may be hard to come by. Thank you for your patience though!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112648

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

[PATCH] D113707: [clang] Make -masm=intel affect inline asm style

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Fairly big update. I'd like to punt on the pragma for now since this became a 
lot more involved than expected already (see dependent patches; several of them 
have already landed).


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

https://reviews.llvm.org/D113707

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

[PATCH] D113585: [clang-tidy] Fix false positive in `bugprone-throw-keyword-missing` check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D113585#3132793 , @fwolff wrote:

> Thanks for reviewing this! Can you also commit it for me?

Sure can! What name and email address would you like used for patch 
attribution? I'm in C standards meetings all this week so if someone else is 
able to land this before I get to it, I will not be hurt. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113585

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

[PATCH] D113585: [clang-tidy] Fix false positive in `bugprone-throw-keyword-missing` check

[Fabian Wolff via Phabricator via cfe-commits]

fwolff added a comment.

Thanks for reviewing this! Can you also commit it for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113585

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

[PATCH] D113585: [clang-tidy] Fix false positive in `bugprone-throw-keyword-missing` check

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, that is certainly not a place where one would put `throw`. :-D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113585

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

[PATCH] D113707: [clang] Make -masm=intel affect inline asm style

[Nico Weber via Phabricator via cfe-commits]

thakis updated this revision to Diff 387381.
thakis added a comment.

- rebase on top of recent (partially still pending) llvm patches
- use `{...|...}` in intrinsic headers


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

https://reviews.llvm.org/D113707

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Headers/immintrin.h
  clang/lib/Headers/intrin.h
  clang/lib/Headers/x86gprintrin.h
  clang/test/CodeGen/inline-asm-intel.c
  clang/test/CodeGen/inline-asm-mixed-style.c
  clang/test/Driver/masm.c

Index: clang/test/Driver/masm.c
===
--- clang/test/Driver/masm.c
+++ clang/test/Driver/masm.c
@@ -6,9 +6,12 @@
 
 int f() {
 // CHECK-INTEL: -x86-asm-syntax=intel
+// CHECK-INTEL: -inline-asm=intel
 // CHECK-ATT: -x86-asm-syntax=att
+// CHECK-ATT: -inline-asm=att
 // CHECK-SOMEREQUIRED: error: unsupported argument 'somerequired' to option 'masm='
 // CHECK-ARM: warning: argument unused during compilation: '-masm=intel'
 // CHECK-CL: -x86-asm-syntax=intel
+// CHECK-CL-NOT: -inline-asm=intel
   return 0;
 }
Index: clang/test/CodeGen/inline-asm-mixed-style.c
===
--- clang/test/CodeGen/inline-asm-mixed-style.c
+++ clang/test/CodeGen/inline-asm-mixed-style.c
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -triple i386-unknown-unknown -fasm-blocks -O0 -emit-llvm -S %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fasm-blocks -O0 -emit-llvm -S %s -o - | FileCheck %s
 // REQUIRES: x86-registered-target
 
+#include 
+
 void f() {
   __asm mov eax, ebx
   __asm mov ebx, ecx
Index: clang/test/CodeGen/inline-asm-intel.c
===
--- /dev/null
+++ clang/test/CodeGen/inline-asm-intel.c
@@ -0,0 +1,82 @@
+// REQUIRES: x86-registered-target
+
+/// Accept intel inline asm but write it out as att:
+// RUN: %clang_cc1 -Werror -target-feature +hreset -target-feature +pconfig -target-feature +sgx -ffreestanding -triple i386-unknown-unknown -mllvm -x86-asm-syntax=att -inline-asm=intel -O0 -S %s -o - | FileCheck --check-prefix=ATT %s
+// RUN: %clang_cc1 -Werror -target-feature +hreset -target-feature +pconfig -target-feature +sgx -ffreestanding -triple x86_64-unknown-unknown -mllvm -x86-asm-syntax=att -inline-asm=intel -O0 -S %s -o - | FileCheck --check-prefix=ATT %s
+
+/// Accept intel inline asm and write it out as intel:
+// RUN: %clang_cc1 -Werror -target-feature +hreset -target-feature +pconfig -target-feature +sgx -ffreestanding -triple i386-unknown-unknown -mllvm -x86-asm-syntax=intel -inline-asm=intel -O0 -S %s -o - | FileCheck  --check-prefix=INTEL %s
+// RUN: %clang_cc1 -Werror -target-feature +hreset -target-feature +pconfig -target-feature +sgx -ffreestanding -triple x86_64-unknown-unknown -mllvm -x86-asm-syntax=intel -inline-asm=intel -O0 -S %s -o - | FileCheck  --check-prefix=INTEL %s
+
+// RUN: %clang_cc1 -Werror -target-feature +hreset -target-feature +pconfig -target-feature +sgx -ffreestanding -triple i386-pc-win32 -mllvm -x86-asm-syntax=intel -inline-asm=intel -O0 -S %s -o - -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 | FileCheck  --check-prefix=INTEL %s
+// RUN: %clang_cc1 -Werror -target-feature +hreset -target-feature +pconfig -target-feature +sgx -ffreestanding -triple x86_64-pc-win32 -mllvm -x86-asm-syntax=intel -inline-asm=intel -O0 -S %s -o - -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 | FileCheck  --check-prefix=INTEL %s
+
+// Test that intrinsics headers still work with -masm=intel.
+#ifdef _MSC_VER
+#include 
+#else
+#include 
+#endif
+
+void f() {
+  // Intrinsic headers contain macros and inline functions.
+  // Inline assembly in both are checked only when they are
+  // referenced, so reference a few intrinsics here.
+  __SSC_MARK(4);
+  int a;
+  _hreset(a);
+  _pconfig_u32(0, (void*)0);
+
+  _encls_u32(0, (void*)0);
+  _enclu_u32(0, (void*)0);
+  _enclv_u32(0, (void*)0);
+#ifdef _MSC_VER
+  __movsb((void*)0, (void*)0, 0);
+  __movsd((void*)0, (void*)0, 0);
+  __movsw((void*)0, (void*)0, 0);
+  __stosb((void*)0, 0, 0);
+  __stosd((void*)0, 0, 0);
+  __stosw((void*)0, 0, 0);
+#ifdef __x86_64__
+  __movsq((void*)0, (void*)0, 0);
+  __stosq((void*)0, 0, 0);
+#endif
+  __cpuid((void*)0, 0);
+  __cpuidex((void*)0, 0, 0);
+  __halt();
+  __nop();
+  __readmsr(0);
+  __readcr3();
+  __writecr3(0);
+
+  _InterlockedExchange_HLEAcquire((void*)0, 0);
+  _InterlockedExchange_HLERelease((void*)0, 0);
+  _InterlockedCompareExchange_HLEAcquire((void*)0, 0, 0);
+  _InterlockedCompareExchange_HLERelease((void*)0, 0, 0);
+#ifdef __x86_64__
+  _InterlockedExchange64_HLEAcquire((void*)0, 0);
+  _InterlockedExchange64_HLERelease((void*)0, 0);
+ 

[PATCH] D113804: [clang-tidy] Fix behavior of `modernize-use-using` with nested structs/unions

[Fabian Wolff via Phabricator via cfe-commits]

fwolff updated this revision to Diff 387377.
fwolff added a comment.

Thanks for your suggestions @whisperity! I have fixed both of them now.


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

https://reviews.llvm.org/D113804

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -302,3 +302,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
   // CHECK-FIXES: using b = InjectedClassNameWithUnnamedArgument;
 };
+
+typedef struct { int a; union { int b; }; } PR50990;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PR50990 = struct { int a; union { int b; }; };
+
+typedef struct { struct { int a; struct { struct { int b; } c; int d; } e; } f; int g; } PR50990_nested;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PR50990_nested = struct { struct { int a; struct { struct { int b; } c; int d; } e; } f; int g; };
+
+typedef struct { struct { int a; } b; union { int c; float d; struct { int e; }; }; struct { double f; } g; } PR50990_siblings;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PR50990_siblings = struct { struct { int a; } b; union { int c; float d; struct { int e; }; }; struct { double f; } g; };
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
@@ -23,7 +23,8 @@
 
   const bool IgnoreMacros;
   SourceLocation LastReplacementEnd;
-  SourceRange LastTagDeclRange;
+  llvm::DenseMap LastTagDeclRanges;
+
   std::string FirstTypedefType;
   std::string FirstTypedefName;
 
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -25,19 +25,42 @@
 }
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
+  Finder->addMatcher(typedefDecl(unless(isInstantiated()),
+ hasParent(decl().bind("parent-decl")))
+ .bind("typedef"),
  this);
-  // This matcher used to find tag declarations in source code within typedefs.
-  // They appear in the AST just *prior* to the typedefs.
-  Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);
+
+  // This matcher is used to find tag declarations in source code within
+  // typedefs. They appear in the AST just *prior* to the typedefs.
+  Finder->addMatcher(
+  tagDecl(
+  anyOf(allOf(unless(anyOf(isImplicit(),
+   classTemplateSpecializationDecl())),
+  hasParent(decl().bind("parent-decl"))),
+// We want the parent of the ClassTemplateDecl, not the parent
+// of the specialization.
+classTemplateSpecializationDecl(hasAncestor(
+classTemplateDecl(hasParent(decl().bind("parent-decl")))
+  .bind("tagdecl"),
+  this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
+  const auto *ParentDecl = Result.Nodes.getNodeAs("parent-decl");
+  if (!ParentDecl) {
+return;
+  }
+
   // Match CXXRecordDecl only to store the range of the last non-implicit full
   // declaration, to later check whether it's within the typdef itself.
   const auto *MatchedTagDecl = Result.Nodes.getNodeAs("tagdecl");
   if (MatchedTagDecl) {
-LastTagDeclRange = MatchedTagDecl->getSourceRange();
+// It is not sufficient to just track the last TagDecl that we've seen,
+// because if one struct or union is nested inside another, the last TagDecl
+// before the typedef will be the nested one (PR#50990). Therefore, we also
+// keep track of the parent declaration, so that we can look up the last
+// TagDecl that is a sibling of the typedef in the AST.
+LastTagDeclRanges[ParentDecl] = MatchedTagDecl->getSourceRange();
 return;
   }
 
@@ -102,12 +125,14 @@
   auto Diag = diag(ReplaceRange.getBegin(), UseUsingWarning);
 
   // If typedef contains a full tag declaration, extract its full text.
-  if (LastTagDeclRange.isValid() &&
-  ReplaceRange.fullyContains(LastTagDeclRange)) {
+  auto LastTagDeclRange = 

[PATCH] D113743: [RFC][clang][DebugInfo] Allow function-local statics and types to be scoped within a lexical block

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

This looks reasonable to me. Thanks!




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:234
+  if (!LexicalBlockStack.empty())
+LexicalBlockMap[] = LexicalBlockStack.back();
+}

maybe use `LexicalBlockMap.insert({, LexicalBlockStack.back()});` to make it 
clear that we are not going to overwrite an entry?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113743

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

[clang] 95edd7f - Making the code compliant to the documentation about Floating Point

[Zahira Ammarguellat via cfe-commits]

Author: Zahira Ammarguellat
Date: 2021-11-15T15:30:10-05:00
New Revision: 95edd7f53e77415ca30abefdcdc5ce723c292ebd

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

LOG: Making the code compliant to the documentation about Floating Point
support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT
(FMA is enabled).

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 00582b6898629..204ccdfa58e51 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -205,11 +205,16 @@ Arm and AArch64 Support in Clang
 
 Floating Point Support in Clang
 ---
-- The -ffp-model=precise now implies -ffp-contract=on rather than
-  -ffp-contract=fast, and the documentation of these features has been
-  clarified. Previously, the documentation claimed that -ffp-model=precise was
-  the default, but this was incorrect because the precise model implied
-  -ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
+- The default setting of FP contraction (FMA) is now -ffp-contract=on (for
+  languages other than CUDA/HIP) even when optimization is off. Previously,
+  the default behavior was equivalent to -ffp-contract=off (-ffp-contract
+  was not set).
+  Related to this, the switch -ffp-model=precise now implies -ffp-contract=on
+  rather than -ffp-contract=fast, and the documentation of these features has
+  been clarified. Previously, the documentation claimed that -ffp-model=precise
+  was the default, but this was incorrect because the precise model implied
+  -ffp-contract=fast, wheras the (now corrected) default behavior is
+  -ffp-contract=on.
   -ffp-model=precise is now exactly the default mode of the compiler.
 
 Internal API Changes



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

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

[Warren Ristow via Phabricator via cfe-commits]

wristow added a comment.

Thanks @zahiraam !  The updated ReleaseNote LGTM.


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

https://reviews.llvm.org/D107994

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

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 387337.

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

https://reviews.llvm.org/D107994

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -205,11 +205,16 @@
 
 Floating Point Support in Clang
 ---
-- The -ffp-model=precise now implies -ffp-contract=on rather than
-  -ffp-contract=fast, and the documentation of these features has been
-  clarified. Previously, the documentation claimed that -ffp-model=precise was
-  the default, but this was incorrect because the precise model implied
-  -ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
+- The default setting of FP contraction (FMA) is now -ffp-contract=on (for
+  languages other than CUDA/HIP) even when optimization is off. Previously,
+  the default behavior was equivalent to -ffp-contract=off (-ffp-contract
+  was not set).
+  Related to this, the switch -ffp-model=precise now implies -ffp-contract=on
+  rather than -ffp-contract=fast, and the documentation of these features has
+  been clarified. Previously, the documentation claimed that -ffp-model=precise
+  was the default, but this was incorrect because the precise model implied
+  -ffp-contract=fast, wheras the (now corrected) default behavior is
+  -ffp-contract=on.
   -ffp-model=precise is now exactly the default mode of the compiler.
 
 Internal API Changes


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -205,11 +205,16 @@
 
 Floating Point Support in Clang
 ---
-- The -ffp-model=precise now implies -ffp-contract=on rather than
-  -ffp-contract=fast, and the documentation of these features has been
-  clarified. Previously, the documentation claimed that -ffp-model=precise was
-  the default, but this was incorrect because the precise model implied
-  -ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
+- The default setting of FP contraction (FMA) is now -ffp-contract=on (for
+  languages other than CUDA/HIP) even when optimization is off. Previously,
+  the default behavior was equivalent to -ffp-contract=off (-ffp-contract
+  was not set).
+  Related to this, the switch -ffp-model=precise now implies -ffp-contract=on
+  rather than -ffp-contract=fast, and the documentation of these features has
+  been clarified. Previously, the documentation claimed that -ffp-model=precise
+  was the default, but this was incorrect because the precise model implied
+  -ffp-contract=fast, wheras the (now corrected) default behavior is
+  -ffp-contract=on.
   -ffp-model=precise is now exactly the default mode of the compiler.
 
 Internal API Changes
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104830: AST: Create __va_list in the std namespace even in C.

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

This also leaks out to users via __builtin_dump_struct 
(https://godbolt.org/z/vx3rjdPdq), and of course the DWARF having the namespace 
in it will result in users debugging plain C seeing C++ namespaces.

ASTDiagnostic's Desugar happens to special-case va_list so it doesn't get 
desugared from the result of the typedef, and thus the struct std::__va_list 
doesn't leak out that way.

I also wouldn't be surprised if some consumers of Clang's AST get confused by a 
namespace appearing for C output; e.g. I could imagine IDEs using clangd 
displaying very confusing information to users writing C code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104830

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

[PATCH] D113925: [HIP] Add HIP scope atomic operations

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 created this revision.
gandhi21299 added reviewers: yaxunl, b-sumner, t-tye, tra, rjmccall.
Herald added subscribers: dexonsmith, Anastasia.
gandhi21299 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add an AtomicScopeModel for HIP and support for OpenCL builtins
that are missing in HIP.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113925

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/SyncScope.h
  clang/lib/AST/Expr.cpp
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCUDA/atomic-ops.cu

Index: clang/test/CodeGenCUDA/atomic-ops.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/atomic-ops.cu
@@ -0,0 +1,302 @@
+// RUN: %clang_cc1 -x hip -std=c++11 -triple amdgcn -fcuda-is-device -emit-llvm %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+
+// CHECK-LABEL: @_Z24atomic32_op_singlethreadPiii
+// CHECK: cmpxchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw xchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw add i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw and i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw or i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw xor i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw min i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw max i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+__device__ int atomic32_op_singlethread(int *ptr, int val, int desired) {
+  bool flag = __hip_atomic_compare_exchange_strong(ptr, , desired, __ATOMIC_RELAXED, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_exchange(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_add(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_and(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_or(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_xor(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_min(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_max(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  return flag ? val : desired;
+}
+
+// CHECK-LABEL: @_Z25atomicu32_op_singlethreadPjjj
+// CHECK: atomicrmw umin i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+// CHECK: atomicrmw umax i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("singlethread-one-as")
+__device__ unsigned int atomicu32_op_singlethread(unsigned int *ptr, unsigned int val, unsigned int desired) {
+  val = __hip_atomic_fetch_min(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  val = __hip_atomic_fetch_max(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_SINGLETHREAD);
+  return val;
+}
+
+// CHECK-LABEL: @_Z21atomic32_op_wavefrontPiii
+// CHECK: cmpxchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw xchg i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw add i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw and i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw or i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw xor i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw min i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+// CHECK: atomicrmw max i32* {{%[0-9]+}}, i32 {{%[0-9]+}} syncscope("wavefront-one-as")
+__device__ int atomic32_op_wavefront(int *ptr, int val, int desired) {
+  bool flag = __hip_atomic_compare_exchange_strong(ptr, , desired, __ATOMIC_RELAXED, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_exchange(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_add(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_and(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_or(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_xor(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_min(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  val = __hip_atomic_fetch_max(ptr, val, __ATOMIC_RELAXED, __HIP_MEMORY_SCOPE_WAVEFRONT);
+  return flag ? val : desired;
+}
+
+// 

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

In D108479#3129577 , @rjmccall wrote:

> In D108479#3129571 , @jrtc27 wrote:
>
>> For CHERI there's the added complication that descriptors and trampolines 
>> can exist for security reasons when crossing security domains, and you 
>> absolutely should not let one compartment get pointers to the entry point of 
>> another compartment's function. You can hand it out if sealed or the 
>> permissions are cleared, as then you can't really do anything with it other 
>> than look at the integer address, but that seems a bit odd.
>
> That would be consistent with getting an unsigned pointer under pointer 
> authentication or an address with the THUMB bit potentially stripped: it's 
> just a raw address that you can't safely call.  In any case, I suspect it 
> would be fine for us to say that we just don't support this builtin when the 
> target is using weird function pointers unless we have some way to bypass the 
> special treatment in LLVM.
>
> I agree that "symbol" address is probably the wrong name.  Maybe 
> `__builtin_function_start` or something like that?  But before we go much 
> further on this, we should get confirmation from Peter that we're targeting 
> the right design.

Having this be defined to return the function body address (and diagnose at 
compile time if that's not supported) seem like the right design to me, and  
`__builtin_function_start` sounds like a good name.

Here's another example of a potential use case that came up in the course of 
the PAuth ABI work: testing whether a stack trace contains a particular 
function:
https://cs.android.com/android/platform/superproject/+/master:bionic/tests/execinfo_test.cpp;l=94

I think that no matter whether it's CFI, PAuth, CHERI or anything else, this 
test would want the address of the real function body. If the test is compiled 
on a platform where taking the real function body address is unsupported, it 
would fail at runtime if we just let this be compiled as a no-op, so we should 
diagnose the issue at compile time where possible.




Comment at: clang/test/SemaCXX/builtins.cpp:44
+void a(void) {}
+static_assert(__builtin_addressof_nocfi(a) == a, "");
+

I don't think this should always evaluate to true, should it? Maybe we should 
forbid these types of comparisons in integral constant expressions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

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

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D64454#3130696 , @whisperity wrote:

> In D64454#3124312 , @aaron.ballman 
> wrote:
>
>> Yeah, I was worried this would get out of sycn and it really did not take 
>> long for that to happen in practice. I think there's utility in listing the 
>> checks in clang-tidy's documentation instead of pointing users to the SA 
>> documentation page. Users should have one website to go to where they can 
>> see what clang-tidy checks are available to them.
>
> Does this still apply given that the quality and formatting match of the 
> documentation website increased over the past one or two releases? The old 
> website  if I see correctly no longer lists 
> the checkers (and the previous format was also increasingly outdated and bad 
> experience...), and we have the RST-based new website 
> .

I think that clang-tidy users should be able to view the clang-tidy docs and 
see what checks are available to them (including static analyzer checks). 
Similarly, I think CSA users should be able to view the CSA docs and see what 
checks are available to them (including the clang-tidy checks that I think 
we're now exposing or were exploring exposing).

> Come to think of it, maybe some RST hacks could automate "pulling in" the 
> list from the (new) CSA website to the Tidy list, as long as maybe they are 
> in a separate category (heading) in the Tidy list?
> Although I'm unsure how much being `/tools/clang/` and 
> `/tools/clang/tools/extra/` mess this thing up.
> In case of Python, for example, there is a well-understood way of registering 
> the documentation site of third-party packages, and //your// documentation 
> generator can automatically generate the cross-reference that links to the 
> other, from your doc's point-of-view, external website. But that involves a 
> considerable amount of "magic".
> As far as I know, Sphinx is written in Python, so one could write Python code 
> that runs //during// the documentation generation (at least the RST part, I'm 
> not sure about the `man` or `infopages` or `groff` stuff!) that can do 
> "magic".

That's a neat idea! So long as it doesn't become a maintenance burden for some 
reason, I think it could possibly be a viable path forward.

> 
>
> What we //COULD// do, however, is get rid of the individual checker 
> documentation files (which all just contain machine generated redirects, at 
> least according to this diff...) and have ALL the cross-referencing links 
> (hand-written, autogenerated by our integration, or autogenerated by RST 
> magic) point to not a page living inside Tidy's scope that just does a 
> redirect, but to the appropriate heading in the (new) CSA doc suite?
>
> (Then again, irrespective of what magic we will use, the non-trivial grouping 
> structure on the new CSA docs site can become a problem...)

If I understand correctly, that would then get rid of the concrete list of 
checks from `list.rst`, wouldn't it? Otherwise, we're still going to get out of 
date trivially as new CSA checks are added but people forget to update 
`list.rst`. (Or did I misunderstand?)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64454

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

[PATCH] D113575: Add `isInitCapture` and `forEachLambdaCapture` matchers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113575

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

[PATCH] D113917: Add infrastructure to support matcher names.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

For a little more context: we're working to improve the debuggability of 
matchers. The first step in that direction seems to be a change of this sort 
that provides some (dynamic) introspection so that a tool, library, etc. can 
see some basic information about the matcher.

note that this leaves a fair amount of room for different design decisions. Of 
all those that we've considered, the most relevant seems to be using an enum 
for the result of `getName` (that is, the introspection method). We chose a 
string so as to easily accomodate user-defined matchers, both those defined 
with the macros and otherwise. However, we are open to different proposals.

In addition, James is preparing a follow up patch with a new matcher that 
exploits this functionality to create detailed matcher logging, which is really 
helpful for debugging matchers during development.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113917

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

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:61-64
-  unsigned errorsIgnored() const {
+  int errorsIgnored() const {
 return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +
ErrorsIgnoredNonUserCode + ErrorsIgnoredLineFilter;
   }

carlosgalvezp wrote:
> carlosgalvezp wrote:
> > aaron.ballman wrote:
> > > carlosgalvezp wrote:
> > > > aaron.ballman wrote:
> > > > > The type changes overflow here from well-defined behavior to 
> > > > > undefined behavior. I don't think that's a serious concern (we have 
> > > > > error limits already), but in general, I don't think we should be 
> > > > > changing types like this without stronger rationale. This particular 
> > > > > bit feels like churn without benefit -- what do we gain by 
> > > > > introducing the possibility for UB here? (I'm not strongly opposed, 
> > > > > but I get worried when well-defined code turns into potentially 
> > > > > undefined code in the name of an NFC cleanup.)
> > > > Do we have coding guidelines for when to use signed vs unsigned? I have 
> > > > quite ugly past experiences using unsigned types for arithmetic and as 
> > > > "counters" so these kinds of things really catch my eye quickly. This 
> > > > goes in line with e.g. the [[ 
> > > > https://google.github.io/styleguide/cppguide.html#Integer_Types | 
> > > > Google style guide ]].
> > > > 
> > > > Yes, you are correct that signed int overflow is UB, but would that 
> > > > then be an argument for banning signed ints in the codebase at all? IMO 
> > > > what's more important is to know the domain in which this is being 
> > > > applied. Do we expect people to have more than 2^31 linter warnings? If 
> > > > we do, would 2^32 really solve the problem, or just delay it? Maybe the 
> > > > proper solution is to go to 64-bit ints if that's an expected use case.
> > > > 
> > > > Anyway if this is a concern I'm happy to revert and take the discussion 
> > > > outside the patch. We used to be over-defensive in this topic as well 
> > > > (UB is not to be taken lightly) but realized that perhaps the problem 
> > > > lies somewhere else.
> > > > Do we have coding guidelines for when to use signed vs unsigned?
> > > 
> > > We don't, and I don't think we could even if we wanted to. There are 
> > > times when unsigned is correct and there are times when signed is 
> > > correct, and no blanket rule covers all the cases.
> > > 
> > > > Yes, you are correct that signed int overflow is UB, but would that 
> > > > then be an argument for banning signed ints in the codebase at all? 
> > > 
> > > No. My point about the change to UB wasn't "I think this is dangerous" -- 
> > > as I mentioned, we have limits on the number of errors that can be 
> > > emitted, so I don't believe we can hit the UB in practice. It's more that 
> > > changing types is rarely an NFC change without further validation and it 
> > > wasn't clear if this validation had happened. Changing types is a 
> > > nontrivial (and rarely NFC) change in C++.
> > > 
> > > > Anyway if this is a concern I'm happy to revert and take the discussion 
> > > > outside the patch. We used to be over-defensive in this topic as well 
> > > > (UB is not to be taken lightly) but realized that perhaps the problem 
> > > > lies somewhere else.
> > > 
> > > Personally, I'm not super motivated to see a change here, but I'm also 
> > > not opposed to it. It mostly just feels like churn. If we're solving a 
> > > problem, I'm happy to see the changes, but we should call out what 
> > > problem we're solving in the patch summary. But this seems more a style 
> > > choice and I certainly wouldn't want to see these changes used as a 
> > > precedent to go switch more unsigned ints to be signed ints in the name 
> > > of style.
> > Thanks, I see how this change is not as NFC as I thought would be. 
> > Reverting!
> > But this seems more a style choice
> 
> PS: IMHO signed vs unsigned is not just a style choice. Using unsigned types 
> for the wrong purpose (e.g. doing arithmetic) can lead to nasty logical bugs, 
> especially when implicit conversions happen under the hood:
> 
> https://godbolt.org/z/5qox7b1nW
> 
> The above code is obvious, but in a real-world case you might not keep in the 
> back of your head that you are operating with an "unsigned" variable in the 
> middle of a 100-lines long function.
> 
> In the end, it's all about where you place the "singularity". With unsigned 
> types, underflow happens around 0. With signed types, UB happens at 
> min()/max(). In real-world applications, we usually code much more often 
> around 0 than around min/max.
> 
> That's my 2 cents, don't mean to change/promote any guidelines. If you think 
> this is a topic of interest I can bring it to the mailing list, otherwise 
> I'll just drop it :)
> That's my 2 

[PATCH] D112648: [clang-tidy] Improve fix-its for `modernize-pass-by-value` check

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added subscribers: ymandel, whisperity, aaron.ballman.
avogelsgesang added a comment.

@ymandel,  @whisperity, @aaron.ballman could one of you review this/point me in 
the direction of a good reviewer for this change?
(Sorry for the spam - I am new to the LLVM project, and I guess I still have to 
learn how to navigate Phabricator/find the correct reviewers for my changes)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112648

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

[PATCH] D113917: Add infrastructure to support matcher names.

[James King via Phabricator via cfe-commits]

jcking1034 added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1021
 
+  std::string getName() const override {
+return "HasOverloadedOperatorNameMatcher";

Here and below, we have the option to use the name of the matcher (for this, we 
could use "hasOverloadedOperatorName" instead). However, some of these classes 
are used in multiple matchers. For example, `HasOverloadedOperatorNameMatcher` 
is used in both `hasOverloadedOperatorName` and `hasAnyOverloadedOperatorName`, 
which I'm a bit concerned about.



Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1409
   return BindableMatcher(
-  makeAllOfComposite(InnerMatchers).template dynCastTo());
+  makeAllOfComposite(InnerMatchers).template dynCastTo(),
+  makeMatcherNameFromType());

@ymandel suggested an alternate implementation where we instead pass the 
matcher name to `makeAllOfComposite`, which then passes the name to 
`constructVariadic`, to avoid making changes to `BindableMatcher`. I may look 
into this again, but my previous attempts to try this approach seemed messy due 
to the fact that these functions are used in a few different places, and 
because `makeAllOfComposite` handles cases with 0 or 1 inner matchers without 
constructing a variadic matcher.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113917

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

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added a comment.

@xazax.hun / @whisperity could you review this change for me, please :) 
Or, alternatively: can you direct me to somebody who could review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

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

[PATCH] D113917: Add infrastructure to support matcher names.

[James King via Phabricator via cfe-commits]

jcking1034 created this revision.
jcking1034 added reviewers: ymandel, tdl-g, aaron.ballman.
jcking1034 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This contains changes to the AST Matcher infrastructure in order to provide
contextual information about each matcher. This information can be useful for
tasks such as performing introspection on matchers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113917

Files:
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/ASTMatchers/ASTMatchersMacros.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp

Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -115,15 +115,19 @@
 template 
 class VariadicMatcher : public DynMatcherInterface {
 public:
-  VariadicMatcher(std::vector InnerMatchers)
-  : InnerMatchers(std::move(InnerMatchers)) {}
+  VariadicMatcher(std::string MatcherName,
+  std::vector InnerMatchers)
+  : MatcherName(MatcherName), InnerMatchers(std::move(InnerMatchers)) {}
 
   bool dynMatches(const DynTypedNode , ASTMatchFinder *Finder,
   BoundNodesTreeBuilder *Builder) const override {
 return Func(DynNode, Finder, Builder, InnerMatchers);
   }
 
+  std::string getName() const override { return MatcherName; }
+
 private:
+  const std::string MatcherName;
   std::vector InnerMatchers;
 };
 
@@ -144,11 +148,39 @@
 return InnerMatcher->TraversalKind();
   }
 
+  std::string getName() const override { return InnerMatcher->getName(); }
+
 private:
   const std::string ID;
   const IntrusiveRefCntPtr InnerMatcher;
 };
 
+/// A matcher that specifies a particular name.
+///
+/// The name provided to the constructor overrides any name that may be
+/// specified by the `InnerMatcher`.
+class NameMatcherImpl : public DynMatcherInterface {
+public:
+  NameMatcherImpl(std::string _MatcherName,
+  IntrusiveRefCntPtr InnerMatcher)
+  : MatcherName(_MatcherName), InnerMatcher(std::move(InnerMatcher)) {}
+
+  bool dynMatches(const DynTypedNode , ASTMatchFinder *Finder,
+  BoundNodesTreeBuilder *Builder) const override {
+return InnerMatcher->dynMatches(DynNode, Finder, Builder);
+  }
+
+  std::string getName() const override { return MatcherName; }
+
+  llvm::Optional TraversalKind() const override {
+return InnerMatcher->TraversalKind();
+  }
+
+private:
+  const std::string MatcherName;
+  const IntrusiveRefCntPtr InnerMatcher;
+};
+
 /// A matcher that always returns true.
 class TrueMatcherImpl : public DynMatcherInterface {
 public:
@@ -158,6 +190,8 @@
   BoundNodesTreeBuilder *) const override {
 return true;
   }
+
+  std::string getName() const override { return "TrueMatcher"; }
 };
 
 /// A matcher that specifies a particular \c TraversalKind.
@@ -180,6 +214,8 @@
 return TK;
   }
 
+  std::string getName() const override { return InnerMatcher->getName(); }
+
 private:
   clang::TraversalKind TK;
   IntrusiveRefCntPtr InnerMatcher;
@@ -219,31 +255,31 @@
   RestrictKind =
   ASTNodeKind::getMostDerivedType(RestrictKind, IM.RestrictKind);
 }
-return DynTypedMatcher(
-SupportedKind, RestrictKind,
-new VariadicMatcher(std::move(InnerMatchers)));
+return DynTypedMatcher(SupportedKind, RestrictKind,
+   new VariadicMatcher(
+   "allOf", std::move(InnerMatchers)));
 
   case VO_AnyOf:
-return DynTypedMatcher(
-SupportedKind, RestrictKind,
-new VariadicMatcher(std::move(InnerMatchers)));
+return DynTypedMatcher(SupportedKind, RestrictKind,
+   new VariadicMatcher(
+   "anyOf", std::move(InnerMatchers)));
 
   case VO_EachOf:
-return DynTypedMatcher(
-SupportedKind, RestrictKind,
-new VariadicMatcher(std::move(InnerMatchers)));
+return DynTypedMatcher(SupportedKind, RestrictKind,
+   new VariadicMatcher(
+   "eachOf", std::move(InnerMatchers)));
 
   case VO_Optionally:
 return DynTypedMatcher(SupportedKind, RestrictKind,
new VariadicMatcher(
-   std::move(InnerMatchers)));
+   "optionally", std::move(InnerMatchers)));
 
   case VO_UnaryNot:
 // FIXME: Implement the Not operator to take a single matcher instead of a
 // vector.
 return DynTypedMatcher(
 SupportedKind, RestrictKind,
-new VariadicMatcher(std::move(InnerMatchers)));
+new VariadicMatcher("not", std::move(InnerMatchers)));
   }
   llvm_unreachable("Invalid Op value.");
 }
@@ -263,6 +299,14 @@
   return Copy;
 }
 
+DynTypedMatcher

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang updated this revision to Diff 387318.
avogelsgesang added a comment.

Update docs to also mention `container.find() != container.end()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
@@ -0,0 +1,111 @@
+// RUN: %check_clang_tidy -std=c++20 %s readability-container-contains %t
+
+// Some *very* simplified versions of `map` etc.
+namespace std {
+
+template 
+struct map {
+  unsigned count(const Key ) const;
+  bool contains(const Key ) const;
+  void *find(const Key );
+  void *end();
+};
+
+template 
+struct set {
+  unsigned count(const Key ) const;
+  bool contains(const Key ) const;
+};
+
+template 
+struct unordered_set {
+  unsigned count(const Key ) const;
+  bool contains(const Key ) const;
+};
+
+template 
+struct multimap {
+  unsigned count(const Key ) const;
+  bool contains(const Key ) const;
+};
+
+} // namespace std
+
+using namespace std;
+
+// Check that we detect various common ways to check for containment
+int testDifferentCheckTypes(map ) {
+  if (MyMap.count(0))
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+// CHECK-FIXES: if (MyMap.contains(0))
+return 1;
+  bool C1 = MyMap.count(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: bool C1 = MyMap.contains(1);
+  auto C2 = static_cast(MyMap.count(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: auto C2 = static_cast(MyMap.contains(1));
+  auto C3 = MyMap.count(2) != 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: auto C3 = MyMap.contains(2);
+  auto C4 = MyMap.count(3) > 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: auto C4 = MyMap.contains(3);
+  auto C5 = MyMap.count(4) >= 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: auto C5 = MyMap.contains(4);
+  auto C6 = MyMap.find(5) != MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: auto C6 = MyMap.contains(5);
+  return C1 + C2 + C3 + C4 + C5 + C6;
+}
+
+// Check that we detect various common ways to check for non-containment
+int testNegativeChecks(map ) {
+  bool C1 = !MyMap.count(-1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: bool C1 = !MyMap.contains(-1);
+  auto C2 = MyMap.count(-2) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: auto C2 = !MyMap.contains(-2);
+  auto C3 = MyMap.count(-3) <= 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: auto C3 = !MyMap.contains(-3);
+  auto C4 = MyMap.count(-4) < 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: auto C4 = !MyMap.contains(-4);
+  auto C5 = MyMap.find(-5) == MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use `contains` instead of `count` to check for containment [readability-container-contains]
+  // CHECK-FIXES: auto C5 = !MyMap.contains(-5);
+  return C1 + C2 + C3 + C4 + C5;
+}
+
+// Check for various types
+int testDifferentTypes(map , unordered_set , set , multimap ) {
+  bool C1 = M.count(1001);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use `contains` instead of `count` to check for containment 

[PATCH] D113587: [analyzer][NFC] Separate CallDescription from CallEvent

[Balázs Benics via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0b9d3a6e53e6: [analyzer][NFC] Separate CallDescription from 
CallEvent (authored by steakhal).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113587

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
  clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/CallDescription.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
  clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
  clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
  clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
  clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
  llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Core/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Core/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Core/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Core/BUILD.gn
@@ -20,6 +20,7 @@
 "BlockCounter.cpp",
 "BugReporter.cpp",
 "BugReporterVisitors.cpp",
+"CallDescription.cpp",
 "CallEvent.cpp",
 "Checker.cpp",
 "CheckerContext.cpp",
Index: clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
===
--- clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
+++ clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
@@ -14,6 +14,7 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
Index: clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
===
--- clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
+++ clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
@@ -12,6 +12,7 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
Index: clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
===
--- clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
+++ clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
@@ -8,6 +8,7 @@
 
 #include "CheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include 

[clang] 0b9d3a6 - [analyzer][NFC] Separate CallDescription from CallEvent

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2021-11-15T19:10:46+01:00
New Revision: 0b9d3a6e53e6c6488b531f3c8c281b485ca3b14a

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

LOG: [analyzer][NFC] Separate CallDescription from CallEvent

`CallDescriptions` deserve its own translation unit.
This patch simply moves the corresponding parts.
Also includes the `CallDescription.h` where it's necessary.

Reviewed By: martong, xazax.hun, Szelethus

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

Added: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
clang/lib/StaticAnalyzer/Core/CallDescription.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/lib/StaticAnalyzer/Checkers/StringChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
clang/lib/StaticAnalyzer/Core/CMakeLists.txt
clang/lib/StaticAnalyzer/Core/CallEvent.cpp
clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
llvm/utils/gn/secondary/clang/lib/StaticAnalyzer/Core/BUILD.gn

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
new file mode 100644
index 0..9148dcaa87544
--- /dev/null
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -0,0 +1,121 @@
+//===- CallDescription.h - function/method call matching   --*- 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
+//
+//===--===//
+//
+/// \file This file defines a generic mechanism for matching for function and
+/// method calls of C, C++, and Objective-C languages. Instances of these
+/// classes are frequently used together with the CallEvent classes.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CALLDESCRIPTION_H
+#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_CALLDESCRIPTION_H
+
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include 
+
+namespace clang {
+class IdentifierInfo;
+} // namespace clang
+
+namespace clang {
+namespace ento {
+
+enum CallDescriptionFlags : int {
+  /// Describes a C standard function that is sometimes implemented as a macro
+  /// that expands to a compiler builtin with some __builtin prefix.
+  /// The builtin may as well have a few extra arguments on top of the 
requested
+  /// number of arguments.
+  CDF_MaybeBuiltin = 1 << 0,
+};
+
+/// This class represents a description of a function call using the number of
+/// arguments and the name of the function.
+class CallDescription {
+  friend class CallEvent;
+  mutable Optional II;
+  // The list of the qualified names used to identify the specified CallEvent,
+  // e.g. "{a, b}" represent the qualified names, like 

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

[Warren Ristow via Phabricator via cfe-commits]

wristow added a comment.

In D107994#3131268 , @zahiraam wrote:

> In D107994#3130494 , @wristow wrote:
>
>> The Release Note change here says:
>>
>>   Floating Point Support in Clang
>>   ---
>>   - The -ffp-model=precise now implies -ffp-contract=on rather than
>> -ffp-contract=fast, and the documentation of these features has been
>> clarified. Previously, the documentation claimed that -ffp-model=precise 
>> was
>> the default, but this was incorrect because the precise model implied
>> -ffp-contract=fast, whereas the default behavior is -ffp-contract=on.
>> -ffp-model=precise is now exactly the default mode of the compiler.
>>
>> Unless I'm missing something, there is a related change here that I think 
>> should be more overtly noted (given the discussions in this review, I 
>> //think// this additional change is understood/expected, but I'm surprised 
>> it's not pointed out explicitly -- so maybe I'm misunderstanding).
>>
>> Specifically, this commit explicitly sets `-ffp-contract=on` in the default 
>> mode (which is what the documentation said, and continues to say).  But 
>> previously, there wasn't //any// explicit setting of `-ffp-contract` by 
>> default (and I think that lack of an explicit setting, was equivalent to 
>> `-ffp-contract=off`).
>> ...
>
> @wristow Are you suggesting a change of wording in the ReleaseNotes?

Yes @zahiraam, either a modification to what was written, or an additional 
separate point.  The critical user-visible change from this commit (AFAICS) is 
that previously the default behavior was `-ffp-contract=off`, whereas now the 
default behavior is `-ffp-contract=on`.  The ReleaseNote as written implies 
that the FMA situation //was// `-ffp-contract=fast`, and it has been changed to 
`-ffp-contract=on`, and either of those modes would result in FMA being done 
for cases like:

  float foo(float a, float b, float c) {
return a * b + c;
  }

In short, the current ReleaseNote implies that FMA would be used by default, 
for cases like the above (because `-ffp-contract=on` was the default) -- but 
this isn't true.  (The current ReleaseNote also clarifies that if 
`-ffp-model=precise` were explicitly specified, then it previously would set 
`-ffp-contract=fast` (which was not what the documentation said) and now it 
sets `-ffp-contract=on` (which //is// what the documentation says) -- this 
aspect is correct.)

I think the ReleaseNote should clarify that the result of this change is that 
previously, the default behavior was equivalent to `-ffp-contract=off` (and so 
no FMA would be done by default), but this commit makes the default behavior 
`-ffp-contract=on` (so FMA is enabled by default, even at `-O0`).  The 
documentation indicates that the default is `-ffp-contract=on`, so this change 
makes the compiler behavior consistent with the documentation, with respect to 
the `-ffp-contact` switch.  It also makes it consistent with the documentation 
for the `-ffp-model` switch.

A possible new wording is below (maybe this is too verbose, so making it more 
concise is fine too, from my POV):

  Floating Point Support in Clang
  ---
  - The default setting of FP contraction (FMA) is now -ffp-contract=on (for
languages other than CUDA/HIP).  This is consistent with the documentation.
Previously, the default behavior was equivalent to -ffp-contract=off, so
the old behavior was that FMA would not be used by default for code like:
float foo(float a, float b, float c) {
  return a * b + c;
}
Whereas now, FMA will be used in the above code, even when optimization
is off.
  
Related to this, the switch -ffp-model=precise now implies -ffp-contract=on
rather than -ffp-contract=fast, and the documentation of these features has
been clarified.  Previously, the documentation claimed that
-ffp-model=precise was the default, but this was incorrect because the
precise model implied -ffp-contract=fast, whereas the (now corrected)
default behavior is -ffp-contract=on.
-ffp-model=precise is now exactly the default mode of the compiler.


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

https://reviews.llvm.org/D107994

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

[PATCH] D113422: [clang-tidy][NFC] Move CachedGlobList to GlobList.h

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 387304.
carlosgalvezp added a comment.

- Replace composition with private inheritance.
- Fix namespaces.

Public inheritance is not possible due to the base class' function being 
const-qualified; the derived class cannot be const since it updates state 
(unless we go into mutable territory, which I think goes in detriment of 
readability). What do you think?


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

https://reviews.llvm.org/D113422

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/GlobList.cpp
  clang-tools-extra/clang-tidy/GlobList.h
  clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
@@ -4,15 +4,20 @@
 namespace clang {
 namespace tidy {
 
-TEST(GlobList, Empty) {
-  GlobList Filter("");
+template  struct GlobListTest : public ::testing::Test {};
+
+using GlobListTypes = ::testing::Types;
+TYPED_TEST_SUITE(GlobListTest, GlobListTypes);
+
+TYPED_TEST(GlobListTest, Empty) {
+  TypeParam Filter("");
 
   EXPECT_TRUE(Filter.contains(""));
   EXPECT_FALSE(Filter.contains("aaa"));
 }
 
-TEST(GlobList, Nothing) {
-  GlobList Filter("-*");
+TYPED_TEST(GlobListTest, Nothing) {
+  TypeParam Filter("-*");
 
   EXPECT_FALSE(Filter.contains(""));
   EXPECT_FALSE(Filter.contains("a"));
@@ -21,8 +26,8 @@
   EXPECT_FALSE(Filter.contains("*"));
 }
 
-TEST(GlobList, Everything) {
-  GlobList Filter("*");
+TYPED_TEST(GlobListTest, Everything) {
+  TypeParam Filter("*");
 
   EXPECT_TRUE(Filter.contains(""));
   EXPECT_TRUE(Filter.contains(""));
@@ -31,8 +36,8 @@
   EXPECT_TRUE(Filter.contains("*"));
 }
 
-TEST(GlobList, OneSimplePattern) {
-  GlobList Filter("aaa");
+TYPED_TEST(GlobListTest, OneSimplePattern) {
+  TypeParam Filter("aaa");
 
   EXPECT_TRUE(Filter.contains("aaa"));
   EXPECT_FALSE(Filter.contains(""));
@@ -41,8 +46,8 @@
   EXPECT_FALSE(Filter.contains("bbb"));
 }
 
-TEST(GlobList, TwoSimplePatterns) {
-  GlobList Filter("aaa,bbb");
+TYPED_TEST(GlobListTest, TwoSimplePatterns) {
+  TypeParam Filter("aaa,bbb");
 
   EXPECT_TRUE(Filter.contains("aaa"));
   EXPECT_TRUE(Filter.contains("bbb"));
@@ -52,11 +57,11 @@
   EXPECT_FALSE(Filter.contains(""));
 }
 
-TEST(GlobList, PatternPriority) {
+TYPED_TEST(GlobListTest, PatternPriority) {
   // The last glob that matches the string decides whether that string is
   // included or excluded.
   {
-GlobList Filter("a*,-aaa");
+TypeParam Filter("a*,-aaa");
 
 EXPECT_FALSE(Filter.contains(""));
 EXPECT_TRUE(Filter.contains("a"));
@@ -65,7 +70,7 @@
 EXPECT_TRUE(Filter.contains(""));
   }
   {
-GlobList Filter("-aaa,a*");
+TypeParam Filter("-aaa,a*");
 
 EXPECT_FALSE(Filter.contains(""));
 EXPECT_TRUE(Filter.contains("a"));
@@ -75,15 +80,16 @@
   }
 }
 
-TEST(GlobList, WhitespacesAtBegin) {
-  GlobList Filter("-*,   a.b.*");
+TYPED_TEST(GlobListTest, WhitespacesAtBegin) {
+  TypeParam Filter("-*,   a.b.*");
 
   EXPECT_TRUE(Filter.contains("a.b.c"));
   EXPECT_FALSE(Filter.contains("b.c"));
 }
 
-TEST(GlobList, Complex) {
-  GlobList Filter("*,-a.*, -b.*, \r  \n  a.1.* ,-a.1.A.*,-..,-...,-..+,-*$, -*qwe* ");
+TYPED_TEST(GlobListTest, Complex) {
+  TypeParam Filter(
+  "*,-a.*, -b.*, \r  \n  a.1.* ,-a.1.A.*,-..,-...,-..+,-*$, -*qwe* ");
 
   EXPECT_TRUE(Filter.contains("aaa"));
   EXPECT_TRUE(Filter.contains("qqq"));
Index: clang-tools-extra/clang-tidy/GlobList.h
===
--- clang-tools-extra/clang-tidy/GlobList.h
+++ clang-tools-extra/clang-tidy/GlobList.h
@@ -11,6 +11,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
 
@@ -47,7 +48,22 @@
   SmallVector Items;
 };
 
-} // end namespace tidy
-} // end namespace clang
+/// A \p GlobList that caches search results, so that search is performed only
+/// once for the same query.
+class CachedGlobList : private GlobList {
+public:
+  /// \see GlobList::GlobList
+  CachedGlobList(StringRef Globs, bool KeepNegativeGlobs = true);
+
+  /// \see GlobList::contains
+  bool contains(StringRef S);
+
+private:
+  enum Tristate { None, Yes, No };
+  llvm::StringMap Cache;
+};
+
+} // namespace tidy
+} // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GLOBLIST_H
Index: clang-tools-extra/clang-tidy/GlobList.cpp
===
--- clang-tools-extra/clang-tidy/GlobList.cpp
+++ clang-tools-extra/clang-tidy/GlobList.cpp
@@ -9,8 +9,8 @@
 #include "GlobList.h"
 #include "llvm/ADT/SmallString.h"
 
-using namespace 

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:61-64
-  unsigned errorsIgnored() const {
+  int errorsIgnored() const {
 return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +
ErrorsIgnoredNonUserCode + ErrorsIgnoredLineFilter;
   }

carlosgalvezp wrote:
> aaron.ballman wrote:
> > carlosgalvezp wrote:
> > > aaron.ballman wrote:
> > > > The type changes overflow here from well-defined behavior to undefined 
> > > > behavior. I don't think that's a serious concern (we have error limits 
> > > > already), but in general, I don't think we should be changing types 
> > > > like this without stronger rationale. This particular bit feels like 
> > > > churn without benefit -- what do we gain by introducing the possibility 
> > > > for UB here? (I'm not strongly opposed, but I get worried when 
> > > > well-defined code turns into potentially undefined code in the name of 
> > > > an NFC cleanup.)
> > > Do we have coding guidelines for when to use signed vs unsigned? I have 
> > > quite ugly past experiences using unsigned types for arithmetic and as 
> > > "counters" so these kinds of things really catch my eye quickly. This 
> > > goes in line with e.g. the [[ 
> > > https://google.github.io/styleguide/cppguide.html#Integer_Types | Google 
> > > style guide ]].
> > > 
> > > Yes, you are correct that signed int overflow is UB, but would that then 
> > > be an argument for banning signed ints in the codebase at all? IMO what's 
> > > more important is to know the domain in which this is being applied. Do 
> > > we expect people to have more than 2^31 linter warnings? If we do, would 
> > > 2^32 really solve the problem, or just delay it? Maybe the proper 
> > > solution is to go to 64-bit ints if that's an expected use case.
> > > 
> > > Anyway if this is a concern I'm happy to revert and take the discussion 
> > > outside the patch. We used to be over-defensive in this topic as well (UB 
> > > is not to be taken lightly) but realized that perhaps the problem lies 
> > > somewhere else.
> > > Do we have coding guidelines for when to use signed vs unsigned?
> > 
> > We don't, and I don't think we could even if we wanted to. There are times 
> > when unsigned is correct and there are times when signed is correct, and no 
> > blanket rule covers all the cases.
> > 
> > > Yes, you are correct that signed int overflow is UB, but would that then 
> > > be an argument for banning signed ints in the codebase at all? 
> > 
> > No. My point about the change to UB wasn't "I think this is dangerous" -- 
> > as I mentioned, we have limits on the number of errors that can be emitted, 
> > so I don't believe we can hit the UB in practice. It's more that changing 
> > types is rarely an NFC change without further validation and it wasn't 
> > clear if this validation had happened. Changing types is a nontrivial (and 
> > rarely NFC) change in C++.
> > 
> > > Anyway if this is a concern I'm happy to revert and take the discussion 
> > > outside the patch. We used to be over-defensive in this topic as well (UB 
> > > is not to be taken lightly) but realized that perhaps the problem lies 
> > > somewhere else.
> > 
> > Personally, I'm not super motivated to see a change here, but I'm also not 
> > opposed to it. It mostly just feels like churn. If we're solving a problem, 
> > I'm happy to see the changes, but we should call out what problem we're 
> > solving in the patch summary. But this seems more a style choice and I 
> > certainly wouldn't want to see these changes used as a precedent to go 
> > switch more unsigned ints to be signed ints in the name of style.
> Thanks, I see how this change is not as NFC as I thought would be. Reverting!
> But this seems more a style choice

PS: IMHO signed vs unsigned is not just a style choice. Using unsigned types 
for the wrong purpose (e.g. doing arithmetic) can lead to nasty logical bugs, 
especially when implicit conversions happen under the hood:

https://godbolt.org/z/5qox7b1nW

The above code is obvious, but in a real-world case you might not keep in the 
back of your head that you are operating with an "unsigned" variable in the 
middle of a 100-lines long function.

In the end, it's all about where you place the "singularity". With unsigned 
types, underflow happens around 0. With signed types, UB happens at 
min()/max(). In real-world applications, we usually code much more often around 
0 than around min/max.

That's my 2 cents, don't mean to change/promote any guidelines. If you think 
this is a topic of interest I can bring it to the mailing list, otherwise I'll 
just drop it :)


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

https://reviews.llvm.org/D113847

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

[PATCH] D113306: [PowerPC] Allow MMA built-ins to accept non-void pointers and arrays

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

LGTM. Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113306

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

[PATCH] D113480: [analyzer] Fix region cast between the same types with different qualifiers.

[Denys Petrov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf0bc7d24882a: [analyzer] Fix region cast between the same 
types with different qualifiers. (authored by ASDenysPetrov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113480

Files:
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/initialization.cpp

Index: clang/test/Analysis/initialization.cpp
===
--- clang/test/Analysis/initialization.cpp
+++ clang/test/Analysis/initialization.cpp
@@ -68,8 +68,7 @@
 void glob_invalid_index4() {
   const int *ptr = glob_arr4[1];
   int idx = -42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = ptr[idx]; // no-warning
+  auto x = ptr[idx]; // expected-warning{{garbage or undefined}}
 }
 
 int const glob_arr5[4][2] = {{1}, 3, 4, 5};
@@ -86,16 +85,11 @@
 
 void glob_ptr_index2() {
   int const *ptr = glob_arr5[1];
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(ptr[0] == 3); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(ptr[1] == 4); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(ptr[2] == 5); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(ptr[3] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(ptr[4] == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(ptr[0] == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[1] == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[2] == 5); // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[3] == 0); // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[4] == 0); // expected-warning{{UNDEFINED}}
 }
 
 void glob_invalid_index5() {
@@ -106,8 +100,7 @@
 void glob_invalid_index6() {
   int const *ptr = _arr5[1][0];
   int idx = 42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = ptr[idx]; // no-warning
+  auto x = ptr[idx]; // expected-warning{{garbage or undefined}}
 }
 
 extern const int glob_arr_no_init[10];
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -96,18 +96,24 @@
   // already be handled.
   QualType PointeeTy = CastToTy->getPointeeType();
   QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
+  CanonPointeeTy = CanonPointeeTy.getLocalUnqualifiedType();
 
   // Handle casts to void*.  We just pass the region through.
-  if (CanonPointeeTy.getLocalUnqualifiedType() == Ctx.VoidTy)
+  if (CanonPointeeTy == Ctx.VoidTy)
 return R;
 
-  // Handle casts from compatible types.
-  if (R->isBoundable())
+  const auto IsSameRegionType = [](const MemRegion *R, QualType OtherTy) {
 if (const auto *TR = dyn_cast(R)) {
   QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
-  if (CanonPointeeTy == ObjTy)
-return R;
+  if (OtherTy == ObjTy.getLocalUnqualifiedType())
+return true;
 }
+return false;
+  };
+
+  // Handle casts from compatible types.
+  if (R->isBoundable() && IsSameRegionType(R, CanonPointeeTy))
+return R;
 
   // Process region cast according to the kind of the region being cast.
   switch (R->getKind()) {
@@ -174,16 +180,11 @@
   CharUnits off = rawOff.getOffset();
 
   if (off.isZero()) {
-// Edge case: we are at 0 bytes off the beginning of baseR.  We
-// check to see if type we are casting to is the same as the base
-// region.  If so, just return the base region.
-if (const auto *TR = dyn_cast(baseR)) {
-  QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
-  QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
-  if (CanonPointeeTy == ObjTy)
-return baseR;
-}
-
+// Edge case: we are at 0 bytes off the beginning of baseR. We check to
+// see if the type we are casting to is the same as the type of the base
+// region. If so, just return the base region.
+if (IsSameRegionType(baseR, CanonPointeeTy))
+  return baseR;
 // Otherwise, create a new ElementRegion at offset 0.
 return MakeElementRegion(cast(baseR), PointeeTy);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f0bc7d2 - [analyzer] Fix region cast between the same types with different qualifiers.

[Denys Petrov via cfe-commits]

Author: Denys Petrov
Date: 2021-11-15T19:23:00+02:00
New Revision: f0bc7d24882ab84f6398a1ea614dd0bf6e2a1085

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

LOG: [analyzer] Fix region cast between the same types with different 
qualifiers.

Summary: Specifically, this fixes the case when we get an access to array 
element through the pointer to element. This covers several FIXME's. in 
https://reviews.llvm.org/D111654.
Example:
  const int arr[4][2];
  const int *ptr = arr[1]; // Fixes this.
The issue is that `arr[1]` is `int*` ({Element{glob_arr5,1 
S64b,int[2]},0 S64b,int}), and `ptr` is `const int*`. We don't take qualifiers 
into account. Consequently, we doesn't match the types as the same ones.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/Store.cpp
clang/test/Analysis/initialization.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/Store.cpp 
b/clang/lib/StaticAnalyzer/Core/Store.cpp
index 97b8b9d3d07a7..3cc0cd224d7a4 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -96,18 +96,24 @@ Optional StoreManager::castRegion(const 
MemRegion *R,
   // already be handled.
   QualType PointeeTy = CastToTy->getPointeeType();
   QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
+  CanonPointeeTy = CanonPointeeTy.getLocalUnqualifiedType();
 
   // Handle casts to void*.  We just pass the region through.
-  if (CanonPointeeTy.getLocalUnqualifiedType() == Ctx.VoidTy)
+  if (CanonPointeeTy == Ctx.VoidTy)
 return R;
 
-  // Handle casts from compatible types.
-  if (R->isBoundable())
+  const auto IsSameRegionType = [](const MemRegion *R, QualType OtherTy) {
 if (const auto *TR = dyn_cast(R)) {
   QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
-  if (CanonPointeeTy == ObjTy)
-return R;
+  if (OtherTy == ObjTy.getLocalUnqualifiedType())
+return true;
 }
+return false;
+  };
+
+  // Handle casts from compatible types.
+  if (R->isBoundable() && IsSameRegionType(R, CanonPointeeTy))
+return R;
 
   // Process region cast according to the kind of the region being cast.
   switch (R->getKind()) {
@@ -174,16 +180,11 @@ Optional 
StoreManager::castRegion(const MemRegion *R,
   CharUnits off = rawOff.getOffset();
 
   if (off.isZero()) {
-// Edge case: we are at 0 bytes off the beginning of baseR.  We
-// check to see if type we are casting to is the same as the base
-// region.  If so, just return the base region.
-if (const auto *TR = dyn_cast(baseR)) {
-  QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
-  QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
-  if (CanonPointeeTy == ObjTy)
-return baseR;
-}
-
+// Edge case: we are at 0 bytes off the beginning of baseR. We check to
+// see if the type we are casting to is the same as the type of the 
base
+// region. If so, just return the base region.
+if (IsSameRegionType(baseR, CanonPointeeTy))
+  return baseR;
 // Otherwise, create a new ElementRegion at offset 0.
 return MakeElementRegion(cast(baseR), PointeeTy);
   }

diff  --git a/clang/test/Analysis/initialization.cpp 
b/clang/test/Analysis/initialization.cpp
index 0883678c8e908..e5b94ea7d0a2b 100644
--- a/clang/test/Analysis/initialization.cpp
+++ b/clang/test/Analysis/initialization.cpp
@@ -68,8 +68,7 @@ void glob_invalid_index3() {
 void glob_invalid_index4() {
   const int *ptr = glob_arr4[1];
   int idx = -42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = ptr[idx]; // no-warning
+  auto x = ptr[idx]; // expected-warning{{garbage or undefined}}
 }
 
 int const glob_arr5[4][2] = {{1}, 3, 4, 5};
@@ -86,16 +85,11 @@ void glob_array_index3() {
 
 void glob_ptr_index2() {
   int const *ptr = glob_arr5[1];
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(ptr[0] == 3); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(ptr[1] == 4); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(ptr[2] == 5); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(ptr[3] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be UNDEFINED.
-  clang_analyzer_eval(ptr[4] == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(ptr[0] == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[1] == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[2] == 5); // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[3] == 0); // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[4] == 0); // 

[PATCH] D113779: [Clang] Add mfp16, mfp16fml and mdotprod flags for ARM target features.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

In D113779#3130853 , @paulwalker-arm 
wrote:

> Rather than adding connivence options after the fact what about allowing 
> `-march=` to be specified multiple times? The first must be the usual format 
> with later ones required to start with `+`.  The defined parsing behaviour 
> would be as if there was a single `-march` instance positioned at the first 
> occurrence but containing the value of all instances when combined from left 
> to right.  For example `-march=armv8.4-a .. -march=+nofp16` or perhaps 
> `+=` syntax like  `-march=armv8.4-a .. -march+=nofp16+nosve` is more 
> intuitive?

I think that would be a convenient option which wouldn't require us to add a 
lot of new options, which would be quite cumbersome. But to address the main 
issues (providing a purely additive way to specify features) I think we would 
need to allow `-march=+feature`, without any preceding `-march=armvXXX`.

In D113779#3131569 , @manojgupta 
wrote:

> Yes, the current approach of "-march=+feature" is terrible and does not 
> work with developers who want flexibility of features. This being pitched as 
> a feature imo is akin to promoting a design bug as a feature. 
> Any additive or subtractive alternative is welcome.

I wouldn't go so far as to call the current `-march` handling terrible, but I 
think there are valid use cases that cannot be addressed with it, as per the 
current discussion.  As you said, providing some way specify features 
additively without also committing to a specific architecture version would be 
desirable for our users IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113779

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

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 387290.
carlosgalvezp added a comment.

Fix refactoring bug.


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

https://reviews.llvm.org/D113847

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -45,18 +45,13 @@
   std::vector EnabledDiagnosticAliases;
 };
 
-/// Contains displayed and ignored diagnostic counters for a ClangTidy
-/// run.
+/// Contains displayed and ignored diagnostic counters for a ClangTidy run.
 struct ClangTidyStats {
-  ClangTidyStats()
-  : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), 
ErrorsIgnoredNOLINT(0),
-ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
-
-  unsigned ErrorsDisplayed;
-  unsigned ErrorsIgnoredCheckFilter;
-  unsigned ErrorsIgnoredNOLINT;
-  unsigned ErrorsIgnoredNonUserCode;
-  unsigned ErrorsIgnoredLineFilter;
+  unsigned ErrorsDisplayed = 0;
+  unsigned ErrorsIgnoredCheckFilter = 0;
+  unsigned ErrorsIgnoredNOLINT = 0;
+  unsigned ErrorsIgnoredNonUserCode = 0;
+  unsigned ErrorsIgnoredLineFilter = 0;
 
   unsigned errorsIgnored() const {
 return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +


Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -45,18 +45,13 @@
   std::vector EnabledDiagnosticAliases;
 };
 
-/// Contains displayed and ignored diagnostic counters for a ClangTidy
-/// run.
+/// Contains displayed and ignored diagnostic counters for a ClangTidy run.
 struct ClangTidyStats {
-  ClangTidyStats()
-  : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), ErrorsIgnoredNOLINT(0),
-ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
-
-  unsigned ErrorsDisplayed;
-  unsigned ErrorsIgnoredCheckFilter;
-  unsigned ErrorsIgnoredNOLINT;
-  unsigned ErrorsIgnoredNonUserCode;
-  unsigned ErrorsIgnoredLineFilter;
+  unsigned ErrorsDisplayed = 0;
+  unsigned ErrorsIgnoredCheckFilter = 0;
+  unsigned ErrorsIgnoredNOLINT = 0;
+  unsigned ErrorsIgnoredNonUserCode = 0;
+  unsigned ErrorsIgnoredLineFilter = 0;
 
   unsigned errorsIgnored() const {
 return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113892: [NFC][clangd] cleanup llvm-else-after-return findings

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Mostly looks good. There's a bunch of ifs that are now trivial (condition and 
body each fit on one line, no else) and we'd usually drop braces there, but 
it's probably not worth the hassle.

I have to say that this rule slightly improves "early exit after error 
condition" type code, but makes it less clear that "there are 3 cases" type 
code is exhaustive.
So personally I'm not sure the check is a win if we want to follow it 
everywhere, I'd consider fixing the cases where it improves things and then 
turning it off. @kadircet am I being too picky here?




Comment at: clang-tools-extra/clangd/JSONTransport.cpp:260
   continue;
 } else {
   // An empty line indicates the end of headers.

This seems inconsistent: we're elseing after continue.

If we've using this style, i'd invert the if here so the "continue" case just 
falls off the end of the loop body. (But keep the comment)



Comment at: clang-tools-extra/clangd/JSONTransport.cpp:283
+Read = retryAfterSignalUnlessShutdown(
+0, [&] { return std::fread([Pos], 1, ContentLength - Pos, In); });
 if (Read == 0) {

Looks like there are unrelated formatting changes in a couple of files



Comment at: clang-tools-extra/clangd/Selection.cpp:350
   }
+  /* fall through and treat as part of the macro body */
 }

FWIW I find this one *much* less clear :-(
I'd personally prefer to ignore the rule in this case, but curious what others 
think


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113892

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

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

FYI this is a minimal repro taken from Martin's preprocessed source (Thanks!):

  template  struct a { using b = const float; };
  template  using d = typename a::b;  
  
  template  void e(d *, c) {}
  template void e(typename a::b *, int);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[PATCH] D113902: [NFC][clangd] exclude test data from clang-tidy

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Hmm, clang-tidy should only be running on entries in compile_commands.json. are 
these files listed somehow?

(I don't like the idea that every test inputs dir may need to contain a 
scattering of files to disable tools. At most we should be able to do this 
under test/)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113902

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

[PATCH] D113892: [NFC][clangd] cleanup llvm-else-after-return findings

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel created this revision.
Herald added subscribers: carlosgalvezp, usaxena95, kadircet, arphaman, 
javed.absar.
kuhnel added subscribers: sammccall, hokein, adamcz, kbobyrev.
kuhnel published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Cleanup of clang-tidy findings: removing "else" after a return statement
to improve readability of the code.

This patch was created by applying the clang-tidy fixes automatically.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113892

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/FuzzyMatch.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/JSONTransport.cpp
  clang-tools-extra/clangd/PathMapping.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp

Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -223,8 +223,7 @@
   while (const auto *CS = llvm::dyn_cast(Last)) {
 if (CS->body_empty())
   return false;
-else
-  Last = CS->body_back();
+Last = CS->body_back();
   }
   return llvm::isa(Last);
 }
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -250,7 +250,8 @@
   for (; Node->Parent; Node = Node->Parent) {
 if (Node->ASTNode.get()) {
   continue;
-} else if (auto *T = Node->ASTNode.get()) {
+}
+if (auto *T = Node->ASTNode.get()) {
   if (T->getAs()) {
 break;
   } else if (Node->Parent->ASTNode.get() ||
Index: clang-tools-extra/clangd/index/Serialization.cpp
===
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -687,7 +687,8 @@
 llvm::Expected readIndexFile(llvm::StringRef Data) {
   if (Data.startswith("RIFF")) {
 return readRIFF(Data);
-  } else if (auto YAMLContents = readYAML(Data)) {
+  }
+  if (auto YAMLContents = readYAML(Data)) {
 return std::move(*YAMLContents);
   } else {
 return error("Not a RIFF file and failed to parse as YAML: {0}",
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -401,8 +401,8 @@
 }
   }
   // Special case: void foo() ^override: jump to the overridden method.
-if (NodeKind.isSame(ASTNodeKind::getFromNodeKind()) ||
-NodeKind.isSame(ASTNodeKind::getFromNodeKind())) {
+  if (NodeKind.isSame(ASTNodeKind::getFromNodeKind()) ||
+  NodeKind.isSame(ASTNodeKind::getFromNodeKind())) {
 // We may be overridding multiple methods - offer them all.
 for (const NamedDecl *ND : CMD->overridden_methods())
   AddResultDecl(ND);
@@ -826,10 +826,9 @@
 log("Found definition heuristically using nearby identifier {0}",
 NearbyIdent->text(SM));
 return ASTResults;
-  } else {
-vlog("No definition found using nearby identifier {0} at {1}",
- Word->Text, Word->Location.printToString(SM));
   }
+  vlog("No definition found using nearby identifier {0} at {1}", Word->Text,
+   Word->Location.printToString(SM));
 }
 // No nearby word, or it didn't refer to anything either. Try the index.
 auto TextualResults =
@@ -1430,28 +1429,27 @@
 // Add decl/def of overridding methods.
 if (Index && Results.References.size() <= Limit &&
 !OverriddenBy.Subjects.empty())
-  Index->relations(
-  OverriddenBy, [&](const SymbolID , const Symbol ) {
-const auto LSPLocDecl =
-toLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
-const auto LSPLocDef =
-toLSPLocation(Object.Definition, *MainFilePath);
-if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
-  ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLocDecl);
-  Result.Attributes =
-  ReferencesResult::Declaration | ReferencesResult::Override;
-  Results.References.push_back(std::move(Result));
-}
-if (LSPLocDef) {
-  ReferencesResult::Reference Result;
-  Result.Loc = std::move(*LSPLocDef);
-   

[PATCH] D113895: [NFC][clangd] fix llvm-namespace-comment finding

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel created this revision.
Herald added subscribers: carlosgalvezp, usaxena95, kadircet, arphaman.
kuhnel published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Fixing the clang-tidy finding.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113895

Files:
  clang-tools-extra/clangd/xpc/XPCTransport.cpp


Index: clang-tools-extra/clangd/xpc/XPCTransport.cpp
===
--- clang-tools-extra/clangd/xpc/XPCTransport.cpp
+++ clang-tools-extra/clangd/xpc/XPCTransport.cpp
@@ -47,7 +47,7 @@
 // C "closure" for XPCTransport::loop() method
 namespace xpcClosure {
 void connection_handler(xpc_connection_t clientConnection);
-}
+} // namespace xpcClosure
 
 class XPCTransport : public Transport {
 public:


Index: clang-tools-extra/clangd/xpc/XPCTransport.cpp
===
--- clang-tools-extra/clangd/xpc/XPCTransport.cpp
+++ clang-tools-extra/clangd/xpc/XPCTransport.cpp
@@ -47,7 +47,7 @@
 // C "closure" for XPCTransport::loop() method
 namespace xpcClosure {
 void connection_handler(xpc_connection_t clientConnection);
-}
+} // namespace xpcClosure
 
 class XPCTransport : public Transport {
 public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113899: [NFC][clangd] fix clang-tidy finding on isa_and_nonnull

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel created this revision.
Herald added subscribers: carlosgalvezp, usaxena95, kadircet, arphaman.
kuhnel added subscribers: sammccall, kbobyrev, adamcz.
kadircet added inline comments.
kuhnel published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.



Comment at: clang-tools-extra/clangd/Selection.cpp:504
   bool TraverseDecl(Decl *X) {
-if (X && isa(X))
+if (isa_and_nonnull(X))
   return Base::TraverseDecl(X); // Already pushed by constructor.

while here `llvm::isa_and_nonnull` we try to qualify symbols from llvm 
namespace.


This is a cleanup of the only llvm-prefer-isa-or-dyn-cast-in-conditionals 
finding in the clangd code base. This patch was created by automatically 
applying the fixes from clang-tidy.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113899

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


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -501,7 +501,7 @@
   //  - those without source range information, we don't record those
   //  - those that can't be stored in DynTypedNode.
   bool TraverseDecl(Decl *X) {
-if (X && isa(X))
+if (isa_and_nonnull(X))
   return Base::TraverseDecl(X); // Already pushed by constructor.
 // Base::TraverseDecl will suppress children, but not this node itself.
 if (X && X->isImplicit())


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -501,7 +501,7 @@
   //  - those without source range information, we don't record those
   //  - those that can't be stored in DynTypedNode.
   bool TraverseDecl(Decl *X) {
-if (X && isa(X))
+if (isa_and_nonnull(X))
   return Base::TraverseDecl(X); // Already pushed by constructor.
 // Base::TraverseDecl will suppress children, but not this node itself.
 if (X && X->isImplicit())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113902: [NFC][clangd] exclude test data from clang-tidy

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
kuhnel added subscribers: sammccall, adamcz, kbobyrev.
kuhnel published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov, aheejin.
Herald added a project: clang-tools-extra.

Adding .clang-tidy files to exclude test data from clang-tidy checks. Otherwise 
these create false positives.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113902

Files:
  clang-tools-extra/clangd/test/Inputs/.clang-tidy
  clang-tools-extra/clangd/test/index-serialization/Inputs/.clang-tidy
  clang-tools-extra/clangd/test/remote-index/Inputs/.clang-tidy


Index: clang-tools-extra/clangd/test/remote-index/Inputs/.clang-tidy
===
--- /dev/null
+++ clang-tools-extra/clangd/test/remote-index/Inputs/.clang-tidy
@@ -0,0 +1,3 @@
+# disable the check on the test input data
+InheritParentConfig: true
+Checks: "-misc-definitions-in-headers"
Index: clang-tools-extra/clangd/test/index-serialization/Inputs/.clang-tidy
===
--- /dev/null
+++ clang-tools-extra/clangd/test/index-serialization/Inputs/.clang-tidy
@@ -0,0 +1,3 @@
+# disable the check on the test input data
+InheritParentConfig: true
+Checks: "-clang-diagnostic-non-virtual-dtor,-readability-identifier-naming"
Index: clang-tools-extra/clangd/test/Inputs/.clang-tidy
===
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/.clang-tidy
@@ -0,0 +1,3 @@
+# disable the check on the test input data
+InheritParentConfig: true
+Checks: 
"-misc-definitions-in-headers,-llvm-header-guard,-clang-diagnostic-unused-variable"


Index: clang-tools-extra/clangd/test/remote-index/Inputs/.clang-tidy
===
--- /dev/null
+++ clang-tools-extra/clangd/test/remote-index/Inputs/.clang-tidy
@@ -0,0 +1,3 @@
+# disable the check on the test input data
+InheritParentConfig: true
+Checks: "-misc-definitions-in-headers"
Index: clang-tools-extra/clangd/test/index-serialization/Inputs/.clang-tidy
===
--- /dev/null
+++ clang-tools-extra/clangd/test/index-serialization/Inputs/.clang-tidy
@@ -0,0 +1,3 @@
+# disable the check on the test input data
+InheritParentConfig: true
+Checks: "-clang-diagnostic-non-virtual-dtor,-readability-identifier-naming"
Index: clang-tools-extra/clangd/test/Inputs/.clang-tidy
===
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/.clang-tidy
@@ -0,0 +1,3 @@
+# disable the check on the test input data
+InheritParentConfig: true
+Checks: "-misc-definitions-in-headers,-llvm-header-guard,-clang-diagnostic-unused-variable"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

There is a reasonable style rule here, but:

- the suggested fixes spell out `const` in ways we generally prefer not to. 
(And the rule doesn't require).
- Strings are a special case. `auto*` for string literals makes no sense to me, 
the check should be modified to either accept `auto` or replace with `const 
char*`




Comment at: clang-tools-extra/clangd/AST.cpp:441
   DeducedType = AT->getDeducedType();
-} else if (auto DT = dyn_cast(D->getReturnType())) {
+} else if (const auto *DT = dyn_cast(D->getReturnType())) {
   // auto in a trailing return type just points to a DecltypeType and

We usually use `auto*` rather than `const auto*` for locals like this.

For the same reason we don't `const` locals: it adds more noise than it helps.
(And if it's critical we spell out these things, we probably shouldn't be using 
auto at all)

When this check was introduced, there was a discussion about this and they 
agreed to have the check stop flagging `auto*`, but if it sees `auto` it will 
still suggest the const. So the automatic fixes generally aren't what we want, 
we need to drop const.



Comment at: clang-tools-extra/clangd/index/IndexAction.cpp:64
 const auto FileID = SM.getFileID(Loc);
-const auto File = SM.getFileEntryForID(FileID);
+const auto *const File = SM.getFileEntryForID(FileID);
 auto URI = toURI(File);

This is a particularly confusing type.
The original code is atypical for clangd, I'd suggest changing to `auto* File`



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:209
 OS << "(| ";
-auto Separator = "";
+const auto *Separator = "";
 for (const auto  : Children) {

`const char*` or `auto` or `StringRef` for strings. "Some kind of pointer" is 
technically correct but unhelpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113898

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

[PATCH] D112639: [openmp][amdgpu] Add comment warning that libm may be broken

[Jon Chesterfield via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e738323a9c4: [openmp][amdgpu] Add comment warning that libm 
may be broken (authored by JonChesterfield).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112639

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


Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -106,6 +106,22 @@
 }
 
 if (HasLibm) {
+  // This is not certain to work. The device libs added here, and passed to
+  // llvm-link, are missing attributes that they expect to be inserted when
+  // passed to mlink-builtin-bitcode. The amdgpu backend does not generate
+  // conservatively correct code when attributes are missing, so this may
+  // be the root cause of miscompilations. Passing via 
mlink-builtin-bitcode
+  // ultimately hits CodeGenModule::addDefaultFunctionDefinitionAttributes
+  // on each function, see D28538 for context.
+  // Potential workarounds:
+  //  - unconditionally link all of the device libs to every translation
+  //unit in clang via mlink-builtin-bitcode
+  //  - build a libm bitcode file as part of the DeviceRTL and explictly
+  //mlink-builtin-bitcode the rocm device libs components at build time
+  //  - drop this llvm-link fork in favour or some calls into LLVM, chosen
+  //to do basically the same work as llvm-link but with that call first
+  //  - write an opt pass that sets that on every function it sees and pipe
+  //the device-libs bitcode through that on the way to this llvm-link
   SmallVector BCLibs =
   AMDGPUOpenMPTC.getCommonDeviceLibNames(Args, SubArchName.str());
   llvm::for_each(BCLibs, [&](StringRef BCFile) {


Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -106,6 +106,22 @@
 }
 
 if (HasLibm) {
+  // This is not certain to work. The device libs added here, and passed to
+  // llvm-link, are missing attributes that they expect to be inserted when
+  // passed to mlink-builtin-bitcode. The amdgpu backend does not generate
+  // conservatively correct code when attributes are missing, so this may
+  // be the root cause of miscompilations. Passing via mlink-builtin-bitcode
+  // ultimately hits CodeGenModule::addDefaultFunctionDefinitionAttributes
+  // on each function, see D28538 for context.
+  // Potential workarounds:
+  //  - unconditionally link all of the device libs to every translation
+  //unit in clang via mlink-builtin-bitcode
+  //  - build a libm bitcode file as part of the DeviceRTL and explictly
+  //mlink-builtin-bitcode the rocm device libs components at build time
+  //  - drop this llvm-link fork in favour or some calls into LLVM, chosen
+  //to do basically the same work as llvm-link but with that call first
+  //  - write an opt pass that sets that on every function it sees and pipe
+  //the device-libs bitcode through that on the way to this llvm-link
   SmallVector BCLibs =
   AMDGPUOpenMPTC.getCommonDeviceLibNames(Args, SubArchName.str());
   llvm::for_each(BCLibs, [&](StringRef BCFile) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 0e73832 - [openmp][amdgpu] Add comment warning that libm may be broken

[Jon Chesterfield via cfe-commits]

Author: Jon Chesterfield
Date: 2021-11-15T15:56:01Z
New Revision: 0e738323a9c445e31b4e1b1dcb2beb19d6f103ef

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

LOG: [openmp][amdgpu] Add comment warning that libm may be broken

Using llvm-link to add rocm device-libs probably doesn't work

Reviewed By: arsenm

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp 
b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
index b138000f8cf2..863e2c597d53 100644
--- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -106,6 +106,22 @@ const char *AMDGCN::OpenMPLinker::constructLLVMLinkCommand(
 }
 
 if (HasLibm) {
+  // This is not certain to work. The device libs added here, and passed to
+  // llvm-link, are missing attributes that they expect to be inserted when
+  // passed to mlink-builtin-bitcode. The amdgpu backend does not generate
+  // conservatively correct code when attributes are missing, so this may
+  // be the root cause of miscompilations. Passing via 
mlink-builtin-bitcode
+  // ultimately hits CodeGenModule::addDefaultFunctionDefinitionAttributes
+  // on each function, see D28538 for context.
+  // Potential workarounds:
+  //  - unconditionally link all of the device libs to every translation
+  //unit in clang via mlink-builtin-bitcode
+  //  - build a libm bitcode file as part of the DeviceRTL and explictly
+  //mlink-builtin-bitcode the rocm device libs components at build time
+  //  - drop this llvm-link fork in favour or some calls into LLVM, chosen
+  //to do basically the same work as llvm-link but with that call first
+  //  - write an opt pass that sets that on every function it sees and pipe
+  //the device-libs bitcode through that on the way to this llvm-link
   SmallVector BCLibs =
   AMDGPUOpenMPTC.getCommonDeviceLibNames(Args, SubArchName.str());
   llvm::for_each(BCLibs, [&](StringRef BCFile) {



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

[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

Hey, I'm really sorry, I missed this, too but I just landed the patch for you, 
apologies for such a huge delay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97265

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

[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Kirill Bobyrev 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 rGda168dd875bf: [clang] Allow clang-check to customize 
analyzer output file or dir name (authored by OikawaKirie, committed by 
kbobyrev).
Herald added a subscriber: manas.

Changed prior to commit:
  https://reviews.llvm.org/D97265?vs=328927=387266#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97265

Files:
  clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
  clang/tools/clang-check/ClangCheck.cpp


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -76,6 +76,10 @@
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
@@ -206,7 +210,19 @@
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the
Index: clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: echo '[{"directory":".","command":"clang++ -c %t/test.cpp -o foo 
-ofoo","file":"%t/test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: echo '// CHECK: {{qwerty}}' > %t/cclog-check
+// RUN: clang-check -p "%t" "%t/test.cpp" -analyze 
-analyzer-output-path=%t/qwerty -extra-arg=-v -extra-arg=-Xclang 
-extra-arg=-verify 2>&1 | FileCheck %t/cclog-check
+// RUN: FileCheck %s --input-file=%t/qwerty
+
+// CHECK: DOCTYPE plist
+// CHECK: Division by zero
+int f() {
+  return 1 / 0; // expected-warning {{Division by zero}}
+}


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -76,6 +76,10 @@
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
@@ -206,7 +210,19 @@
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the
Index: clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: echo '[{"directory":".","command":"clang++ -c %t/test.cpp -o foo -ofoo","file":"%t/test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: echo '// CHECK: {{qwerty}}' > 

[clang] da168dd - [clang] Allow clang-check to customize analyzer output file or dir name

[Kirill Bobyrev via cfe-commits]

Author: Ella Ma
Date: 2021-11-15T16:49:41+01:00
New Revision: da168dd875bf0392e8e88834009d776bfbaae376

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

LOG: [clang] Allow clang-check to customize analyzer output file or dir name

Required by https://stackoverflow.com/questions/58073606

As the output argument is stripped out in the clang-check tool, it seems 
impossible for clang-check users to customize the output file name, even with 
-extra-args and -extra-arg-before.

This patch adds the -analyzer-output-path argument to allow users to adjust the 
output name. And if the argument is not set or the analyzer is not enabled, the 
original strip output adjuster will remove the output arguments.

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

Added: 
clang/test/Tooling/clang-check-set-analyzer-output-path.cpp

Modified: 
clang/tools/clang-check/ClangCheck.cpp

Removed: 




diff  --git a/clang/test/Tooling/clang-check-set-analyzer-output-path.cpp 
b/clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
new file mode 100644
index ..ff5e86ae7892
--- /dev/null
+++ b/clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: echo '[{"directory":".","command":"clang++ -c %t/test.cpp -o foo 
-ofoo","file":"%t/test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: echo '// CHECK: {{qwerty}}' > %t/cclog-check
+// RUN: clang-check -p "%t" "%t/test.cpp" -analyze 
-analyzer-output-path=%t/qwerty -extra-arg=-v -extra-arg=-Xclang 
-extra-arg=-verify 2>&1 | FileCheck %t/cclog-check
+// RUN: FileCheck %s --input-file=%t/qwerty
+
+// CHECK: DOCTYPE plist
+// CHECK: Division by zero
+int f() {
+  return 1 / 0; // expected-warning {{Division by zero}}
+}

diff  --git a/clang/tools/clang-check/ClangCheck.cpp 
b/clang/tools/clang-check/ClangCheck.cpp
index 11fdeb71fd9e..4d6ded029e2f 100644
--- a/clang/tools/clang-check/ClangCheck.cpp
+++ b/clang/tools/clang-check/ClangCheck.cpp
@@ -76,6 +76,10 @@ static cl::opt
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
@@ -206,7 +210,19 @@ int main(int argc, const char **argv) {
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the



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

[PATCH] D112430: [ARM][libunwind] add PACBTI-M support for libunwind

[Daniel Kiss via Phabricator via cfe-commits]

danielkiss accepted this revision.
danielkiss added a comment.

LGTM, Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112430

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

[PATCH] D113896: [NFC][clangd] cleanup of header guard names

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Mostly LG, just a few files are test inputs and one is empty




Comment at: 
clang-tools-extra/clangd/test/Inputs/background-index/sub_dir/foo.h:1
-#ifndef FOO_H
-#define FOO_H
+#ifndef 
LLVM_CLANG_TOOLS_EXTRA_CLANGD_TEST_INPUTS_BACKGROUND_INDEX_SUB_DIR_FOO_H
+#define 
LLVM_CLANG_TOOLS_EXTRA_CLANGD_TEST_INPUTS_BACKGROUND_INDEX_SUB_DIR_FOO_H

Please don't modify tests, they don't need to follow LLVM style and should be 
as simple as possible



Comment at: clang-tools-extra/clangd/test/index-serialization/Inputs/sample.h:1
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TEST_INDEX_SERIALIZATION_INPUTS_SAMPLE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TEST_INDEX_SERIALIZATION_INPUTS_SAMPLE_H

In particular, adding header guards to test files without them isn't NFC



Comment at: clang-tools-extra/clangd/unittests/TestScheme.h:1
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTSCHEME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTSCHEME_H

What's up with this change?
If this file is empty it should just be deleted instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113896

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