[PATCH] D83154: clang: Add -fcoverage-prefix-map

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D83154#2146085 , @phosek wrote:

> In D83154#2134984 , @MaskRay wrote:
>
> > -fdebug-prefix-map does not make sense to me because coverage is not debug 
> > info.
> >  I am on the fence whether we need -fcoverage-prefix-map. I created 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96092 (Should --coverage 
> > respect -ffile-prefix-map?)
>
>
> Looks like GCC has `-fprofile-prefix-path` 
>  in the queue which is 
> trying to achieve a similar thing. I'd prefer `-fprofile-prefix-map` to be 
> consistent with the existing `-f*-prefix-map` options, both in terms of 
> spelling and usage. I think that `-fprofile-prefix-map` is better than 
> `-fcoverage-prefix-map` because it makes it clear that it applies 
> `-fprofile-*` set of options.


`-fprofile-prefix-map` looks good to me. I made another comment and CCed the 
author of 
https://gcc.gnu.org/git/?p=gcc.git=commit;h=44b326839d864fc10c459916abcc97f35a9ac3de
 (which added -fprofile-prefix-path) in that bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83154



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 277272.
yaxunl marked 3 inline comments as done.
yaxunl added a comment.

Only allow cuid to be alphanumeric and underscore.


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

https://reviews.llvm.org/D80858

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Action.h
  clang/include/clang/Driver/Compilation.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Driver/Action.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCUDA/static-device-var.cu
  clang/test/Driver/hip-cuid.hip
  clang/test/Frontend/hip-cuid.hip
  clang/test/SemaCUDA/static-device-var.cu

Index: clang/test/SemaCUDA/static-device-var.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/static-device-var.cu
@@ -0,0 +1,37 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang_cc1 -triple nvptx -fcuda-is-device \
+// RUN:-emit-llvm -o - %s -fsyntax-only -verify
+
+// RUN: %clang_cc1 -triple x86_64-gnu-linux \
+// RUN:-emit-llvm -o - %s -fsyntax-only -verify
+
+#include "Inputs/cuda.h"
+
+__device__ void f1() {
+  const static int b = 123;
+  static int a;
+  // expected-error@-1 {{within a __device__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+}
+
+__global__ void k1() {
+  const static int b = 123;
+  static int a;
+  // expected-error@-1 {{within a __global__ function, only __shared__ variables or const variables without device memory qualifier may be marked 'static'}}
+}
+
+static __device__ int x;
+static __constant__ int y;
+
+__global__ void kernel(int *a) {
+  a[0] = x;
+  a[1] = y;
+}
+
+int* getDeviceSymbol(int *x);
+
+void foo() {
+  getDeviceSymbol();
+  getDeviceSymbol();
+}
Index: clang/test/Frontend/hip-cuid.hip
===
--- /dev/null
+++ clang/test/Frontend/hip-cuid.hip
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -cuid=abc-123 -offload-arch=gfx906 %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=INVALID %s
+
+// INVALID: invalid value 'abc-123' in '-cuid=abc-123' (alphanumeric characters and underscore only)
+
+// RUN: %clang_cc1 -cuid=abc_123 -offload-arch=gfx906 %s
Index: clang/test/Driver/hip-cuid.hip
===
--- /dev/null
+++ clang/test/Driver/hip-cuid.hip
@@ -0,0 +1,130 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// Check invalid -fuse-cuid= option.
+
+// RUN: not %clang -### -x hip \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   --offload-arch=gfx906 \
+// RUN:   -c -nogpulib -fuse-cuid=invalid \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck -check-prefixes=INVALID %s
+
+// INVALID: invalid value 'invalid' in '-fuse-cuid=invalid'
+
+// Check random CUID generator.
+
+// RUN: %clang -### -x hip \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   --offload-arch=gfx906 \
+// RUN:   -c -nogpulib -fuse-cuid=random \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck -check-prefixes=COMMON,HEX %s
+
+// Check fixed CUID.
+
+// RUN: %clang -### -x hip \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   --offload-arch=gfx906 \
+// RUN:   -c -nogpulib -cuid=xyz_123 \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck -check-prefixes=COMMON,FIXED %s
+
+// Check fixed CUID override -fuse-cuid.
+
+// RUN: %clang -### -x hip \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   --offload-arch=gfx906 \
+// RUN:   -c -nogpulib -fuse-cuid=random -cuid=xyz_123 \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck -check-prefixes=COMMON,FIXED %s
+
+// Check hash CUID generator.
+
+// RUN: %clang -### -x hip \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   --offload-arch=gfx900 \
+// RUN:   --offload-arch=gfx906 \
+// RUN:   -c -nogpulib -fuse-cuid=hash \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck -check-prefixes=COMMON,HEX %s
+
+// COMMON: "{{.*}}clang{{.*}}" "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// COMMON-SAME: "-target-cpu" "gfx900"
+// HEX-SAME: 

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 9 inline comments as done.
yaxunl added inline comments.
Herald added a subscriber: dang.



Comment at: clang/lib/AST/ASTContext.cpp:10068
+isa(D) && cast(D)->isFileVarDecl() &&
+cast(D)->getStorageClass() == SC_Static) {
+  return GVA_StrongExternal;

JonChesterfield wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > Are you sure this doesn't apply to e.g. local statics?  Can't you have 
> > > kernel lambdas, or am I confusing HIP with another language?
> > function-scope static var in a device function is only visible to the 
> > device function. Host code cannot access it, therefore no need to 
> > externalize it.
> This doesn't sound right. An inline function can return a pointer to a 
> function scope static variable, e.g. to implement a singleton in a header 
> file.  I think host code can then access said variable.
As long as we are not accessing the static variable by symbol we do not need 
externalize it.

If a device function returns a pointer to its static variable and somehow 
passes that pointer to host code, the host code can use it directly by 
hipMemCpy.



Comment at: clang/lib/AST/ASTContext.cpp:10068
+isa(D) && cast(D)->isFileVarDecl() &&
+cast(D)->getStorageClass() == SC_Static) {
+  return GVA_StrongExternal;

rjmccall wrote:
> yaxunl wrote:
> > JonChesterfield wrote:
> > > yaxunl wrote:
> > > > rjmccall wrote:
> > > > > Are you sure this doesn't apply to e.g. local statics?  Can't you 
> > > > > have kernel lambdas, or am I confusing HIP with another language?
> > > > function-scope static var in a device function is only visible to the 
> > > > device function. Host code cannot access it, therefore no need to 
> > > > externalize it.
> > > This doesn't sound right. An inline function can return a pointer to a 
> > > function scope static variable, e.g. to implement a singleton in a header 
> > > file.  I think host code can then access said variable.
> > As long as we are not accessing the static variable by symbol we do not 
> > need externalize it.
> > 
> > If a device function returns a pointer to its static variable and somehow 
> > passes that pointer to host code, the host code can use it directly by 
> > hipMemCpy.
> Right, and IIRC you can declare __host__ __device__ functions as well, which 
> ought to agree on the variable if they agree on globals.
If we have a static variable in a device function, it is only visible in the 
function and not visible by any host code. We only need externalize it if it 
needs to be accessed `by symbol` in the host code, however, that is impossible, 
therefore we do not need externalize it.

For static variables in a host device function, the static variables should be 
different instances on host side and device side. The rationale is that a 
static variable is per function, whereas a host device function is actually two 
functions: a host instance and a device instance, which could be totally 
different by using conditional macros. If it is requested that the static 
variable in a host device function is one instance, it requires special 
handling in runtime so that the same variable can be accessed on both device 
side and host side by common load/store instructions, but that is not the case. 
Therefore the device side instance of a static variable in a host device 
function is still only visible to device codes, not visible to host codes. 
Since it cannot be accessed `by symbol` by host code, it does not needs to be 
externalized.
 



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6069
+llvm::raw_ostream ) const {
+  OS << ".static." << getLangOpts().CUID;
+}

tra wrote:
> I suspect that will have interesting issues if CUID is an arbitrary 
> user-supplied string. We may want to impose some sort of sanity check or 
> filtering on the cuid value. Considering that it's a CC1 flag, it's not a 
> critical problem, but some safeguards would be useful there, too. Should we 
> limit allowed character set?
will only allow alphanumeric and underscore in CUID for simplicity.



Comment at: clang/test/Driver/hip-cuid.hip:35
+// RUN:   --offload-arch=gfx906 \
+// RUN:   -c -nogpulib -cuid=abcd \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \

tra wrote:
> Nit: `abcd` could potentially match the value generated by hash. I'd change 
> it to contain characters other than hex.
done


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

https://reviews.llvm.org/D80858



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

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen added a comment.

@asb @lenary I thought this path is ready to land?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D82930: [HIP] Fix rocm detection

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Great, thanks :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82930



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

[PATCH] D83154: clang: Add -fcoverage-prefix-map

[Petr Hosek via Phabricator via cfe-commits]

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

In D83154#2134984 , @MaskRay wrote:

> -fdebug-prefix-map does not make sense to me because coverage is not debug 
> info.
>  I am on the fence whether we need -fcoverage-prefix-map. I created 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96092 (Should --coverage respect 
> -ffile-prefix-map?)


Looks like GCC has `-fprofile-prefix-path` 
 in the queue which is 
trying to achieve a similar thing. I'd prefer `-fprofile-prefix-map` to be 
consistent with the existing `-f*-prefix-map` options, both in terms of 
spelling and usage. I think that `-fprofile-prefix-map` is better than 
`-fcoverage-prefix-map` because it makes it clear that it applies `-fprofile-*` 
set of options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83154



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

[PATCH] D82728: [clang] Add -Wsuggest-override

[David Blaikie via Phabricator via cfe-commits]

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

In D82728#2142071 , @logan-5 wrote:

> Feels like a dumb question, but I'm not sure if and how those build failures 
> are related to this patch? They seem to involve completely separate parts of 
> the LLVM project (and `.c` files to boot). Was it that the build was failing 
> for an unrelated reason at the moment the bots happened to build this patch? 
> Or am I misunderstanding something more obvious?


Yeah, don't think those are related, no. Wouldn't worry about them.

Looks good - thanks for the patch and all the details! Might be worth turning 
on by default in the LLVM build (after all the cleanup)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82728



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

[PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a subscriber: mehdi_amini.
MaskRay added a comment.

In D82477#2145967 , @clayborg wrote:

> In D82477#2145565 , @MaskRay wrote:
>
> > Hi, your git commit contains extra Phabricator tags. You can drop 
> > `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git 
> > commit with the following script:
> >
> >   arcfilter () {
> >   arc amend
> >   git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} 
> > /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ 
> > {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
> >   }
> >   
> >
> > `Reviewed By: ` is considered important by some people. Please keep the 
> > tag. (`--date=now` is my personal preference (author dates are usually not 
> > useful. Using committer dates can make log almost monotonic in time))
> >
> > `llvm/utils/git/pre-push.py` can validate the message does not include 
> > unneeded tags.
>
>
> Can we modify this script to remove the unneeded tags instead of detecting it?


@mehdi_amini ^^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477



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

[clang] b8409c0 - Fix `-Wreturn-type` warning. NFC.

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2020-07-11T16:20:41-04:00
New Revision: b8409c03ed90807f3d49c7d98dceea98cf461f7a

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

LOG: Fix `-Wreturn-type` warning. NFC.

Added: 


Modified: 
clang/lib/Tooling/Syntax/BuildTree.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp 
b/clang/lib/Tooling/Syntax/BuildTree.cpp
index 6d13f1ace83b..1f192180ec45 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -750,6 +750,7 @@ class BuildTreeVisitor : public 
RecursiveASTVisitor {
 return new (allocator()) syntax::FloatUserDefinedLiteralExpression;
   }
 }
+llvm_unreachable("Unknown literal operator kind.");
   }
 
   bool WalkUpFromUserDefinedLiteral(UserDefinedLiteral *S) {



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

[PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Greg Clayton via Phabricator via cfe-commits]

clayborg added a comment.

In D82477#2145565 , @MaskRay wrote:

> Hi, your git commit contains extra Phabricator tags. You can drop 
> `Reviewers:` `Subscribers:` `Tags:` and the text `Summary:` from the git 
> commit with the following script:
>
>   arcfilter () {
>   arc amend
>   git log -1 --pretty=%B | awk '/Reviewers:|Subscribers:/{p=1} 
> /Reviewed By:|Differential Revision:/{p=0} !p && !/^Summary:$/ 
> {sub(/^Summary: /,"");print}' | git commit --amend --date=now -F -
>   }
>   
>
> `Reviewed By: ` is considered important by some people. Please keep the tag. 
> (`--date=now` is my personal preference (author dates are usually not useful. 
> Using committer dates can make log almost monotonic in time))
>
> `llvm/utils/git/pre-push.py` can validate the message does not include 
> unneeded tags.


Can we modify this script to remove the unneeded tags instead of detecting it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477



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

[PATCH] D82930: [HIP] Fix rocm detection

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D82930#2145827 , @njames93 wrote:

> This is causing a test case to fail on mac 
> http://45.33.8.238/mac/17048/step_7.txt


5d2c3e031a6861b3e95673d0e238c09938dd9c0d 
 should 
fix it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82930



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

[PATCH] D82930: [HIP] Fix rocm detection

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D82930#2145914 , @thakis wrote:

> This breaks check-clang on macOS: http://45.33.8.238/mac/17053/step_7.txt
>
> The output contains
>
>   5: clang: warning: argument unused during compilation: 
> '--rocm-path=/Users/thakis/src/llvm-project/clang/test/Driver/Inputs/rocm' 
> [-Wunused-command-line-argument]
>   
>
> which if true would explain the failure.
>
> That bot uses the GN build, but I verified that the test fails the same way 
> in the CMake build.
>
> Please take a look, and if it takes a while to fix, please revert while you 
> investigate.


should be fixed by 5d2c3e031a6861b3e95673d0e238c09938dd9c0d 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82930



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

[clang] 5d2c3e0 - Fix regression due to test hip-version.hip

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2020-07-11T12:45:29-04:00
New Revision: 5d2c3e031a6861b3e95673d0e238c09938dd9c0d

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

LOG: Fix regression due to test hip-version.hip

Added RocmInstallationDetector to Darwin and MinGW.

Fixed duplicate ROCm detector in ROCm toolchain.

Added: 


Modified: 
clang/lib/Driver/ToolChains/AMDGPU.cpp
clang/lib/Driver/ToolChains/AMDGPU.h
clang/lib/Driver/ToolChains/Darwin.cpp
clang/lib/Driver/ToolChains/Darwin.h
clang/lib/Driver/ToolChains/FreeBSD.cpp
clang/lib/Driver/ToolChains/FreeBSD.h
clang/lib/Driver/ToolChains/HIP.cpp
clang/lib/Driver/ToolChains/MinGW.cpp
clang/lib/Driver/ToolChains/MinGW.h
clang/test/Driver/hip-version.hip

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp 
b/clang/lib/Driver/ToolChains/AMDGPU.cpp
index cfc71d7810b4..bc6d1fcd4a00 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -489,9 +489,9 @@ bool AMDGPUToolChain::isWave64(const llvm::opt::ArgList 
,
 /// ROCM Toolchain
 ROCMToolChain::ROCMToolChain(const Driver , const llvm::Triple ,
  const ArgList )
-: AMDGPUToolChain(D, Triple, Args),
-  RocmInstallation(D, Triple, Args, /*DetectHIPRuntime=*/false,
-   /*DetectDeviceLib=*/true) {}
+: AMDGPUToolChain(D, Triple, Args) {
+  RocmInstallation.detectDeviceLibrary();
+}
 
 void AMDGPUToolChain::addClangTargetOptions(
 const llvm::opt::ArgList ,

diff  --git a/clang/lib/Driver/ToolChains/AMDGPU.h 
b/clang/lib/Driver/ToolChains/AMDGPU.h
index 71c66188b045..5d44faf28b05 100644
--- a/clang/lib/Driver/ToolChains/AMDGPU.h
+++ b/clang/lib/Driver/ToolChains/AMDGPU.h
@@ -90,9 +90,6 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public 
Generic_ELF {
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
-protected:
-  RocmInstallationDetector RocmInstallation;
-
 public:
   ROCMToolChain(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );

diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index 6bf42e6029eb..2e1190c34ea7 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -779,7 +779,7 @@ MachO::MachO(const Driver , const llvm::Triple , 
const ArgList )
 /// Darwin - Darwin tool chain for i386 and x86_64.
 Darwin::Darwin(const Driver , const llvm::Triple , const ArgList 
)
 : MachO(D, Triple, Args), TargetInitialized(false),
-  CudaInstallation(D, Triple, Args) {}
+  CudaInstallation(D, Triple, Args), RocmInstallation(D, Triple, Args) {}
 
 types::ID MachO::LookupTypeForExtension(StringRef Ext) const {
   types::ID Ty = ToolChain::LookupTypeForExtension(Ext);
@@ -831,6 +831,11 @@ void Darwin::AddCudaIncludeArgs(const ArgList ,
   CudaInstallation.AddCudaIncludeArgs(DriverArgs, CC1Args);
 }
 
+void Darwin::AddHIPIncludeArgs(const ArgList ,
+   ArgStringList ) const {
+  RocmInstallation.AddHIPIncludeArgs(DriverArgs, CC1Args);
+}
+
 // This is just a MachO name translation routine and there's no
 // way to join this into ARMTargetParser without breaking all
 // other assumptions. Maybe MachO should consider standardising
@@ -2736,4 +2741,5 @@ SanitizerMask Darwin::getSupportedSanitizers() const {
 
 void Darwin::printVerboseInfo(raw_ostream ) const {
   CudaInstallation.print(OS);
+  RocmInstallation.print(OS);
 }

diff  --git a/clang/lib/Driver/ToolChains/Darwin.h 
b/clang/lib/Driver/ToolChains/Darwin.h
index a543a8fc27b9..64c252efea7d 100644
--- a/clang/lib/Driver/ToolChains/Darwin.h
+++ b/clang/lib/Driver/ToolChains/Darwin.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_DARWIN_H
 
 #include "Cuda.h"
+#include "ROCm.h"
 #include "clang/Driver/DarwinSDKInfo.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
@@ -293,6 +294,7 @@ class LLVM_LIBRARY_VISIBILITY Darwin : public MachO {
   mutable Optional SDKInfo;
 
   CudaInstallationDetector CudaInstallation;
+  RocmInstallationDetector RocmInstallation;
 
 private:
   void AddDeploymentTarget(llvm::opt::DerivedArgList ) const;
@@ -475,6 +477,8 @@ class LLVM_LIBRARY_VISIBILITY Darwin : public MachO {
 
   void AddCudaIncludeArgs(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
+  void AddHIPIncludeArgs(const llvm::opt::ArgList ,
+ llvm::opt::ArgStringList ) const override;
 
   bool UseObjCMixedDispatch() const override {
 // This is only used with the non-fragile ABI and non-legacy dispatch.

diff  --git a/clang/lib/Driver/ToolChains/FreeBSD.cpp 
b/clang/lib/Driver/ToolChains/FreeBSD.cpp

[PATCH] D82930: [HIP] Fix rocm detection

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

…oh, njames said that already 5h ago :) I guess I'll wait another hour or two, 
and then I'll revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82930



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

[PATCH] D82930: [HIP] Fix rocm detection

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks check-clang on macOS: http://45.33.8.238/mac/17053/step_7.txt

The output contains

  5: clang: warning: argument unused during compilation: 
'--rocm-path=/Users/thakis/src/llvm-project/clang/test/Driver/Inputs/rocm' 
[-Wunused-command-line-argument]

which if true would explain the failure.

That bot uses the GN build, but I verified that the test fails the same way in 
the CMake build.

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




Comment at: clang/test/Driver/Inputs/rocm/bin/.hipVersion:1
+# Auto-generated by cmake
+HIP_VERSION_MAJOR=3

Is this true? I don't see any cmake code generating this. Did I miss it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82930



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

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

Also I think that `FileMatchTrie` introduced in https://reviews.llvm.org/D30 is 
not efficient solution to fight with symlinks and for most cases 
`InterpolatingCompilationDatabase` will be accurate enough. But I am not sure, 
is it safe to completely remove `FileMatchTrie`? What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83621



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

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, ilya-biryukov.
Herald added a project: clang.

If there is no record in compile_commands.json, we try to find suitable record 
with `MatchTrie.findEquivalent()` call.
This is very expensive operation with a lot of `llvm::sys::fs::equivalent()` 
calls in some cases.

This patch adds caching for `MatchTrie.findEquivalent()` call result.

Example scenario without this patch:

- compile_commands.json generated at clangd build (contains ~3000 files)..
- it tooks more than 1 second to get compile command for newly created file in 
the root folder of LLVM project.
- we wait for 1 second every time when clangd requests compile command for this 
file (at file change).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83621

Files:
  clang/include/clang/Tooling/JSONCompilationDatabase.h
  clang/lib/Tooling/JSONCompilationDatabase.cpp


Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -230,14 +230,28 @@
   SmallString<128> NativeFilePath;
   llvm::sys::path::native(FilePath, NativeFilePath);
 
-  std::string Error;
-  llvm::raw_string_ostream ES(Error);
-  StringRef Match = MatchTrie.findEquivalent(NativeFilePath, ES);
-  if (Match.empty())
-return {};
-  const auto CommandsRefI = IndexByFile.find(Match);
-  if (CommandsRefI == IndexByFile.end())
-return {};
+  // Avoid usage of `MatchTrie` if possible.
+  auto CommandsRefI = IndexByFile.find(NativeFilePath);
+  if (CommandsRefI == IndexByFile.end()) {
+llvm::StringRef Match;
+// Try to get cached value.
+auto MatchIt = MatchCache.find(NativeFilePath);
+if (MatchIt == MatchCache.end()) {
+  std::string Error;
+  llvm::raw_string_ostream ES(Error);
+  Match = MatchTrie.findEquivalent(NativeFilePath, ES);
+  // Save into cache even if the match result is empty.
+  MatchCache[NativeFilePath] = Match;
+} else {
+  // Cached value.
+  Match = MatchIt->second;
+}
+if (Match.empty())
+  return {};
+CommandsRefI = IndexByFile.find(Match);
+if (CommandsRefI == IndexByFile.end())
+  return {};
+  }
   std::vector Commands;
   getCommands(CommandsRefI->getValue(), Commands);
   return Commands;
Index: clang/include/clang/Tooling/JSONCompilationDatabase.h
===
--- clang/include/clang/Tooling/JSONCompilationDatabase.h
+++ clang/include/clang/Tooling/JSONCompilationDatabase.h
@@ -129,6 +129,8 @@
   std::vector AllCommands;
 
   FileMatchTrie MatchTrie;
+  // Cache for `MatchTrie`.
+  mutable llvm::StringMap MatchCache;
 
   std::unique_ptr Database;
   JSONCommandLineSyntax Syntax;


Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -230,14 +230,28 @@
   SmallString<128> NativeFilePath;
   llvm::sys::path::native(FilePath, NativeFilePath);
 
-  std::string Error;
-  llvm::raw_string_ostream ES(Error);
-  StringRef Match = MatchTrie.findEquivalent(NativeFilePath, ES);
-  if (Match.empty())
-return {};
-  const auto CommandsRefI = IndexByFile.find(Match);
-  if (CommandsRefI == IndexByFile.end())
-return {};
+  // Avoid usage of `MatchTrie` if possible.
+  auto CommandsRefI = IndexByFile.find(NativeFilePath);
+  if (CommandsRefI == IndexByFile.end()) {
+llvm::StringRef Match;
+// Try to get cached value.
+auto MatchIt = MatchCache.find(NativeFilePath);
+if (MatchIt == MatchCache.end()) {
+  std::string Error;
+  llvm::raw_string_ostream ES(Error);
+  Match = MatchTrie.findEquivalent(NativeFilePath, ES);
+  // Save into cache even if the match result is empty.
+  MatchCache[NativeFilePath] = Match;
+} else {
+  // Cached value.
+  Match = MatchIt->second;
+}
+if (Match.empty())
+  return {};
+CommandsRefI = IndexByFile.find(Match);
+if (CommandsRefI == IndexByFile.end())
+  return {};
+  }
   std::vector Commands;
   getCommands(CommandsRefI->getValue(), Commands);
   return Commands;
Index: clang/include/clang/Tooling/JSONCompilationDatabase.h
===
--- clang/include/clang/Tooling/JSONCompilationDatabase.h
+++ clang/include/clang/Tooling/JSONCompilationDatabase.h
@@ -129,6 +129,8 @@
   std::vector AllCommands;
 
   FileMatchTrie MatchTrie;
+  // Cache for `MatchTrie`.
+  mutable llvm::StringMap MatchCache;
 
   std::unique_ptr Database;
   JSONCommandLineSyntax Syntax;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82930: [HIP] Fix rocm detection

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.
Herald added a subscriber: ormris.

This is causing a test case to fail on mac 
http://45.33.8.238/mac/17048/step_7.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82930



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

[PATCH] D82085: [TRE] allow TRE for non-capturing calls.

[Alexey Lapshin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf7907e9d223d: [TRE] allow TRE for non-capturing calls. 
(authored by avl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82085

Files:
  llvm/lib/Transforms/Scalar/TailRecursionElimination.cpp
  llvm/test/Transforms/TailCallElim/basic.ll
  llvm/test/Transforms/TailCallElim/tre-multiple-exits.ll
  llvm/test/Transforms/TailCallElim/tre-noncapturing-alloca-calls.ll

Index: llvm/test/Transforms/TailCallElim/tre-noncapturing-alloca-calls.ll
===
--- /dev/null
+++ llvm/test/Transforms/TailCallElim/tre-noncapturing-alloca-calls.ll
@@ -0,0 +1,74 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -tailcallelim -verify-dom-info -S | FileCheck %s
+
+; IR for that test was generated from the following C++ source:
+;
+;int count;
+;__attribute__((noinline)) void globalIncrement(const int* param) { count += *param; }
+;
+;void test(int recurseCount)
+;{
+;if (recurseCount == 0) return;
+;int temp = 10;
+;globalIncrement();
+;test(recurseCount - 1);
+;}
+;
+
+@count = dso_local local_unnamed_addr global i32 0, align 4
+
+; Function Attrs: nofree noinline norecurse nounwind uwtable
+declare void @_Z15globalIncrementPKi(i32* nocapture readonly %param) #0
+
+; Test that TRE could be done for recursive tail routine containing
+; call to function receiving a pointer to local stack.
+
+; Function Attrs: nounwind uwtable
+define dso_local void @_Z4testi(i32 %recurseCount) local_unnamed_addr #1 {
+; CHECK-LABEL: @_Z4testi(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[TEMP:%.*]] = alloca i32, align 4
+; CHECK-NEXT:br label [[TAILRECURSE:%.*]]
+; CHECK:   tailrecurse:
+; CHECK-NEXT:[[RECURSECOUNT_TR:%.*]] = phi i32 [ [[RECURSECOUNT:%.*]], [[ENTRY:%.*]] ], [ [[SUB:%.*]], [[IF_END:%.*]] ]
+; CHECK-NEXT:[[CMP:%.*]] = icmp eq i32 [[RECURSECOUNT_TR]], 0
+; CHECK-NEXT:br i1 [[CMP]], label [[RETURN:%.*]], label [[IF_END]]
+; CHECK:   if.end:
+; CHECK-NEXT:[[TMP0:%.*]] = bitcast i32* [[TEMP]] to i8*
+; CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull [[TMP0]])
+; CHECK-NEXT:store i32 10, i32* [[TEMP]], align 4
+; CHECK-NEXT:call void @_Z15globalIncrementPKi(i32* nonnull [[TEMP]])
+; CHECK-NEXT:[[SUB]] = add nsw i32 [[RECURSECOUNT_TR]], -1
+; CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull [[TMP0]])
+; CHECK-NEXT:br label [[TAILRECURSE]]
+; CHECK:   return:
+; CHECK-NEXT:ret void
+;
+entry:
+  %temp = alloca i32, align 4
+  %cmp = icmp eq i32 %recurseCount, 0
+  br i1 %cmp, label %return, label %if.end
+
+if.end:   ; preds = %entry
+  %0 = bitcast i32* %temp to i8*
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* nonnull %0) #6
+  store i32 10, i32* %temp, align 4
+  call void @_Z15globalIncrementPKi(i32* nonnull %temp)
+  %sub = add nsw i32 %recurseCount, -1
+  call void @_Z4testi(i32 %sub)
+  call void @llvm.lifetime.end.p0i8(i64 4, i8* nonnull %0) #6
+  br label %return
+
+return:   ; preds = %entry, %if.end
+  ret void
+}
+
+; Function Attrs: argmemonly nounwind willreturn
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #2
+
+; Function Attrs: argmemonly nounwind willreturn
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #2
+
+attributes #0 = { nofree noinline norecurse nounwind uwtable }
+attributes #1 = { nounwind uwtable }
+attributes #2 = { argmemonly nounwind willreturn }
Index: llvm/test/Transforms/TailCallElim/tre-multiple-exits.ll
===
--- /dev/null
+++ llvm/test/Transforms/TailCallElim/tre-multiple-exits.ll
@@ -0,0 +1,125 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -tailcallelim -verify-dom-info -S | FileCheck %s
+
+; This test checks that TRE would be done for only one recursive call.
+; The test_multiple_exits function has three recursive calls.
+; First recursive call could not be eliminated because there is
+; escaped pointer to local variable. Second recursive call could
+; be eliminated. Thrid recursive call could not be eliminated since
+; this is not last call. Thus, test checks that TRE would be done
+; for only second recursive call.
+
+; IR for that test was generated from the following C++ source:
+;
+; void capture_arg (int*);
+; void test_multiple_exits (int param);
+;   if (param >= 0 && param < 10) {
+; int temp;
+; capture_arg();
+; // TRE could not be done because pointer to local
+; // variable "temp" is escaped.
+; test_multiple_exits(param + 1);
+;   } else if (param >=10 && param < 20) {
+; // TRE should be done.
+; test_multiple_exits(param + 1);
+;   } else if 

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc3bdc9814d94: [clang-tidy] Reworked enum options 
handling(again) (authored by njames93).

Changed prior to commit:
  https://reviews.llvm.org/D82188?vs=275520=277227#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82188

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp
@@ -6,6 +6,20 @@
 
 namespace clang {
 namespace tidy {
+
+enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
+
+template <> struct OptionEnumMapping {
+  static llvm::ArrayRef> getEnumMapping() {
+static constexpr std::pair Mapping[] = {
+{Colours::Red, "Red"},   {Colours::Orange, "Orange"},
+{Colours::Yellow, "Yellow"}, {Colours::Green, "Green"},
+{Colours::Blue, "Blue"}, {Colours::Indigo, "Indigo"},
+{Colours::Violet, "Violet"}};
+return makeArrayRef(Mapping);
+  }
+};
+
 namespace test {
 
 TEST(ParseLineFilter, EmptyFilter) {
@@ -208,16 +222,10 @@
 #undef CHECK_ERROR_INT
 }
 
+// FIXME: Figure out why this test causes crashes on mac os.
+#ifndef __APPLE__
 TEST(ValidConfiguration, ValidEnumOptions) {
 
-  enum class Colours { Red, Orange, Yellow, Green, Blue, Indigo, Violet };
-  static constexpr std::pair Mapping[] = {
-  {"Red", Colours::Red},   {"Orange", Colours::Orange},
-  {"Yellow", Colours::Yellow}, {"Green", Colours::Green},
-  {"Blue", Colours::Blue}, {"Indigo", Colours::Indigo},
-  {"Violet", Colours::Violet}};
-  static const auto Map = makeArrayRef(Mapping);
-
   ClangTidyOptions Options;
   auto  = Options.CheckOptions;
 
@@ -237,34 +245,37 @@
 #define CHECK_ERROR_ENUM(Name, Expected)   \
   CHECK_ERROR(Name, UnparseableEnumOptionError, Expected)
 
-  CHECK_VAL(TestCheck.getLocal("Valid", Map), Colours::Red);
-  CHECK_VAL(TestCheck.getGlobal("GlobalValid", Map), Colours::Violet);
-  CHECK_VAL(TestCheck.getLocal("ValidWrongCase", Map, /*IgnoreCase*/ true),
-Colours::Red);
+  CHECK_VAL(TestCheck.getIntLocal("Valid"), Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValid"), Colours::Violet);
+
   CHECK_VAL(
-  TestCheck.getGlobal("GlobalValidWrongCase", Map, /*IgnoreCase*/ true),
-  Colours::Violet);
-  CHECK_ERROR_ENUM(TestCheck.getLocal("Invalid", Map),
+  TestCheck.getIntLocal("ValidWrongCase", /*IgnoreCase*/ true),
+  Colours::Red);
+  CHECK_VAL(TestCheck.getIntGlobal("GlobalValidWrongCase",
+/*IgnoreCase*/ true),
+Colours::Violet);
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("Invalid"),
"invalid configuration value "
"'Scarlet' for option 'test.Invalid'");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("ValidWrongCase", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("ValidWrongCase"),
"invalid configuration value 'rED' for option "
"'test.ValidWrongCase'; did you mean 'Red'?");
-  CHECK_ERROR_ENUM(TestCheck.getLocal("NearMiss", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntLocal("NearMiss"),
"invalid configuration value 'Oragne' for option "
"'test.NearMiss'; did you mean 'Orange'?");
-  CHECK_ERROR_ENUM(TestCheck.getGlobal("GlobalInvalid", Map),
+  CHECK_ERROR_ENUM(TestCheck.getIntGlobal("GlobalInvalid"),
"invalid configuration value "
   

[clang-tools-extra] c3bdc98 - [clang-tidy] Reworked enum options handling(again)

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-07-11T10:13:20+01:00
New Revision: c3bdc9814d947946bf8e1062f6bf41b7f8813f80

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

LOG: [clang-tidy] Reworked enum options handling(again)

Reland b9306fd after fixing the issue causing mac builds to fail unittests.

Following on from D77085, I was never happy with the passing a mapping to the 
option get/store functions. This patch addresses this by using explicit 
specializations to handle the serializing and deserializing of enum options.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
clang-tools-extra/clang-tidy/ClangTidyCheck.h
clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp
clang-tools-extra/clang-tidy/utils/IncludeSorter.h
clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
clang-tools-extra/unittests/clang-tidy/ClangTidyOptionsTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
index 780a3569afdb..e149978bcdea 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -161,11 +161,13 @@ void 
ClangTidyCheck::OptionsView::store(ClangTidyOptions::OptionMap ,
   store(Options, LocalName, llvm::itostr(Value));
 }
 
-llvm::Expected ClangTidyCheck::OptionsView::getEnumInt(
-StringRef LocalName, ArrayRef> Mapping,
-bool CheckGlobal, bool IgnoreCase) {
-  auto Iter = CheckGlobal ? findPriorityOption(CheckOptions, NamePrefix, 
LocalName)
-  : CheckOptions.find((NamePrefix + LocalName).str());
+llvm::Expected
+ClangTidyCheck::OptionsView::getEnumInt(StringRef LocalName,
+ArrayRef Mapping,
+bool CheckGlobal, bool IgnoreCase) {
+  auto Iter = CheckGlobal
+  ? findPriorityOption(CheckOptions, NamePrefix, LocalName)
+  : CheckOptions.find((NamePrefix + LocalName).str());
   if (Iter == CheckOptions.end())
 return llvm::make_error((NamePrefix + 
LocalName).str());
 
@@ -174,19 +176,19 @@ llvm::Expected 
ClangTidyCheck::OptionsView::getEnumInt(
   unsigned EditDistance = -1;
   for (const auto  : Mapping) {
 if (IgnoreCase) {
-  if (Value.equals_lower(NameAndEnum.first))
-return NameAndEnum.second;
-} else if (Value.equals(NameAndEnum.first)) {
-  return NameAndEnum.second;
-} else if (Value.equals_lower(NameAndEnum.first)) {
-  Closest = NameAndEnum.first;
+  if (Value.equals_lower(NameAndEnum.second))
+return NameAndEnum.first;
+} else if (Value.equals(NameAndEnum.second)) {
+  return NameAndEnum.first;
+} else if (Value.equals_lower(NameAndEnum.second)) {
+  Closest = NameAndEnum.second;
   EditDistance = 0;
   continue;
 }
-unsigned Distance = Value.edit_distance(NameAndEnum.first);
+unsigned Distance = Value.edit_distance(NameAndEnum.second);
 if (Distance < EditDistance) {
   EditDistance = Distance;
-  Closest = NameAndEnum.first;
+  Closest = NameAndEnum.second;
 }
   }
   if (EditDistance < 3)

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h 
b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
index dfe01a8aaa30..3c625ee0cb79 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -26,6 +26,13 @@ class SourceManager;
 
 namespace tidy {
 
+/// This class should be specialized by any enum type that needs to be 
converted
+/// to and from an \ref llvm::StringRef.
+template  struct OptionEnumMapping {
+  // Specializations of this struct must implement this function.
+  static ArrayRef> 

[PATCH] D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

Seems like a bug in instsimplify:

  define i1 @f(i32 %x, i32 %y) {
%cmp9.not.1 = icmp eq i32 %x, %y
%cmp15  = icmp slt i32 %x, %y
%spec.select39 = select i1 %cmp9.not.1, i1 undef, i1 %cmp15
%spec.select40 = xor i1 %cmp9.not.1, 1
%spec.select  = and i1 %spec.select39, %spec.select40
ret i1 %spec.select
  }
  =>
  define i1 @f(i32 %x, i32 %y) {
%cmp9.not.1 = icmp eq i32 %x, %y
%cmp15 = icmp slt i32 %x, %y
%spec.select39 = select i1 %cmp9.not.1, i1 undef, i1 %cmp15
ret i1 %spec.select39
  }

https://godbolt.org/z/a8f7hT

Alive2 says it's incorrect: https://alive2.llvm.org/ce/z/-8Q4HL

Seems to be related with ValueTracking's isImpliedCondition since this 
optimizations happens only when operands of the two icmps are the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83360



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