[PATCH] D51200: Introduce per-callsite inline intrinsics

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/CodeGen/CGFunctionInfo.h:517
 
+  unsigned InlineCall : 2;
+

xbolva00 wrote:
> rjmccall wrote:
> > Please don't propagate this information through the `CGFunctionInfo`.  I 
> > really want this type to be *less* site-specific, not *more*.
> I like this patch but sadly, it was abandoned.
> 
> What way of passing this info would you recommend?
I don't remember much about this anymore, but CGCall is perfectly capable of 
adding attributes based on call-site-specific information.


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

https://reviews.llvm.org/D51200



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 2 inline comments as done.
cchen added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7246-7248
+// Set implicit behavior except for "default" for defaultmap
+if ((Bits & OMP_MAP_IMPLICIT) &&
+(ImplicitBehavior != OMPC_DEFAULTMAP_MODIFIER_default)) {

ABataev wrote:
> Hmm, this is strange, Do we really need this kind of processing here? The 
> variables must be mapped implicitly in Sema and, thus, all this processing of 
> the default mapping rules should not be required.
I'm now having design question about setting the correct implicit map type in 
Sema for the below situation:
```
int *ptr_1, *ptr_2, arr[50];
#pragma omp target defaultmap(alloc:pointer) defaultmap(from:aggregate)
{
  ptr_1++, ptr_2++;
  arr[0]++;
}
```
In this case we need to store two maptypes - alloc and from for an 
`ActOnOpenMPMapClause` but `ActOnOpenMPMapClause` only pass one maptype so I'm 
wondering should I modify the interface of OMPMapClause which pass an array of 
maptypes rather than one maptype variable?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:149
 SourceLocation DefaultAttrLoc;
-DefaultMapAttributes DefaultMapAttr = DMA_unspecified;
-SourceLocation DefaultMapAttrLoc;
+DefaultmapInfo DefaultmapMap[3];
+

ABataev wrote:
> ABataev wrote:
> > Maybe, it would be better to make `DMVC_unspecified` the last one in the 
> > `DefaultMapVariableCategory` and use it as an array dimension here rather 
> > than rely on the magical number?
> Not done.
Not sure about this one. I've already put DMVC_unspecified to the last one in 
the DefaultMapVariableCategory enum so that I now don't need magic number. Or 
you are pointing something else? Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[clang] abc04ff - [analyzer] Require darwin for scan-build tests

[Devin Coughlin via cfe-commits]

Author: Devin Coughlin
Date: 2019-11-04T21:17:55-08:00
New Revision: abc04ff4012c62c98aa9f0d840114b2f56855dc8

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

LOG: [analyzer] Require darwin for scan-build tests

Let's at least get some coverage from these tests. We can generalize to
other platforms later.

Added: 


Modified: 
clang/test/Analysis/scan-build/exclude_directories.test
clang/test/Analysis/scan-build/help.test
clang/test/Analysis/scan-build/html_output.test
clang/test/Analysis/scan-build/plist_html_output.test
clang/test/Analysis/scan-build/plist_output.test

Removed: 




diff  --git a/clang/test/Analysis/scan-build/exclude_directories.test 
b/clang/test/Analysis/scan-build/exclude_directories.test
index 2511a42312bd..3db68d635763 100644
--- a/clang/test/Analysis/scan-build/exclude_directories.test
+++ b/clang/test/Analysis/scan-build/exclude_directories.test
@@ -1,4 +1,4 @@
-XFAIL: x86_64-scei-ps4
+REQUIRES: system-darwin
 
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -o %t.output_dir %clang \

diff  --git a/clang/test/Analysis/scan-build/help.test 
b/clang/test/Analysis/scan-build/help.test
index c553d6b1b890..ce4696e0380d 100644
--- a/clang/test/Analysis/scan-build/help.test
+++ b/clang/test/Analysis/scan-build/help.test
@@ -1,3 +1,5 @@
+REQUIRES: system-darwin
+
 RUN: %scan-build -h | FileCheck %s
 RUN: %scan-build --help | FileCheck %s
 

diff  --git a/clang/test/Analysis/scan-build/html_output.test 
b/clang/test/Analysis/scan-build/html_output.test
index 20347900c9bb..f2345fa06062 100644
--- a/clang/test/Analysis/scan-build/html_output.test
+++ b/clang/test/Analysis/scan-build/html_output.test
@@ -1,4 +1,4 @@
-XFAIL: x86_64-scei-ps4
+REQUIRES: system-darwin
 
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -o %t.output_dir %clang %S/Inputs/single_null_dereference.c \

diff  --git a/clang/test/Analysis/scan-build/plist_html_output.test 
b/clang/test/Analysis/scan-build/plist_html_output.test
index ec059460a395..feefa25617d3 100644
--- a/clang/test/Analysis/scan-build/plist_html_output.test
+++ b/clang/test/Analysis/scan-build/plist_html_output.test
@@ -1,4 +1,4 @@
-XFAIL: x86_64-scei-ps4
+REQUIRES: system-darwin
 
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -plist-html -o %t.output_dir %clang 
%S/Inputs/single_null_dereference.c \

diff  --git a/clang/test/Analysis/scan-build/plist_output.test 
b/clang/test/Analysis/scan-build/plist_output.test
index 9812cbdcc332..ffef11656317 100644
--- a/clang/test/Analysis/scan-build/plist_output.test
+++ b/clang/test/Analysis/scan-build/plist_output.test
@@ -1,4 +1,4 @@
-XFAIL: x86_64-scei-ps4
+REQUIRES: system-darwin
 
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -plist -o %t.output_dir %clang 
%S/Inputs/single_null_dereference.c \



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

[clang] 48223d9 - [analyzer] Fixup scan-build tests for non-Darwin platforms.

[Devin Coughlin via cfe-commits]

Author: Devin Coughlin
Date: 2019-11-04T21:12:11-08:00
New Revision: 48223d92a98e2eb7da6844d56471953f83da191e

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

LOG: [analyzer] Fixup scan-build tests for non-Darwin platforms.

This is a fix to 0aba69eb1a01c44185009f50cc633e3c648e9950 to
address failing bots.

Added: 


Modified: 
clang/test/Analysis/scan-build/exclude_directories.test
clang/test/Analysis/scan-build/help.test
clang/test/Analysis/scan-build/html_output.test
clang/test/Analysis/scan-build/plist_html_output.test
clang/test/Analysis/scan-build/plist_output.test

Removed: 




diff  --git a/clang/test/Analysis/scan-build/exclude_directories.test 
b/clang/test/Analysis/scan-build/exclude_directories.test
index db53a34aa65d..2511a42312bd 100644
--- a/clang/test/Analysis/scan-build/exclude_directories.test
+++ b/clang/test/Analysis/scan-build/exclude_directories.test
@@ -1,3 +1,5 @@
+XFAIL: x86_64-scei-ps4
+
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -o %t.output_dir %clang \
 RUN: %S/Inputs/multidirectory_project/directory1/file1.c \

diff  --git a/clang/test/Analysis/scan-build/help.test 
b/clang/test/Analysis/scan-build/help.test
index 4d1972b70653..c553d6b1b890 100644
--- a/clang/test/Analysis/scan-build/help.test
+++ b/clang/test/Analysis/scan-build/help.test
@@ -11,7 +11,6 @@ CHECK: USAGE: scan-build [options]  [build 
options]
 CHECK: AVAILABLE CHECKERS:
 ...
 CHECK:optin.performance.GCDAntipattern
-CHECK:  + osx.API
 ...
 
 

diff  --git a/clang/test/Analysis/scan-build/html_output.test 
b/clang/test/Analysis/scan-build/html_output.test
index 07aa481fae16..20347900c9bb 100644
--- a/clang/test/Analysis/scan-build/html_output.test
+++ b/clang/test/Analysis/scan-build/html_output.test
@@ -1,3 +1,5 @@
+XFAIL: x86_64-scei-ps4
+
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -o %t.output_dir %clang %S/Inputs/single_null_dereference.c \
 RUN: | FileCheck %s -check-prefix CHECK-STDOUT

diff  --git a/clang/test/Analysis/scan-build/plist_html_output.test 
b/clang/test/Analysis/scan-build/plist_html_output.test
index ed26b272246e..ec059460a395 100644
--- a/clang/test/Analysis/scan-build/plist_html_output.test
+++ b/clang/test/Analysis/scan-build/plist_html_output.test
@@ -1,3 +1,5 @@
+XFAIL: x86_64-scei-ps4
+
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -plist-html -o %t.output_dir %clang 
%S/Inputs/single_null_dereference.c \
 RUN: | FileCheck %s -check-prefix CHECK-STDOUT

diff  --git a/clang/test/Analysis/scan-build/plist_output.test 
b/clang/test/Analysis/scan-build/plist_output.test
index 1ee331b8d0e6..9812cbdcc332 100644
--- a/clang/test/Analysis/scan-build/plist_output.test
+++ b/clang/test/Analysis/scan-build/plist_output.test
@@ -1,3 +1,5 @@
+XFAIL: x86_64-scei-ps4
+
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -plist -o %t.output_dir %clang 
%S/Inputs/single_null_dereference.c \
 RUN: | FileCheck %s -check-prefix CHECK-STDOUT



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

[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4264e7bbfdb3: [CUDA][HIP] Disable emitting 
llvm.linker.options in device compilation (authored by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D57829?vs=227636=227816#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57829

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCUDA/ms-linker-options.cu
  clang/test/Driver/hip-autolink.hip


Index: clang/test/Driver/hip-autolink.hip
===
--- /dev/null
+++ clang/test/Driver/hip-autolink.hip
@@ -0,0 +1,14 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+//
+// RUN: %clang --target=i386-pc-windows-msvc --cuda-gpu-arch=gfx906 -nogpulib \
+// RUN:   --cuda-device-only -x hip %s -### 2>&1 | FileCheck 
--check-prefix=DEV %s
+// RUN: %clang --target=i386-pc-windows-msvc --cuda-gpu-arch=gfx906 -nogpulib \
+// RUN:   --cuda-host-only -x hip %s -### 2>&1 | FileCheck --check-prefix=HOST 
%s
+
+// DEV: "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// DEV-SAME: "-fno-autolink"
+
+// HOST: "-cc1" "-triple" "i386-pc-windows-msvc{{.*}}"
+// HOST-NOT: "-fno-autolink"
Index: clang/test/CodeGenCUDA/ms-linker-options.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/ms-linker-options.cu
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -fms-extensions -x hip %s \
+// RUN:   -fno-autolink -triple amdgcn-amd-amdhsa \
+// RUN:   | FileCheck -check-prefix=DEV %s
+// RUN: %clang_cc1 -emit-llvm -o - -fms-extensions -x hip %s -triple \
+// RUN:x86_64-pc-windows-msvc | FileCheck -check-prefix=HOST %s
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -fms-extensions %s \
+// RUN:   -fno-autolink -triple amdgcn-amd-amdhsa \
+// RUN:   | FileCheck -check-prefix=DEV %s
+// RUN: %clang_cc1 -emit-llvm -o - -fms-extensions %s -triple \
+// RUN:x86_64-pc-windows-msvc | FileCheck -check-prefix=HOST %s
+
+// DEV-NOT: llvm.linker.options
+// DEV-NOT: llvm.dependent-libraries
+// HOST: lvm.linker.options
+// HOST: "/DEFAULTLIB:libcpmt.lib"
+// HOST: "/FAILIFMISMATCH:\22myLib_version=9\22"
+
+#pragma comment(lib, "libcpmt")
+#pragma detect_mismatch("myLib_version", "9")
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -378,15 +378,20 @@
 CmdArgs.push_back("-fexceptions");
 }
 
-static bool ShouldDisableAutolink(const ArgList , const ToolChain ) {
+static bool ShouldEnableAutolink(const ArgList , const ToolChain ,
+ const JobAction ) {
   bool Default = true;
   if (TC.getTriple().isOSDarwin()) {
 // The native darwin assembler doesn't support the linker_option 
directives,
 // so we disable them if we think the .s file will be passed to it.
 Default = TC.useIntegratedAs();
   }
-  return !Args.hasFlag(options::OPT_fautolink, options::OPT_fno_autolink,
-   Default);
+  // The linker_option directives are intended for host compilation.
+  if (JA.isDeviceOffloading(Action::OFK_Cuda) ||
+  JA.isDeviceOffloading(Action::OFK_HIP))
+Default = false;
+  return Args.hasFlag(options::OPT_fautolink, options::OPT_fno_autolink,
+  Default);
 }
 
 static bool ShouldDisableDwarfDirectory(const ArgList ,
@@ -4391,7 +4396,7 @@
   if (ShouldDisableDwarfDirectory(Args, TC))
 CmdArgs.push_back("-fno-dwarf-directory-asm");
 
-  if (ShouldDisableAutolink(Args, TC))
+  if (!ShouldEnableAutolink(Args, TC, JA))
 CmdArgs.push_back("-fno-autolink");
 
   // Add in -fdebug-compilation-dir if necessary.


Index: clang/test/Driver/hip-autolink.hip
===
--- /dev/null
+++ clang/test/Driver/hip-autolink.hip
@@ -0,0 +1,14 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+//
+// RUN: %clang --target=i386-pc-windows-msvc --cuda-gpu-arch=gfx906 -nogpulib \
+// RUN:   --cuda-device-only -x hip %s -### 2>&1 | FileCheck --check-prefix=DEV %s
+// RUN: %clang --target=i386-pc-windows-msvc --cuda-gpu-arch=gfx906 -nogpulib \
+// RUN:   --cuda-host-only -x hip %s -### 2>&1 | FileCheck --check-prefix=HOST %s
+
+// DEV: "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// DEV-SAME: "-fno-autolink"
+
+// HOST: "-cc1" "-triple" "i386-pc-windows-msvc{{.*}}"
+// HOST-NOT: "-fno-autolink"
Index: clang/test/CodeGenCUDA/ms-linker-options.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/ms-linker-options.cu
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -fms-extensions -x 

[PATCH] D69781: [analyzer] Add test directory for scan-build

[Devin Coughlin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0aba69eb1a01: [analyzer] Add test directory for scan-build. 
(authored by dcoughlin).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69781

Files:
  
clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory1/file1.c
  
clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory2/file2.c
  clang/test/Analysis/scan-build/Inputs/single_null_dereference.c
  clang/test/Analysis/scan-build/exclude_directories.test
  clang/test/Analysis/scan-build/help.test
  clang/test/Analysis/scan-build/html_output.test
  clang/test/Analysis/scan-build/plist_html_output.test
  clang/test/Analysis/scan-build/plist_output.test
  clang/test/lit.cfg.py
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -411,6 +411,7 @@
 ToolSubst('%clang_cpp', command=self.config.clang, extra_args=['--driver-mode=cpp']+additional_flags),
 ToolSubst('%clang_cl', command=self.config.clang, extra_args=['--driver-mode=cl']+additional_flags),
 ToolSubst('%clangxx', command=self.config.clang, extra_args=['--driver-mode=g++']+additional_flags),
+ToolSubst('%scan-build', command='scan-build'),
 ]
 self.add_tool_substitutions(tool_substitutions)
 
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -62,7 +62,7 @@
 
 tools = [
 'c-index-test', 'clang-diff', 'clang-format', 'clang-tblgen', 'opt', 'llvm-ifs',
-ToolSubst('%clang_extdef_map', command=FindTool(
+'scan-build', ToolSubst('%clang_extdef_map', command=FindTool(
 'clang-extdef-mapping'), unresolved='ignore'),
 ]
 
Index: clang/test/Analysis/scan-build/plist_output.test
===
--- /dev/null
+++ clang/test/Analysis/scan-build/plist_output.test
@@ -0,0 +1,20 @@
+RUN: rm -rf %t.output_dir && mkdir %t.output_dir
+RUN: %scan-build -plist -o %t.output_dir %clang %S/Inputs/single_null_dereference.c \
+RUN: | FileCheck %s -check-prefix CHECK-STDOUT
+
+// Test plist output
+
+CHECK-STDOUT: scan-build: Using '{{.*}}' for static analysis
+CHECK-STDOUT: scan-build: Analysis run complete.
+CHECK-STDOUT: scan-build: Analysis results (plist files) deposited in '{{.*}}'
+
+// We expect a single plist file
+RUN: ls %t.output_dir/*/ | FileCheck %s -check-prefix CHECK-FILENAMES
+
+CHECK-FILENAMES: report-{{.*}}.plist
+
+// The report should describe the issue.
+RUN: cat %t.output_dir/*/report-*.plist \
+RUN: | FileCheck %s -check-prefix CHECK-REPORT-PLIST-CONTENTS
+
+CHECK-REPORT-PLIST-CONTENTS: typeDereference of null pointer
Index: clang/test/Analysis/scan-build/plist_html_output.test
===
--- /dev/null
+++ clang/test/Analysis/scan-build/plist_html_output.test
@@ -0,0 +1,20 @@
+RUN: rm -rf %t.output_dir && mkdir %t.output_dir
+RUN: %scan-build -plist-html -o %t.output_dir %clang %S/Inputs/single_null_dereference.c \
+RUN: | FileCheck %s -check-prefix CHECK-STDOUT
+
+// Test combined plist and html output with -plist-html
+
+CHECK-STDOUT: scan-build: Using '{{.*}}' for static analysis
+CHECK-STDOUT: scan-build: Analysis run complete.
+CHECK-STDOUT: scan-build: Analysis results (plist files) deposited in '{{.*}}'
+CHECK-STDOUT: scan-build: 1 bug found.
+CHECK-STDOUT: scan-build: Run 'scan-view {{.*}}' to examine bug reports.
+
+// We expect both html files and the plist files.
+RUN: ls %t.output_dir/*/ | FileCheck %s -check-prefix CHECK-FILENAMES
+
+CHECK-FILENAMES: index.html
+CHECK-FILENAMES-DAG: report-{{.*}}.html
+CHECK-FILENAMES-DAG: report-{{.*}}.plist
+CHECK-FILENAMES: scanview.css
+CHECK-FILENAMES: sorttable.js
Index: clang/test/Analysis/scan-build/html_output.test
===
--- /dev/null
+++ clang/test/Analysis/scan-build/html_output.test
@@ -0,0 +1,30 @@
+RUN: rm -rf %t.output_dir && mkdir %t.output_dir
+RUN: %scan-build -o %t.output_dir %clang %S/Inputs/single_null_dereference.c \
+RUN: | FileCheck %s -check-prefix CHECK-STDOUT
+
+// Test html output
+
+CHECK-STDOUT: scan-build: Using '{{.*}}' for static analysis
+CHECK-STDOUT: scan-build: 1 bug found.
+CHECK-STDOUT: scan-build: Run 'scan-view {{.*}}' to examine bug reports.
+
+// We expect an index file, a file for the report, and sibling support files.
+RUN: ls %t.output_dir/*/ | FileCheck %s -check-prefix CHECK-FILENAMES
+
+CHECK-FILENAMES: index.html
+CHECK-FILENAMES: report-{{.*}}.html
+CHECK-FILENAMES: scanview.css
+CHECK-FILENAMES: sorttable.js
+
+
+// The index should have a link to the report for the 

[clang] 0aba69e - [analyzer] Add test directory for scan-build.

[Devin Coughlin via cfe-commits]

Author: Devin Coughlin
Date: 2019-11-04T20:26:35-08:00
New Revision: 0aba69eb1a01c44185009f50cc633e3c648e9950

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

LOG: [analyzer] Add test directory for scan-build.

The static analyzer's scan-build script is critical infrastructure but
is not well tested. To start to address this, add a new test directory under
tests/Analysis for scan-build lit tests and seed it with several tests. The
goal is that future scan-build changes will be accompanied by corresponding
tests.

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

Added: 

clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory1/file1.c

clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory2/file2.c
clang/test/Analysis/scan-build/Inputs/single_null_dereference.c
clang/test/Analysis/scan-build/exclude_directories.test
clang/test/Analysis/scan-build/help.test
clang/test/Analysis/scan-build/html_output.test
clang/test/Analysis/scan-build/plist_html_output.test
clang/test/Analysis/scan-build/plist_output.test

Modified: 
clang/test/lit.cfg.py
llvm/utils/lit/lit/llvm/config.py

Removed: 




diff  --git 
a/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory1/file1.c
 
b/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory1/file1.c
new file mode 100644
index ..7fffb69e01a0
--- /dev/null
+++ 
b/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory1/file1.c
@@ -0,0 +1,9 @@
+int main() {
+  return 0;
+}
+
+void function1(int *p) {
+  if (!p) {
+*p = 7; // This will emit a null pointer diagnostic.
+  }
+}

diff  --git 
a/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory2/file2.c
 
b/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory2/file2.c
new file mode 100644
index ..ed0e17212337
--- /dev/null
+++ 
b/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory2/file2.c
@@ -0,0 +1,5 @@
+void function2(int *o) {
+  if (!o) {
+*o = 7; // This will emit a null pointer diagnostic.
+  }
+}

diff  --git a/clang/test/Analysis/scan-build/Inputs/single_null_dereference.c 
b/clang/test/Analysis/scan-build/Inputs/single_null_dereference.c
new file mode 100644
index ..21a43dfd08a5
--- /dev/null
+++ b/clang/test/Analysis/scan-build/Inputs/single_null_dereference.c
@@ -0,0 +1,5 @@
+int main() {
+  int *p = 0;
+  *p = 7; // We expect a diagnostic about this.
+  return 0;
+}

diff  --git a/clang/test/Analysis/scan-build/exclude_directories.test 
b/clang/test/Analysis/scan-build/exclude_directories.test
new file mode 100644
index ..db53a34aa65d
--- /dev/null
+++ b/clang/test/Analysis/scan-build/exclude_directories.test
@@ -0,0 +1,34 @@
+RUN: rm -rf %t.output_dir && mkdir %t.output_dir
+RUN: %scan-build -o %t.output_dir %clang \
+RUN: %S/Inputs/multidirectory_project/directory1/file1.c \
+RUN: %S/Inputs/multidirectory_project/directory2/file2.c \
+RUN: | FileCheck %s -check-prefix CHECK-NO-EXCLUDE
+
+// The purpose of this test is to ensure that the --exclude command line option
+// actually excludes reports from inside the specified directories.
+
+
+// First, let's make sure that without --exclude issues in both
+// directory1 and directory2 are found.
+CHECK-NO-EXCLUDE: scan-build: 2 bugs found.
+
+
+// Only one issue should be found when directory1 is excluded.
+RUN: rm -rf %t.output_dir && mkdir %t.output_dir
+RUN: %scan-build -o %t.output_dir --exclude directory1 %clang \
+RUN: %S/Inputs/multidirectory_project/directory1/file1.c \
+RUN: %S/Inputs/multidirectory_project/directory2/file2.c \
+RUN: | FileCheck %s -check-prefix CHECK-EXCLUDE1
+
+CHECK-EXCLUDE1: scan-build: 1 bug found.
+
+
+// When both directories are excluded, no issues should be reported.
+RUN: rm -rf %t.output_dir && mkdir %t.output_dir
+RUN: %scan-build -o %t.output_dir --exclude directory1 --exclude directory2 
%clang \
+RUN: %S/Inputs/multidirectory_project/directory1/file1.c \
+RUN: %S/Inputs/multidirectory_project/directory2/file2.c \
+RUN: | FileCheck %s -check-prefix CHECK-EXCLUDE-BOTH
+
+CHECK-EXCLUDE-BOTH: scan-build: 0 bugs found.
+

diff  --git a/clang/test/Analysis/scan-build/help.test 
b/clang/test/Analysis/scan-build/help.test
new file mode 100644
index ..4d1972b70653
--- /dev/null
+++ b/clang/test/Analysis/scan-build/help.test
@@ -0,0 +1,18 @@
+RUN: %scan-build -h | FileCheck %s
+RUN: %scan-build --help | FileCheck %s
+
+Test for help output from scan-build.
+
+
+CHECK: USAGE: scan-build [options]  [build options]
+
+...
+
+CHECK: AVAILABLE CHECKERS:
+...
+CHECK:optin.performance.GCDAntipattern
+CHECK:  + osx.API
+...
+
+
+

diff  --git 

[clang] 4264e7b - [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2019-11-04T23:21:39-05:00
New Revision: 4264e7bbfdb30ed8fe1e0907bfa25e4d1bb04207

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

LOG: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

The linker options (e.g. pragma detect_mismatch) are intended for host
compilation only, therefore disable it for device compilation.

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

Added: 
clang/test/CodeGenCUDA/ms-linker-options.cu
clang/test/Driver/hip-autolink.hip

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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 369d8cc1aa96..16bf68254d19 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -378,15 +378,20 @@ static void addExceptionArgs(const ArgList , 
types::ID InputType,
 CmdArgs.push_back("-fexceptions");
 }
 
-static bool ShouldDisableAutolink(const ArgList , const ToolChain ) {
+static bool ShouldEnableAutolink(const ArgList , const ToolChain ,
+ const JobAction ) {
   bool Default = true;
   if (TC.getTriple().isOSDarwin()) {
 // The native darwin assembler doesn't support the linker_option 
directives,
 // so we disable them if we think the .s file will be passed to it.
 Default = TC.useIntegratedAs();
   }
-  return !Args.hasFlag(options::OPT_fautolink, options::OPT_fno_autolink,
-   Default);
+  // The linker_option directives are intended for host compilation.
+  if (JA.isDeviceOffloading(Action::OFK_Cuda) ||
+  JA.isDeviceOffloading(Action::OFK_HIP))
+Default = false;
+  return Args.hasFlag(options::OPT_fautolink, options::OPT_fno_autolink,
+  Default);
 }
 
 static bool ShouldDisableDwarfDirectory(const ArgList ,
@@ -4391,7 +4396,7 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
   if (ShouldDisableDwarfDirectory(Args, TC))
 CmdArgs.push_back("-fno-dwarf-directory-asm");
 
-  if (ShouldDisableAutolink(Args, TC))
+  if (!ShouldEnableAutolink(Args, TC, JA))
 CmdArgs.push_back("-fno-autolink");
 
   // Add in -fdebug-compilation-dir if necessary.

diff  --git a/clang/test/CodeGenCUDA/ms-linker-options.cu 
b/clang/test/CodeGenCUDA/ms-linker-options.cu
new file mode 100644
index ..0be25fbbdfd4
--- /dev/null
+++ b/clang/test/CodeGenCUDA/ms-linker-options.cu
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -fms-extensions -x hip %s \
+// RUN:   -fno-autolink -triple amdgcn-amd-amdhsa \
+// RUN:   | FileCheck -check-prefix=DEV %s
+// RUN: %clang_cc1 -emit-llvm -o - -fms-extensions -x hip %s -triple \
+// RUN:x86_64-pc-windows-msvc | FileCheck -check-prefix=HOST %s
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -fms-extensions %s \
+// RUN:   -fno-autolink -triple amdgcn-amd-amdhsa \
+// RUN:   | FileCheck -check-prefix=DEV %s
+// RUN: %clang_cc1 -emit-llvm -o - -fms-extensions %s -triple \
+// RUN:x86_64-pc-windows-msvc | FileCheck -check-prefix=HOST %s
+
+// DEV-NOT: llvm.linker.options
+// DEV-NOT: llvm.dependent-libraries
+// HOST: lvm.linker.options
+// HOST: "/DEFAULTLIB:libcpmt.lib"
+// HOST: "/FAILIFMISMATCH:\22myLib_version=9\22"
+
+#pragma comment(lib, "libcpmt")
+#pragma detect_mismatch("myLib_version", "9")

diff  --git a/clang/test/Driver/hip-autolink.hip 
b/clang/test/Driver/hip-autolink.hip
new file mode 100644
index ..9c1b65f1592b
--- /dev/null
+++ b/clang/test/Driver/hip-autolink.hip
@@ -0,0 +1,14 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+//
+// RUN: %clang --target=i386-pc-windows-msvc --cuda-gpu-arch=gfx906 -nogpulib \
+// RUN:   --cuda-device-only -x hip %s -### 2>&1 | FileCheck 
--check-prefix=DEV %s
+// RUN: %clang --target=i386-pc-windows-msvc --cuda-gpu-arch=gfx906 -nogpulib \
+// RUN:   --cuda-host-only -x hip %s -### 2>&1 | FileCheck --check-prefix=HOST 
%s
+
+// DEV: "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// DEV-SAME: "-fno-autolink"
+
+// HOST: "-cc1" "-triple" "i386-pc-windows-msvc{{.*}}"
+// HOST-NOT: "-fno-autolink"



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

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

[Brian Gesiak via Phabricator via cfe-commits]

modocache updated this revision to Diff 227808.
modocache added a comment.

Rebasing onto the monorepo. @rsmith, I confirmed the test cases that this diff 
adds still fail on trunk, and that the Clang source changes made in this diff 
fix the test case failures. Tomorrow I plan on poking around to see if I can 
reproduce similar issues in the C++20 modules implementation. But in the 
meantime, how do you feel about this patch? You suggested a change to 
`~FindExistingResult` but also that it'd be difficult to test/verify such a 
change. Is that change still something you're looking for before accepting this 
diff?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58920

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/pr39287-2/a.h
  clang/test/Modules/Inputs/pr39287-2/module.modulemap
  clang/test/Modules/Inputs/pr39287/a.h
  clang/test/Modules/Inputs/pr39287/module.modulemap
  clang/test/Modules/pr39287-2.cpp
  clang/test/Modules/pr39287.cpp

Index: clang/test/Modules/pr39287.cpp
===
--- /dev/null
+++ clang/test/Modules/pr39287.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++17 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/pr39287 %s -verify
+
+class A {
+  virtual ~A() {}
+};
+
+#include "a.h"
+
+namespace std { class type_info; }
+
+void foo() {
+  typeid(foo); // expected-warning {{expression result unused}}
+}
Index: clang/test/Modules/pr39287-2.cpp
===
--- /dev/null
+++ clang/test/Modules/pr39287-2.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++17 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/pr39287-2 %s -verify
+
+class A {
+  virtual ~A() {}
+};
+
+#include "a.h"
+
+void foo() {
+  typeid(foo); // expected-warning {{expression result unused}}
+}
Index: clang/test/Modules/Inputs/pr39287/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/pr39287/module.modulemap
@@ -0,0 +1,3 @@
+module "a.h" {
+  header "a.h"
+}
Index: clang/test/Modules/Inputs/pr39287/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/pr39287/a.h
@@ -0,0 +1 @@
+namespace std {}
Index: clang/test/Modules/Inputs/pr39287-2/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/pr39287-2/module.modulemap
@@ -0,0 +1,3 @@
+module "a.h" {
+  header "a.h"
+}
Index: clang/test/Modules/Inputs/pr39287-2/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/pr39287-2/a.h
@@ -0,0 +1 @@
+namespace std { class type_info; }
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -47,6 +47,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Sema/IdentifierResolver.h"
+#include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ContinuousRangeMap.h"
@@ -3195,6 +3196,9 @@
 // Add the declaration to its redeclaration context so later merging
 // lookups will find it.
 MergeDC->makeDeclVisibleInContextImpl(New, /*Internal*/true);
+if (isa(New) && Name.getAsString() == "std")
+  if (!Reader.getSema()->StdNamespace)
+Reader.getSema()->StdNamespace = New;
   }
 }
 
@@ -3358,6 +3362,14 @@
   return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber,
 TypedefNameForLinkage);
 }
+if (isa(D) && D->getName() == "std") {
+  auto StdPtr = Reader.getSema()->StdNamespace;
+  if (StdPtr.isValid() && !StdPtr.isOffset())
+if (auto *Std = cast_or_null(StdPtr.get(nullptr)))
+  if (isSameEntity(Std, D))
+return FindExistingResult(Reader, D, Std, AnonymousDeclNumber,
+  TypedefNameForLinkage);
+}
   } else {
 // Not in a mergeable context.
 return FindExistingResult(Reader);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53482: Add clang-format stability check with FormatTests

[Wouter van Oortmerssen via Phabricator via cfe-commits]

aardappel added a comment.

@krasimir first you say that fixing it is hard, then when someone wants to 
attempt, that it is not necessary?

Plenty of people run `clang-format` in automated fashion as part of some 
pipeline, seems to me results ping-ponging between two formattings and 
cluttering commits would be pretty bad.

I just ran into this bug (https://bugs.llvm.org/show_bug.cgi?id=39280) and now 
instructions to my contributors are going to have to be: please run 
`clang-format` twice :)


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

https://reviews.llvm.org/D53482



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

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

[Warren Ristow via Phabricator via cfe-commits]

wristow added inline comments.



Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:614
   // during ThinLTO and perform the rest of the optimizations afterward.
   if (PrepareForThinLTO) {
 // Ensure we perform any last passes, but do so before renaming anonymous

wenlei wrote:
> this also need to be `PrepareForThinLTO || PrepareForLTO` for oldPM?
I agree this is another instance where a balancing act question applies.  In 
this case, assuming the comment about the concern of code bloat is accurate, 
it's not so much about compile-time resources in the full LTO back-end, but 
rather about minimizing the ThinLTO bitcode write/read time.  So if as this WIP 
evolves, it ultimately is a win for SamplePGO to suppress some loop 
optimizations (unrolling/vectorization) here, then that will probably also be a 
//small // win in full LTO compile time.  That said, in addition to these 
loop-related optimizations, there are other transformations here that are done 
in the full LTO pipeline (but not in the ThinLTO pipeline).  So I suspect if 
some change to check for `PrepareForThinLTO || PrepareForLTO` (rather than only 
`PrepareForThinLTO`) makes sense here from a Sample PGO perspective, then the 
change will be more complicated than simply adding the small set of passes here 
followed by the early return (that is, I think there are probably things after 
the `return` on line 621 that still ought to be enabled for full LTO -- 
essentially continuing to do them in the pre-link stage for full LTO, to try to 
avoid needing to do too much work in the full LTO backend stage, since it's 
more of a problem for the full backend to absorb that compile time cost).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69732



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

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

[Warren Ristow via Phabricator via cfe-commits]

wristow added a comment.

> This probably needs to be taken over by someone who cares about full LTO 
> performance

We at PlayStation are definitely interested in full LTO performance, so we're 
looking into this.  We certainly agree with the rationale that if suppressing 
some optimizations is useful to allow better SamplePGO matching, then we'd 
expect that would apply equally to both ThinLTO and full LTO.

I guess much of this comes down to a balancing act between:

1. The amount of the runtime benefit with Sample PGO if these loop 
optimizations are deferred to the full LTO back-end (like they are for ThinLTO).
2. The cost in compile-time resources in the full LTO back-end to do these loop 
optimizations at that later stage.

From the discussion here, the Sample PGO runtime win (point 1) seems more or 
less to be a given.  If we find the compile-time cost in the full LTO back-end 
(point 2) is not significant, then the decision should be easy.  So after 
seeing this patch, we're doing some experiments to at least try to get a handle 
on this.  (I'm a bit concerned we won't be able to draw any hard conclusions 
from the results of our experiments, but at least we'll be able to make a 
better informed assessment.)

FTR, for PlayStation, we're using the old PM.  But we'll do some experiments 
for both the old and new PM, to get a sense of the answers to the (old PM) 
`LoopUnrollAndJam` point, and the (new PM) FIXME comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69732



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

[PATCH] D68108: Redeclare Objective-C property accessors inside the ObjCImplDecl in which they are synthesized.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 227801.
aprantl added a comment.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.

Accidentally sent my diff-upon-previous-diff instead of diff against master.


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

https://reviews.llvm.org/D68108

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclObjC.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclObjC.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Index/IndexDecl.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaPseudoObject.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/AST/ast-dump-decl-json.m
  clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
  clang/test/CodeGenObjC/debug-info-synthesis.m
  clang/test/CodeGenObjC/debug-property-synth.m
  clang/test/CodeGenObjC/debuginfo-properties.m
  clang/test/CodeGenObjC/instance-method-metadata.m
  clang/test/SemaObjC/iboutlet.m
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -629,6 +629,11 @@
 Decl *D = *I;
 if (D->getLexicalDeclContext() != DC)
   continue;
+// Filter out synthesized property accessor redeclarations.
+if (isa(DC))
+  if (auto *OMD = dyn_cast(D))
+if (OMD->isSynthesizedAccessorStub())
+  continue;
 const Optional V = handleDeclForVisitation(D);
 if (!V.hasValue())
   continue;
Index: clang/test/SemaObjC/iboutlet.m
===
--- clang/test/SemaObjC/iboutlet.m
+++ clang/test/SemaObjC/iboutlet.m
@@ -11,7 +11,7 @@
 
 IBInspectable @property (readonly) IBOutlet NSView *myView1; // expected-warning {{readonly IBOutlet property 'myView1' when auto-synthesized may not work correctly with 'nib' loader}} expected-note {{property should be changed to be readwrite}}
 
-@property (getter = MyGetter, READONLY) IBOutlet NSView *myView2; // expected-warning {{readonly IBOutlet property 'myView2' when auto-synthesized may not work correctly with 'nib' loader}}
+@property (getter = MyGetter2, READONLY) IBOutlet NSView *myView2; // expected-warning {{readonly IBOutlet property 'myView2' when auto-synthesized may not work correctly with 'nib' loader}}
 
 @end
 
Index: clang/test/CodeGenObjC/instance-method-metadata.m
===
--- clang/test/CodeGenObjC/instance-method-metadata.m
+++ clang/test/CodeGenObjC/instance-method-metadata.m
@@ -1,6 +1,5 @@
 // REQUIRES: x86-registered-target
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -S -o %t %s 
-// RUN: FileCheck < %t %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -S %s -o - | FileCheck %s
 
 // rdar://9072317
 
Index: clang/test/CodeGenObjC/debuginfo-properties.m
===
--- clang/test/CodeGenObjC/debuginfo-properties.m
+++ clang/test/CodeGenObjC/debuginfo-properties.m
@@ -11,19 +11,6 @@
 
 @protocol HasASelection 
 @property (nonatomic, retain) Selection* selection;
-// CHECK: !DISubprogram(name: "-[MyClass selection]"
-// CHECK-SAME:  line: [[@LINE-2]]
-// CHECK-SAME:  DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK: !DISubprogram(name: "-[MyClass setSelection:]"
-// CHECK-SAME:  line: [[@LINE-5]]
-// CHECK-SAME:  DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK: !DISubprogram(name: "-[OtherClass selection]"
-// CHECK-SAME:  line: [[@LINE-8]]
-// CHECK-SAME:  DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK: !DISubprogram(name: "-[OtherClass setSelection:]"
-// CHECK-SAME:  line: [[@LINE-11]]
-// CHECK-SAME:  DISPFlagLocalToUnit | DISPFlagDefinition
-
 @end
 
 @interface MyClass : NSObject  {
@@ -33,6 +20,12 @@
 
 @implementation MyClass
 @synthesize selection = _selection;
+// CHECK: !DISubprogram(name: "-[MyClass selection]"
+// CHECK-SAME:  line: [[@LINE-2]]
+// CHECK-SAME:  DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK: !DISubprogram(name: "-[MyClass setSelection:]"
+// CHECK-SAME:  line: [[@LINE-5]]
+// CHECK-SAME:  DISPFlagLocalToUnit | DISPFlagDefinition
 @end
 
 @interface OtherClass : NSObject  {
@@ -41,4 +34,10 @@
 @end
 @implementation OtherClass
 @synthesize selection = _selection;
+// CHECK: !DISubprogram(name: 

[PATCH] D68108: Redeclare Objective-C property accessors inside the ObjCImplDecl in which they are synthesized.

[Adrian Prantl via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5063
  const_cast(D), PID);
 }
   }

rjmccall wrote:
> aprantl wrote:
> > rjmccall wrote:
> > > Is this special treatment still necessary?  Won't we encounter the getter 
> > > and setter on the normal pass over the method definitions in the 
> > > `@implementation`?
> > We don't know which ObjMethodDecls without bodies are property accessor 
> > implementations since there is no pointer from the ObjCMethodDecl back to 
> > the ObjCPropertyImplDecl.
> I mean, this doesn't seem like an unreasonable thing to be able to discover 
> given just an `ObjCMethodDecl`.  We can certainly set something on the 
> declaration that says that it represents a synthesized method definition (and 
> is therefore a definition for the purposes of `isDefined` or anything 
> similar).
If we were to move this into the loop / function that emits each method decl, 
we would still somehow need to get back to the `ObjCPropertyImplDecl `. There 
is no pointer in that direction, so we'd have to loop over all property impls 
until we find it. This here is going to be faster. Storing a pointer to a 
possible `ObjCPropertyImplDecl` in `ObjCMethodDecl` seems wasteful. Let me know 
if I misunderstood what you were suggesting.


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

https://reviews.llvm.org/D68108



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

[PATCH] D68108: Redeclare Objective-C property accessors inside the ObjCImplDecl in which they are synthesized.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 227799.
aprantl added a comment.

Rebased patch and made it slightly more explicit whether an ObjCMethodDecl is a 
synthesized accessor stub by reserving a bit for it in ObjCMethodDecl.


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

https://reviews.llvm.org/D68108

Files:
  clang/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Index/IndexDecl.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -632,7 +632,7 @@
 // Filter out synthesized property accessor redeclarations.
 if (isa(DC))
   if (auto *OMD = dyn_cast(D))
-if (OMD->isImplicit() && !OMD->hasBody())
+if (OMD->isSynthesizedAccessorStub())
   continue;
 const Optional V = handleDeclForVisitation(D);
 if (!V.hasValue())
Index: clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
@@ -144,8 +144,9 @@
   continue;
 
 const Stmt *Body = M->getBody();
-if (!Body)
+if (M->isSynthesizedAccessorStub())
   continue;
+assert(Body);
 
 MethodCrawler MC(IvarToPropMap, M->getCanonicalDecl(), InterD, BR, this,
  DCtx);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1043,7 +1043,6 @@
   ObjCMethodDecl *AccessorDecl, SourceLocation AtLoc,
   SourceLocation PropertyLoc) {
   ObjCMethodDecl *Decl = AccessorDecl;
-  //  llvm::errs()<<"redeclare: ";  Decl->dump();
   ObjCMethodDecl *ImplDecl = ObjCMethodDecl::Create(
   Context, AtLoc, PropertyLoc, Decl->getSelector(), Decl->getReturnType(),
   Decl->getReturnTypeSourceInfo(), Impl, Decl->isInstanceMethod(),
@@ -2145,8 +2144,8 @@
 property->getSetterMethodDecl()) {
   auto *getterImpl = propertyImpl->getGetterMethodDecl();
   auto *setterImpl = propertyImpl->getSetterMethodDecl();
-  if ((!getterImpl || !getterImpl->getBody()) &&
-  (!setterImpl || !setterImpl->getBody())) {
+  if ((!getterImpl || getterImpl->isSynthesizedAccessorStub()) &&
+  (!setterImpl || setterImpl->isSynthesizedAccessorStub())) {
 SourceLocation loc = propertyImpl->getLocation();
 if (loc.isInvalid())
   loc = impDecl->getBeginLoc();
@@ -2189,9 +2188,9 @@
   SetterMethod = Property->isClassProperty() ?
  IMPDecl->getClassMethod(Property->getSetterName()) :
  IMPDecl->getInstanceMethod(Property->getSetterName());
-  if (GetterMethod && !GetterMethod->getBody())
+  if (GetterMethod && GetterMethod->isSynthesizedAccessorStub())
 GetterMethod = nullptr;
-  if (SetterMethod && !SetterMethod->getBody())
+  if (SetterMethod && SetterMethod->isSynthesizedAccessorStub())
 SetterMethod = nullptr;
   LookedUpGetterSetter = true;
   if (GetterMethod) {
@@ -2218,9 +2217,9 @@
 continue;
   GetterMethod = PIDecl->getGetterMethodDecl();
   SetterMethod = PIDecl->getSetterMethodDecl();
-  if (GetterMethod && !GetterMethod->getBody())
+  if (GetterMethod && !GetterMethod->isSynthesizedAccessorStub())
 GetterMethod = nullptr;
-  if (SetterMethod && !SetterMethod->getBody())
+  if (SetterMethod && !SetterMethod->isSynthesizedAccessorStub())
 SetterMethod = nullptr;
   if ((bool)GetterMethod ^ (bool)SetterMethod) {
 SourceLocation MethodLoc =
@@ -2265,7 +2264,7 @@
 if (PD && !PD->hasAttr() &&
 !PD->isClassProperty()) {
   ObjCMethodDecl *IM = PID->getGetterMethodDecl();
-  if (IM && IM->getBody())
+  if (IM && IM->isSynthesizedAccessorStub())
 continue;
   ObjCMethodDecl *method = PD->getGetterMethodDecl();
   if (!method)
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -2829,7 +2829,7 @@
   // ImpMethodDecl may be null as in a @dynamic property.
   if (ImpMethodDecl) {
 // Skip property accessor function stubs.
-if (I->isPropertyAccessor() && !ImpMethodDecl->getBody())
+if (ImpMethodDecl->isSynthesizedAccessorStub())
   continue;
 if (!WarnCategoryMethodImpl)
   WarnConflictingTypedMethods(ImpMethodDecl, I,
@@ -2858,7 

[libunwind] 6db7a5c - build: explicitly set the linker language for unwind

[Saleem Abdulrasool via cfe-commits]

Author: Saleem Abdulrasool
Date: 2019-11-04T16:55:31-08:00
New Revision: 6db7a5cd7c800a588e94ce5c1ef24ae4d60ecdd3

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

LOG: build: explicitly set the linker language for unwind

The unwinder should not depend on libc++.  In fact, we do not end up
with a link against libc++ as we do not have a dependency on libc++ at
runtime.  This ensures that we link with `clang` rather than `clang++`.

Added: 


Modified: 
libunwind/src/CMakeLists.txt

Removed: 




diff  --git a/libunwind/src/CMakeLists.txt b/libunwind/src/CMakeLists.txt
index ed20ff0015f4..1ecda26406b1 100644
--- a/libunwind/src/CMakeLists.txt
+++ b/libunwind/src/CMakeLists.txt
@@ -127,6 +127,8 @@ if (LIBUNWIND_ENABLE_SHARED)
 "${LIBUNWIND_COMPILE_FLAGS}"
   LINK_FLAGS
 "${LIBUNWIND_LINK_FLAGS}"
+  LINKER_LANGUAGE
+C
   OUTPUT_NAME
 "unwind"
   VERSION



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

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.cpp:488
+void AdjacentArgumentsOfSameTypeCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;

whisperity wrote:
> whisperity wrote:
> > Eugene.Zelenko wrote:
> > > Check seems to be useful for C and probably for Objective-C.
> > I'm not knowledgeable about Objective-C at all to make a decision on how 
> > the "adjacent argument ranges" could be calculated and what mixups are 
> > possible. As for C, should a `cppcoreguidelines-` check be enabled for C? 
> > Or you mean we should allow it to work, and the user will toggle how they 
> > see fit.
> I've added a  `FIXME` for ObjC as I'm really not qualified in that language. 
> C support has been added.
The check should keep working on Objective-C to check C-style calls, and it 
should also keep working on Objective-C++ to check C++-style method calls. You 
should be able to test this by renaming any of your `.c`/`.cpp` test files to 
`.m`/`.mm` (of course you don't *have* to duplicate your tests; just keep a 
couple of functions to see that the warning is still there).

However, the check shouldn't try to check Objective-C message expressions / 
method declarations, because Objective-C message syntax has so-called 
"parameter labels": the caller is forced to spell out which parameter it's 
passing, which effectively mitigates the problem.

Note that `ObjCMethodDecl` is not a sub-class of `FunctionDecl`, so your 
checker is already working correctly (as long as you remove the explicit 
suppression).


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

https://reviews.llvm.org/D69560



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

[PATCH] D69763: [Clang][Test]: Remaining "lld-link2" -> "lld-link"

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

I am curious, how did this work since there is no longer an `lld-link2`? Were 
these tests failing or not being run?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69763



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

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.



Comment at: clang/lib/Driver/Job.cpp:340-355
+  typedef int (*ClangDriverMainFunc)(SmallVectorImpl &);
+  ClangDriverMainFunc ClangDriverMain = nullptr;
+
+  if (ReenterTool) {
+StringRef F = llvm::sys::path::filename(Executable);
+if (F.endswith_lower(".exe"))
+  F = F.drop_back(4);

Meinersbur wrote:
> This looks fragile as it will break when the user chooses to rename the 
> executable (`clang-cuda`, `--driver-mode=...`,...). Why not moving the 
> ClangDriverMain logic into the clangDriver module. Or, at least, pass it 
> through a lambda. It would also make it easier to use clangDriver as a 
> library (if that was ever an intended use-case?!?)
I tried this on Linux and it did not work out-of-the box (`ClangDriverMain` 
being nullptr) due to the executable's name being `clang-10`. After fixing, 
`check-clang` failed with 5 errors in the Driver test.

The performance benefit was within noise: 6m20s vs. 6m18s on a 2-Socket 28 
cores-per-processor (112 SMT threads) Skylake-Xeon system for `ninja all 
check-clang`, assertions enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7689
+  // Coerce HIP pointer arguments from generic pointers to global ones.
+  llvm::Type *coerce(llvm::Type *Ty, unsigned DefaultAS,
+ unsigned GlobalAS) const {

hliao wrote:
> tra wrote:
> > Now it could use a more descriptive name, too. :-)
> > 
> > You can now also make DefaultAS/GlobalAS into local variables as you have 
> > access to `getContext()` here.
> name is changed but I want to leave `DefaultAS` and `GlobalAS` as parameters 
> as they may vary from HIP to OpenCL and different targets. Even though it may 
> be rare case, I want to avoid careless errors.
You may not need it, ever and it would be easy to add, but I'll leave it up to 
you.

If you do want to keep them as parameters you may want to consider renaming 
them to FromAS/ToAS.
There's nothing in the code that has anything to do with whether they are for 
generic/specific address space and the function name does not indicate the 
direction of coercion between them. It's very easy to pass them in the wrong 
order and not notice it. Making them local variables would avoid it. Giving 
names some sort of 'directionality' would at least give user a hint what goes 
where, even if it would not prevent making the error.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826



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

[PATCH] D69832: [WebAssembly] -fwasm-exceptions enables reference-types

[Heejin Ahn via Phabricator via cfe-commits]

aheejin created this revision.
aheejin added a reviewer: tlively.
Herald added subscribers: cfe-commits, sunfish, jgravelle-google, sbc100, 
dschuff.
Herald added a project: clang.

This adds `-mreference-types` and `-mno-reference-types` flags to clang
and make `-fwasm-exceptions` enables reference types feature in clang
and the backend.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69832

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain.c


Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -79,11 +79,11 @@
 // RUN:   | FileCheck -check-prefix=PTHREAD_NO_SIGN_EXT %s
 // PTHREAD_NO_SIGN_EXT: invalid argument '-pthread' not allowed with 
'-mno-sign-ext'
 
-// '-fwasm-exceptions' sets +exception-handling
+// '-fwasm-exceptions' sets +exception-handling and +reference-types
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
 // RUN:--sysroot=/foo %s -fwasm-exceptions 2>&1 \
 // RUN:  | FileCheck -check-prefix=WASM_EXCEPTIONS %s
-// WASM_EXCEPTIONS: clang{{.*}}" "-cc1" {{.*}} "-target-feature" 
"+exception-handling"
+// WASM_EXCEPTIONS: clang{{.*}}" "-cc1" {{.*}} "-target-feature" 
"+exception-handling" "-target-feature" "+reference-types"
 
 // '-fwasm-exceptions' not allowed with '-mno-exception-handling'
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
@@ -91,6 +91,12 @@
 // RUN:   | FileCheck -check-prefix=WASM_EXCEPTIONS_NO_EH %s
 // WASM_EXCEPTIONS_NO_EH: invalid argument '-fwasm-exceptions' not allowed 
with '-mno-exception-handling'
 
+// '-fwasm-exceptions' not allowed with '-mno-reference-types'
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
+// RUN: --sysroot=/foo %s -fwasm-exceptions -mno-reference-types 2>&1 \
+// RUN:   | FileCheck -check-prefix=WASM_EXCEPTIONS_NO_REFTYPES %s
+// WASM_EXCEPTIONS_NO_REFTYPES: invalid argument '-fwasm-exceptions' not 
allowed with '-mno-reference-types'
+
 // '-fwasm-exceptions' not allowed with
 // '-mllvm -enable-emscripten-cxx-exceptions'
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -181,6 +181,12 @@
   getDriver().Diag(diag::err_drv_argument_not_allowed_with)
   << "-fwasm-exceptions"
   << "-mno-exception-handling";
+// '-fwasm-exceptions' is not compatible with '-mno-reference-types'
+if (DriverArgs.hasFlag(options::OPT_mno_reference_types,
+   options::OPT_mexception_handing, false))
+  getDriver().Diag(diag::err_drv_argument_not_allowed_with)
+  << "-fwasm-exceptions"
+  << "-mno-reference-types";
 // '-fwasm-exceptions' is not compatible with
 // '-mllvm -enable-emscripten-cxx-exceptions'
 for (const Arg *A : DriverArgs.filtered(options::OPT_mllvm)) {
@@ -189,9 +195,11 @@
 << "-fwasm-exceptions"
 << "-mllvm -enable-emscripten-cxx-exceptions";
 }
-// '-fwasm-exceptions' implies exception-handling
+// '-fwasm-exceptions' implies exception-handling and reference-types
 CC1Args.push_back("-target-feature");
 CC1Args.push_back("+exception-handling");
+CC1Args.push_back("-target-feature");
+CC1Args.push_back("+reference-types");
   }
 }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2311,6 +2311,8 @@
 def mno_multivalue : Flag<["-"], "mno-multivalue">, 
Group;
 def mtail_call : Flag<["-"], "mtail-call">, Group;
 def mno_tail_call : Flag<["-"], "mno-tail-call">, Group;
+def mreference_types : Flag<["-"], "mreference-types">, 
Group;
+def mno_reference_types : Flag<["-"], "mno-reference-types">, 
Group;
 
 def mamdgpu_debugger_abi : Joined<["-"], "mamdgpu-debugger-abi=">,
   Flags<[HelpHidden]>,


Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -79,11 +79,11 @@
 // RUN:   | FileCheck -check-prefix=PTHREAD_NO_SIGN_EXT %s
 // PTHREAD_NO_SIGN_EXT: invalid argument '-pthread' not allowed with '-mno-sign-ext'
 
-// '-fwasm-exceptions' sets +exception-handling
+// '-fwasm-exceptions' sets +exception-handling and +reference-types
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown \
 // RUN:--sysroot=/foo %s -fwasm-exceptions 2>&1 \
 // RUN:  | FileCheck -check-prefix=WASM_EXCEPTIONS %s
-// WASM_EXCEPTIONS: clang{{.*}}" "-cc1" {{.*}} 

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Michael Liao via Phabricator via cfe-commits]

hliao marked 3 inline comments as done.
hliao added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7719
+  // Arrary types.
+  if (auto ATy = dyn_cast(Ty)) {
+auto T = ATy->getElementType();

hliao wrote:
> hliao wrote:
> > arsenm wrote:
> > > No tests with arrays or structs?
> > > 
> > > It's also not immediately obvious to me that this optimization is still 
> > > valid if the pointer is buried in a struct
> > the original generic kernel pointer promotion to a global one only handles 
> > the pointer directly passed. From a critical workload, I found quite a few 
> > cases where the global pointers are passed through a by-val struct. We 
> > didn't handle that yet. With this case, we could start to handle that.
> struct tests are added. From test cases, it seems to me that arry is not 
> passed by value. I need to double-confirm.
a test case for arrary types is added.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7689
+  // Coerce HIP pointer arguments from generic pointers to global ones.
+  llvm::Type *coerce(llvm::Type *Ty, unsigned DefaultAS,
+ unsigned GlobalAS) const {

tra wrote:
> Now it could use a more descriptive name, too. :-)
> 
> You can now also make DefaultAS/GlobalAS into local variables as you have 
> access to `getContext()` here.
name is changed but I want to leave `DefaultAS` and `GlobalAS` as parameters as 
they may vary from HIP to OpenCL and different targets. Even though it may be 
rare case, I want to avoid careless errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 227795.
hliao added a comment.

- revise member function name.
- add the test case for by-val array types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu

Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -x hip %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+
+// Coerced struct from `struct S` without all generic pointers lowered into
+// global ones.
+// CHECK: %struct.S.coerce = type { i32 addrspace(1)*, float addrspace(1)* }
+// CHECK: %struct.T.coerce = type { [2 x float addrspace(1)*] }
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
+__global__ void kernel1(int *x) {
+  x[0]++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* dereferenceable(4) %x.coerce)
+__global__ void kernel2(int ) {
+  x++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
+__global__ void kernel3(__attribute__((address_space(2))) int *x,
+__attribute__((address_space(1))) int *y) {
+  y[0] = x[0];
+}
+
+// CHECK: define void @_Z4funcPi(i32* %x)
+__device__ void func(int *x) {
+  x[0]++;
+}
+
+struct S {
+  int *x;
+  float *y;
+};
+// `by-val` struct will be coerced into a similar struct with all generic
+// pointers lowerd into global ones.
+// CHECK: define amdgpu_kernel void @_Z7kernel41S(%struct.S.coerce %s.coerce)
+__global__ void kernel4(struct S s) {
+  s.x[0]++;
+  s.y[0] += 1.f;
+}
+
+// If a pointer to struct is passed, only the pointer itself is coerced into the global one.
+// CHECK: define amdgpu_kernel void @_Z7kernel5P1S(%struct.S addrspace(1)* %s.coerce)
+__global__ void kernel5(struct S *s) {
+  s->x[0]++;
+  s->y[0] += 1.f;
+}
+
+struct T {
+  float *x[2];
+};
+// `by-val` array is also coerced.
+// CHECK: define amdgpu_kernel void @_Z7kernel61T(%struct.T.coerce %t.coerce)
+__global__ void kernel6(struct T t) {
+  t.x[0][0] += 1.f;
+  t.x[1][0] += 2.f;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7685,6 +7685,42 @@
   bool isHomogeneousAggregateSmallEnough(const Type *Base,
  uint64_t Members) const override;
 
+  // Coerce HIP pointer arguments from generic pointers to global ones.
+  llvm::Type *coerceKernelArgumentType(llvm::Type *Ty, unsigned DefaultAS,
+   unsigned GlobalAS) const {
+// Structure types.
+if (auto STy = dyn_cast(Ty)) {
+  SmallVector EltTys;
+  bool Changed = false;
+  for (auto T : STy->elements()) {
+auto NT = coerceKernelArgumentType(T, DefaultAS, GlobalAS);
+EltTys.push_back(NT);
+Changed |= (NT != T);
+  }
+  // Skip if there is no change in element types.
+  if (!Changed)
+return STy;
+  if (STy->hasName())
+return llvm::StructType::create(
+EltTys, (STy->getName() + ".coerce").str(), STy->isPacked());
+  return llvm::StructType::get(getVMContext(), EltTys, STy->isPacked());
+}
+// Arrary types.
+if (auto ATy = dyn_cast(Ty)) {
+  auto T = ATy->getElementType();
+  auto NT = coerceKernelArgumentType(T, DefaultAS, GlobalAS);
+  // Skip if there is no change in that element type.
+  if (NT == T)
+return ATy;
+  return llvm::ArrayType::get(NT, ATy->getNumElements());
+}
+// Single value types.
+if (Ty->isPointerTy() && Ty->getPointerAddressSpace() == DefaultAS)
+  return llvm::PointerType::get(
+  cast(Ty)->getElementType(), GlobalAS);
+return Ty;
+  }
+
 public:
   explicit AMDGPUABIInfo(CodeGen::CodeGenTypes ) :
 DefaultABIInfo(CGT) {}
@@ -7812,14 +7848,22 @@
 
   // TODO: Can we omit empty structs?
 
-  // Coerce single element structs to its element.
+  llvm::Type *LTy = nullptr;
   if (const Type *SeltTy = isSingleElementStruct(Ty, getContext()))
-return ABIArgInfo::getDirect(CGT.ConvertType(QualType(SeltTy, 0)));
+LTy = CGT.ConvertType(QualType(SeltTy, 0));
+
+  if (getContext().getLangOpts().HIP) {
+if (!LTy)
+  LTy = CGT.ConvertType(Ty);
+LTy = coerceKernelArgumentType(
+LTy, getContext().getTargetAddressSpace(LangAS::Default),
+getContext().getTargetAddressSpace(LangAS::cuda_device));
+  }
 
   // If we set CanBeFlattened to true, CodeGen will expand the struct to its
   // individual elements, which 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3250
   // -gline-directives-only supported only for the DWARF debug info.
   if (DWARFVersion == 0 && DebugInfoKind == 
codegenoptions::DebugDirectivesOnly)
 DebugInfoKind = codegenoptions::NoDebugInfo;

probinson wrote:
> After setting DWARFVersion above, when nothing was requested, this looks 
> peculiar.  Is `DWARFVersion == 0` a proxy for `-gcodeview` ?
Yep, DWARFVersion shouldn't be set when none was requested - see discussion 
above between myself & @cmtice - that should leave this code fine/correct.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6174
+DwarfVersion = ParseDebugDefaultVersion(getToolChain(), Args);
+
   if (DwarfVersion == 0)

probinson wrote:
> I have to say, this sequence is a whole lot easier to follow than what's up 
> in RenderDebugOptions.  It would be nice if they were both so easy to 
> understand.
> 
> Although it makes me wonder, does `-fdebug-default-version` go unclaimed here 
> if there's a `-gdwarf-2` on the command line?
Once the "else if (DefaultDWARFVersion != 0)" clause is removed, and the 
ParseDebugDefaultVersion call is sunk down into the "if (EmitDwarf)" case - 
hopefully that'll simplify the RenderDebugOptions code enough to be reasonable?

I think the complication there is that "-g" can imply CodeView if nothing 
explicitly requests DWARF & the target is windows, whereas there's no support 
for that kind of CV fallback here? So some of that might be inherent.

& agreed - worth testing that -fdebug-default-version doesn't get a -Wunused 
warning if someone says "-fdebug-default-version 4 -gdwarf-2". @cmtice is that 
tested? You might be able to add -Werror to the existing test case's RUN lines 
to demonstrate that no such warnings are produced?



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1151
+  || Value > 5
+  || Value < 1)
+TC.getDriver().Diag(diag::err_drv_invalid_int_value)

probinson wrote:
> Clang doesn't support DWARF v1.  I expect nobody does at this point (DWARF 2 
> came out in 1993).
You'd recommend/request moving this bound up to 2 (to be consistent with the 
fact that clang supports -gdwarf-2 technically (even though we probably don't 
produce fully conformant DWARFv2 these days, I would guess))?


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

https://reviews.llvm.org/D69822



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

In D69785#1733233 , @jdoerfert wrote:

> In D69785#1732951 , @rogfer01 wrote:
>
> > I made a small experiment lowering a `taskwait` (which is even simpler than 
> > `barrier` in "lowering" complexity). It was pretty straightforward after 
> > all.
>
>
> Nice, can you share the code? Was it based on the OpenMPIRBuilder (this one 
> or the old one) or somehow separate?


I posted the WIP D69828 .

Kind regards,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D69822#1733269 , @dblaikie wrote:

> In D69822#1733262 , @probinson wrote:
>
> > The maze of twisty -g passages gets a new secret door.  Oh well.
>
>
> If you find this to be a significant complication, I'd really like to discuss 
> it further - I know there are some twisty things (one of the worst I created 
> when it comes to -fsplit-dwarf-inlining, -gsplit-dwarf, -gmlt combinations) - 
> but this seems pretty simple/tidy/orthogonal to me.


No, no, if we can at least keep the twistiness in one place (or two I suppose, 
one for C/C++ and one for asm) it's not a real problem.

Maybe a strategic helper function could be useful to make those two places more 
obviously the same.


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

https://reviews.llvm.org/D69822



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

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124
+  if (const SymbolicRegion *SR = DestMR->getSymbolicBase())
+if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR))
+  return exprToStr(SizeExpr, C);

Charusso wrote:
> NoQ wrote:
> > Charusso wrote:
> > > Charusso wrote:
> > > > NoQ wrote:
> > > > > Again, you will have to highlight the allocation site with a note. 
> > > > > Therefore you will have to write a bug visitor that traverses the 
> > > > > size expression at some point (or, equivalently, a note tag when the 
> > > > > size expression is evaluated). Therefore you don't need to store the 
> > > > > expression in the program state.
> > > > Yes, you have pointed out the necessary visitor, but it needs more 
> > > > thinking.
> > > > 
> > > > I have a memory region which could be any kind of "memory block region" 
> > > > therefore I have no idea where is the size expression. We are 
> > > > supporting ~20 different allocations, which is nothing compared to the 
> > > > wild with the not so uncommon 5+ parameter allocators. Therefore I 
> > > > still do not want to reverse engineer a small MallocChecker + 
> > > > ExprEngine + BuiltinFunctionChecker inside my checker. They provide the 
> > > > necessary `DynamicSizeInfo` easily, which could be used in at least 4 
> > > > checkers at the moment (which I have pointed out earlier in D68725).
> > > > 
> > > > If I have the size expression in the dynamic size map, and I can 
> > > > clearly point out the destination buffer, it is a lot more simplified 
> > > > to traverse the graph where the buffer and its size comes from.
> > > Well, you really do not want to store `SizeExpr` of `malloc(SizeExpr)` 
> > > and you are right I will have to traverse from it to see whether the 
> > > `SizeExpr` is ambiguous or not, where it comes from.
> > > 
> > > I want to rely on the `trackExpressionValue()` as the `SizeExpr` is 
> > > available by `getDynamicSizeExpr()`, so it is one or two lines of code.
> > > 
> > > Would you create your own switch-case to see where is the size expression 
> > > goes in the allocation and use `trackExpressionValue()` on it? So that 
> > > you do not store information in the global state which results in better 
> > > run-time / less memory.
> > > 
> > > At first I really wanted to model `malloc()` and `realloc()` and stuff, 
> > > then I realized the `MallocChecker` provides every information I need. 
> > > Would it be a better idea to create my own tiny `MallocChecker` inside my 
> > > checker which does nothing but marks the size expression being 
> > > interesting with `NoteTags`?
> > > 
> > > Also I am thinking of a switch-case on the `DefinedOrUnknownSVal Size` 
> > > which somewhere has an expression inside it which I could 
> > > `trackExpressionValue()` on.
> > > 
> > > Basically we are missing the rules what to use and I have picked the 
> > > easiest solution. Could you share please which would be the right 
> > > direction for such a simple task?
> > > I want to rely on the `trackExpressionValue()` as the `SizeExpr` is 
> > > available by `getDynamicSizeExpr()`, so it is one or two lines of code.
> > 
> > This won't work. `trackExpressionValue()` can only track an active 
> > expression (that has, or at least should have, a value in the bug-node's 
> > environment). You'll have to make a visitor or a note tag.
> > 
> > You can either make your own visitor (which will detect the node in which 
> > the extent information becomes present), or convert `MallocChecker` to use 
> > note tags and then inter-operate with those tags (though the 
> > interestingness map - "i mark the symbol as interesting so i'm interested 
> > in highlighting the allocation site" - or a similar mechanism). The second 
> > approach is more work because no such interoperation has ever been 
> > implemented yet, but it should be pretty rewarding for the future.
> > This won't work. trackExpressionValue() can only track an active expression 
> > (that has, or at least should have, a value in the bug-node's environment). 
> > You'll have to make a visitor or a note tag.
> So because most likely after the `malloc()` the `size` symbol dies, the 
> `trackExpressionValue()` cannot track dead symbols? Because we could make the 
> `size` dying base on the `buffer`, we have some dependency logic for that. It 
> also represents the truth, the size is part of that memory block's region. 
> After that we could track the expression of the `size`?
> So because most likely after the malloc() the size symbol dies...?

After the `malloc()` is consumed, the size //expression// dies and gets cleaned 
up from the //Environment//. The symbol will only die if the value wasn't put 
into the //Store// in the process of modeling the statement that consumes the 
`malloc()` expression (such as an assignment). But `trackExpressionValue()` can 
only track live (active) expressions.

[PATCH] D69743: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

[Vedant Kumar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa5c8ec4baa2c: [CGDebugInfo] Emit subprograms for decls when 
AT_tail_call is understood (authored by vsk).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69743

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-extern-call.c
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll


Index: llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
===
--- llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
+++ llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
@@ -25,6 +25,14 @@
 
 @sink = global i32 0, align 4, !dbg !0
 
+define void @__has_no_subprogram() {
+entry:
+  %0 = load volatile i32, i32* @sink, align 4
+  %inc = add nsw i32 %0, 1
+  store volatile i32 %inc, i32* @sink, align 4
+  ret void
+}
+
 ; ASM: DW_TAG_subprogram
 ; ASM:   DW_AT_call_all_calls
 ; OBJ: [[bat_sp:.*]]: DW_TAG_subprogram
@@ -70,6 +78,7 @@
 ; OBJ: DW_AT_call_tail_call
 define void @_Z3foov() !dbg !25 {
 entry:
+  tail call void @__has_no_subprogram()
   tail call void @_Z3barv(), !dbg !26
   tail call void @_Z3batv(), !dbg !27
   tail call void @_Z3barv(), !dbg !26
Index: clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
===
--- clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
+++ clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
@@ -56,6 +56,7 @@
 
 // NO-ATTR-NOT: FlagAllCallsDescribed
 
+// HAS-ATTR-DAG: DISubprogram(name: "declaration1", {{.*}}, flags: 
DIFlagPrototyped
 // HAS-ATTR-DAG: DISubprogram(name: "declaration2", {{.*}}, flags: 
DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
 // HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: 
DIFlagPrototyped, spFlags: DISPFlagOptimized)
 // HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped 
| DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
Index: clang/test/CodeGen/debug-info-extern-call.c
===
--- clang/test/CodeGen/debug-info-extern-call.c
+++ clang/test/CodeGen/debug-info-extern-call.c
@@ -1,8 +1,21 @@
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target 
x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s 
-check-prefix=CHECK-EXT
-// CHECK-EXT: !DISubprogram(name: "fn1"
+// When entry values are emitted, expect a subprogram for extern decls so that
+// the dwarf generator can describe call site parameters at extern call sites.
+//
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target 
x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s 
-check-prefix=ENTRY-VAL
+// ENTRY-VAL: !DISubprogram(name: "fn1"
 
-// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | 
FileCheck %s
-// CHECK-NOT: !DISubprogram(name: "fn1"
+// Similarly, when the debugger tuning is gdb, expect a subprogram for extern
+// decls so that the dwarf generator can describe information needed for tail
+// call frame reconstrution.
+//
+// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -ggdb -S -emit-llvm %s -o 
- | FileCheck %s -check-prefix=GDB
+// GDB: !DISubprogram(name: "fn1"
+//
+// Do not emit a subprogram for extern decls when entry values are disabled and
+// the tuning is not set to gdb.
+//
+// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o 
- | FileCheck %s -check-prefix=SCE
+// SCE-NOT: !DISubprogram(name: "fn1"
 
 extern int fn1(int a, int b);
 
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3748,9 +3748,7 @@
 void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
   QualType CalleeType,
   const FunctionDecl *CalleeDecl) {
-  auto  = CGM.getCodeGenOpts();
-  if (!CGOpts.EnableDebugEntryValues || !CGM.getLangOpts().Optimize ||
-  !CallOrInvoke)
+  if (!CallOrInvoke || getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
 return;
 
   auto *Func = CallOrInvoke->getCalledFunction();
@@ -4824,10 +4822,10 @@
   bool SupportsDWARFv4Ext =
   CGM.getCodeGenOpts().DwarfVersion == 4 &&
   (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
-   (CGM.getCodeGenOpts().EnableDebugEntryValues &&
-   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
+   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB);
 
-  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
+  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5 

[clang] a5c8ec4 - [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

[Vedant Kumar via cfe-commits]

Author: Vedant Kumar
Date: 2019-11-04T15:14:24-08:00
New Revision: a5c8ec4baa2c00d8dbca36ffd236a40f9e0c07ed

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

LOG: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

Currently, clang emits subprograms for declared functions when the
target debugger or DWARF standard is known to support entry values
(DW_OP_entry_value & the GNU equivalent).

Treat DW_AT_tail_call the same way to allow debuggers to follow cross-TU
tail calls.

Pre-patch debug session with a cross-TU tail call:

```
  * frame #0: 0x00010fa4 main`target at b.c:4:3 [opt]
frame #1: 0x00010f99 main`main at a.c:8:10 [opt]
```

Post-patch (note that the tail-calling frame, "helper", is visible):

```
  * frame #0: 0x00010fa4 main`target at b.c:4:3 [opt]
frame #1: 0x00010f80 main`helper [opt] [artificial]
frame #2: 0x00010f99 main`main at a.c:8:10 [opt]
```

rdar://46577651

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

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGen/debug-info-extern-call.c
clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index e0bb3fda7acf..c0c8fd3c8f16 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3748,9 +3748,7 @@ void CGDebugInfo::EmitFunctionDecl(GlobalDecl GD, 
SourceLocation Loc,
 void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
   QualType CalleeType,
   const FunctionDecl *CalleeDecl) {
-  auto  = CGM.getCodeGenOpts();
-  if (!CGOpts.EnableDebugEntryValues || !CGM.getLangOpts().Optimize ||
-  !CallOrInvoke)
+  if (!CallOrInvoke || getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
 return;
 
   auto *Func = CallOrInvoke->getCalledFunction();
@@ -4824,10 +4822,10 @@ llvm::DINode::DIFlags 
CGDebugInfo::getCallSiteRelatedAttrs() const {
   bool SupportsDWARFv4Ext =
   CGM.getCodeGenOpts().DwarfVersion == 4 &&
   (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
-   (CGM.getCodeGenOpts().EnableDebugEntryValues &&
-   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
+   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB);
 
-  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
+  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5 &&
+  !CGM.getCodeGenOpts().EnableDebugEntryValues)
 return llvm::DINode::FlagZero;
 
   return llvm::DINode::FlagAllCallsDescribed;

diff  --git a/clang/test/CodeGen/debug-info-extern-call.c 
b/clang/test/CodeGen/debug-info-extern-call.c
index e35669b78f93..7f58fe59a635 100644
--- a/clang/test/CodeGen/debug-info-extern-call.c
+++ b/clang/test/CodeGen/debug-info-extern-call.c
@@ -1,8 +1,21 @@
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target 
x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s 
-check-prefix=CHECK-EXT
-// CHECK-EXT: !DISubprogram(name: "fn1"
+// When entry values are emitted, expect a subprogram for extern decls so that
+// the dwarf generator can describe call site parameters at extern call sites.
+//
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target 
x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s 
-check-prefix=ENTRY-VAL
+// ENTRY-VAL: !DISubprogram(name: "fn1"
 
-// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | 
FileCheck %s
-// CHECK-NOT: !DISubprogram(name: "fn1"
+// Similarly, when the debugger tuning is gdb, expect a subprogram for extern
+// decls so that the dwarf generator can describe information needed for tail
+// call frame reconstrution.
+//
+// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -ggdb -S -emit-llvm %s -o 
- | FileCheck %s -check-prefix=GDB
+// GDB: !DISubprogram(name: "fn1"
+//
+// Do not emit a subprogram for extern decls when entry values are disabled and
+// the tuning is not set to gdb.
+//
+// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o 
- | FileCheck %s -check-prefix=SCE
+// SCE-NOT: !DISubprogram(name: "fn1"
 
 extern int fn1(int a, int b);
 

diff  --git a/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp 
b/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
index 1cb2b6c609f3..667c2469b55e 100644
--- a/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
+++ b/clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
@@ -56,6 +56,7 @@
 
 // NO-ATTR-NOT: FlagAllCallsDescribed
 
+// HAS-ATTR-DAG: 

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.
Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124
+  if (const SymbolicRegion *SR = DestMR->getSymbolicBase())
+if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR))
+  return exprToStr(SizeExpr, C);

NoQ wrote:
> Charusso wrote:
> > Charusso wrote:
> > > NoQ wrote:
> > > > Again, you will have to highlight the allocation site with a note. 
> > > > Therefore you will have to write a bug visitor that traverses the size 
> > > > expression at some point (or, equivalently, a note tag when the size 
> > > > expression is evaluated). Therefore you don't need to store the 
> > > > expression in the program state.
> > > Yes, you have pointed out the necessary visitor, but it needs more 
> > > thinking.
> > > 
> > > I have a memory region which could be any kind of "memory block region" 
> > > therefore I have no idea where is the size expression. We are supporting 
> > > ~20 different allocations, which is nothing compared to the wild with the 
> > > not so uncommon 5+ parameter allocators. Therefore I still do not want to 
> > > reverse engineer a small MallocChecker + ExprEngine + 
> > > BuiltinFunctionChecker inside my checker. They provide the necessary 
> > > `DynamicSizeInfo` easily, which could be used in at least 4 checkers at 
> > > the moment (which I have pointed out earlier in D68725).
> > > 
> > > If I have the size expression in the dynamic size map, and I can clearly 
> > > point out the destination buffer, it is a lot more simplified to traverse 
> > > the graph where the buffer and its size comes from.
> > Well, you really do not want to store `SizeExpr` of `malloc(SizeExpr)` and 
> > you are right I will have to traverse from it to see whether the `SizeExpr` 
> > is ambiguous or not, where it comes from.
> > 
> > I want to rely on the `trackExpressionValue()` as the `SizeExpr` is 
> > available by `getDynamicSizeExpr()`, so it is one or two lines of code.
> > 
> > Would you create your own switch-case to see where is the size expression 
> > goes in the allocation and use `trackExpressionValue()` on it? So that you 
> > do not store information in the global state which results in better 
> > run-time / less memory.
> > 
> > At first I really wanted to model `malloc()` and `realloc()` and stuff, 
> > then I realized the `MallocChecker` provides every information I need. 
> > Would it be a better idea to create my own tiny `MallocChecker` inside my 
> > checker which does nothing but marks the size expression being interesting 
> > with `NoteTags`?
> > 
> > Also I am thinking of a switch-case on the `DefinedOrUnknownSVal Size` 
> > which somewhere has an expression inside it which I could 
> > `trackExpressionValue()` on.
> > 
> > Basically we are missing the rules what to use and I have picked the 
> > easiest solution. Could you share please which would be the right direction 
> > for such a simple task?
> > I want to rely on the `trackExpressionValue()` as the `SizeExpr` is 
> > available by `getDynamicSizeExpr()`, so it is one or two lines of code.
> 
> This won't work. `trackExpressionValue()` can only track an active expression 
> (that has, or at least should have, a value in the bug-node's environment). 
> You'll have to make a visitor or a note tag.
> 
> You can either make your own visitor (which will detect the node in which the 
> extent information becomes present), or convert `MallocChecker` to use note 
> tags and then inter-operate with those tags (though the interestingness map - 
> "i mark the symbol as interesting so i'm interested in highlighting the 
> allocation site" - or a similar mechanism). The second approach is more work 
> because no such interoperation has ever been implemented yet, but it should 
> be pretty rewarding for the future.
> This won't work. trackExpressionValue() can only track an active expression 
> (that has, or at least should have, a value in the bug-node's environment). 
> You'll have to make a visitor or a note tag.
So because most likely after the `malloc()` the `size` symbol dies, the 
`trackExpressionValue()` cannot track dead symbols? Because we could make the 
`size` dying base on the `buffer`, we have some dependency logic for that. It 
also represents the truth, the size is part of that memory block's region. 
After that we could track the expression of the `size`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69813



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1955
   Flags<[CC1Option]>;
+def fdebug_default_version: Joined<["-"], "fdebug-default-version=">, 
Group;
 def fdebug_prefix_map_EQ

If this is specifically the default DWARF version, I think the word "dwarf" 
ought to be in the option name.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3148
   // decision should be made in the driver as well though.
+  unsigned DefaultDWARFVersion = ParseDebugDefaultVersion(TC, Args);
   unsigned DWARFVersion = 0;

I would rather see all the DWARF version weirdness all in one place (i.e., 
please move this down to the rest of it) particularly given that the "default" 
DWARF version now means two different things (something from the command line, 
and something based on the toolchain).



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3250
   // -gline-directives-only supported only for the DWARF debug info.
   if (DWARFVersion == 0 && DebugInfoKind == 
codegenoptions::DebugDirectivesOnly)
 DebugInfoKind = codegenoptions::NoDebugInfo;

After setting DWARFVersion above, when nothing was requested, this looks 
peculiar.  Is `DWARFVersion == 0` a proxy for `-gcodeview` ?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6174
+DwarfVersion = ParseDebugDefaultVersion(getToolChain(), Args);
+
   if (DwarfVersion == 0)

I have to say, this sequence is a whole lot easier to follow than what's up in 
RenderDebugOptions.  It would be nice if they were both so easy to understand.

Although it makes me wonder, does `-fdebug-default-version` go unclaimed here 
if there's a `-gdwarf-2` on the command line?


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

https://reviews.llvm.org/D69822



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7689
+  // Coerce HIP pointer arguments from generic pointers to global ones.
+  llvm::Type *coerce(llvm::Type *Ty, unsigned DefaultAS,
+ unsigned GlobalAS) const {

Now it could use a more descriptive name, too. :-)

You can now also make DefaultAS/GlobalAS into local variables as you have 
access to `getContext()` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826



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

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

[Rian Quinn via Phabricator via cfe-commits]

rianquinn added a comment.

Looking at the unit tests, it appears that you have successfully fixed the bug


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69573



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D69822#1733262 , @probinson wrote:

> In D69822#1733243 , @dblaikie wrote:
>
> > In D69822#1733226 , @probinson 
> > wrote:
> >
> > > > Want to decouple setting the DWARF version from enabling/disabling 
> > > > generation of debug info.
> > >
> > > Um, why?
> >
> >
> > If you've got a big build system with various users opting in/out of DWARF 
> > and you want to migrate to DWARFv5, say, but you can't add "-gdwarf-5" to 
> > your build system, because that'd turn on debug info in cases that don't 
> > need it - but it's easier to change the default than to modify all cases 
> > that enable dwarf across the codebase.
> >
> > Open to discussion about whether this is a good/bad idea - but it'd be 
> > useful for Google at least & seemed low-cost enough to go this route.
>
>
> Because you want the default to be based on your corporate environment rather 
> than the target platform.


Sure - if we know our users have access to a modern debugger, but we don't own 
a whole platform. You could imagine someone shipping clang+debugger in a 3rd 
party/non-platform IDE might want something like this too/similar to the Google 
situation. We can't dictate what's suitable for all of Linux, but for the 
subset of users that are using a specific toolchain/situation, we can.

> The maze of twisty -g passages gets a new secret door.  Oh well.

If you find this to be a significant complication, I'd really like to discuss 
it further - I know there are some twisty things (one of the worst I created 
when it comes to -fsplit-dwarf-inlining, -gsplit-dwarf, -gmlt combinations) - 
but this seems pretty simple/tidy/orthogonal to me.


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

https://reviews.llvm.org/D69822



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 227787.
hliao added a comment.

revise code following reviwers' comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu

Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -x hip %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+
+// Coerced struct from `struct S` without all generic pointers lowered into
+// global ones.
+// CHECK: %struct.S.coerce = type { i32 addrspace(1)*, float addrspace(1)* }
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
+__global__ void kernel1(int *x) {
+  x[0]++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* dereferenceable(4) %x.coerce)
+__global__ void kernel2(int ) {
+  x++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
+__global__ void kernel3(__attribute__((address_space(2))) int *x,
+__attribute__((address_space(1))) int *y) {
+  y[0] = x[0];
+}
+
+// CHECK: define void @_Z4funcPi(i32* %x)
+__device__ void func(int *x) {
+  x[0]++;
+}
+
+struct S {
+  int *x;
+  float *y;
+};
+// `by-val` struct will be coerced into a similar struct with all generic
+// pointers lowerd into global ones.
+// CHECK: define amdgpu_kernel void @_Z7kernel41S(%struct.S.coerce %s.coerce)
+__global__ void kernel4(struct S s) {
+  s.x[0]++;
+  s.y[0] += 1.f;
+}
+
+// If a pointer to struct is passed, only the pointer itself is coerced into the global one.
+// CHECK: define amdgpu_kernel void @_Z7kernel5P1S(%struct.S addrspace(1)* %s.coerce)
+__global__ void kernel5(struct S *s) {
+  s->x[0]++;
+  s->y[0] += 1.f;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7685,6 +7685,42 @@
   bool isHomogeneousAggregateSmallEnough(const Type *Base,
  uint64_t Members) const override;
 
+  // Coerce HIP pointer arguments from generic pointers to global ones.
+  llvm::Type *coerce(llvm::Type *Ty, unsigned DefaultAS,
+ unsigned GlobalAS) const {
+// Structure types.
+if (auto STy = dyn_cast(Ty)) {
+  SmallVector EltTys;
+  bool Changed = false;
+  for (auto T : STy->elements()) {
+auto NT = coerce(T, DefaultAS, GlobalAS);
+EltTys.push_back(NT);
+Changed |= (NT != T);
+  }
+  // Skip if there is no change in element types.
+  if (!Changed)
+return STy;
+  if (STy->hasName())
+return llvm::StructType::create(
+EltTys, (STy->getName() + ".coerce").str(), STy->isPacked());
+  return llvm::StructType::get(getVMContext(), EltTys, STy->isPacked());
+}
+// Arrary types.
+if (auto ATy = dyn_cast(Ty)) {
+  auto T = ATy->getElementType();
+  auto NT = coerce(T, DefaultAS, GlobalAS);
+  // Skip if there is no change in that element type.
+  if (NT == T)
+return ATy;
+  return llvm::ArrayType::get(NT, ATy->getNumElements());
+}
+// Single value types.
+if (Ty->isPointerTy() && Ty->getPointerAddressSpace() == DefaultAS)
+  return llvm::PointerType::get(
+  cast(Ty)->getElementType(), GlobalAS);
+return Ty;
+  }
+
 public:
   explicit AMDGPUABIInfo(CodeGen::CodeGenTypes ) :
 DefaultABIInfo(CGT) {}
@@ -7812,14 +7848,21 @@
 
   // TODO: Can we omit empty structs?
 
-  // Coerce single element structs to its element.
+  llvm::Type *LTy = nullptr;
   if (const Type *SeltTy = isSingleElementStruct(Ty, getContext()))
-return ABIArgInfo::getDirect(CGT.ConvertType(QualType(SeltTy, 0)));
+LTy = CGT.ConvertType(QualType(SeltTy, 0));
+
+  if (getContext().getLangOpts().HIP) {
+if (!LTy)
+  LTy = CGT.ConvertType(Ty);
+LTy = coerce(LTy, getContext().getTargetAddressSpace(LangAS::Default),
+ getContext().getTargetAddressSpace(LangAS::cuda_device));
+  }
 
   // If we set CanBeFlattened to true, CodeGen will expand the struct to its
   // individual elements, which confuses the Clover OpenCL backend; therefore we
   // have to set it to false here. Other args of getDirect() are just defaults.
-  return ABIArgInfo::getDirect(nullptr, 0, nullptr, false);
+  return ABIArgInfo::getDirect(LTy, 0, nullptr, false);
 }
 
 ABIArgInfo AMDGPUABIInfo::classifyArgumentType(QualType Ty,
Index: clang/lib/CodeGen/CGCall.cpp
===
--- 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D69822#1733243 , @dblaikie wrote:

> In D69822#1733226 , @probinson wrote:
>
> > > Want to decouple setting the DWARF version from enabling/disabling 
> > > generation of debug info.
> >
> > Um, why?
>
>
> If you've got a big build system with various users opting in/out of DWARF 
> and you want to migrate to DWARFv5, say, but you can't add "-gdwarf-5" to 
> your build system, because that'd turn on debug info in cases that don't need 
> it - but it's easier to change the default than to modify all cases that 
> enable dwarf across the codebase.
>
> Open to discussion about whether this is a good/bad idea - but it'd be useful 
> for Google at least & seemed low-cost enough to go this route.


Because you want the default to be based on your corporate environment rather 
than the target platform.  The maze of twisty -g passages gets a new secret 
door.  Oh well.


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

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D69822#1733258 , @aprantl wrote:

> Since this sounds like it is hidden inside of some other tooling — is passing 
> the frontend option `-dwarf-version=5` not an option?


"hidden inside of some other tooling" - in the same sense as most build flags 
are passed by a build system, yes. But generally we (Google specifically, all 
of us LLVM/Clang users in general) try not to use implementation details like 
that.




Comment at: clang/test/CodeGen/debug-default-version.c:24
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf 
-S -emit-llvm -o - %s \

probinson wrote:
> Does that actually work?  Last I checked, DWARF and COFF didn't play nicely, 
> but I admit that was quite a while ago.
Existing test cases cover scenarios like this (see 
clang/test/CodeGen/dwarf-version.c), so this seems consistent to preserve that 
sort of functionality, however well it currently works.


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

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Since this sounds like it is hidden inside of some other tooling — is passing 
the frontend option `-dwarf-version=5` not an option?


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

https://reviews.llvm.org/D69822



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

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

This patch reduced the build time from 12 to 7 minutes? Sounds too good to be 
true. Thanks for the work! I would primarily interested in this to avoid 
determining the `-cc1` command line when debugging.

However, the implementation of calling it's own main function gives the 
impression to be more like a hack. How much restructuring would be needed to 
properly create a CompilerInstance inside the driver instead?




Comment at: clang/lib/Driver/Job.cpp:340-355
+  typedef int (*ClangDriverMainFunc)(SmallVectorImpl &);
+  ClangDriverMainFunc ClangDriverMain = nullptr;
+
+  if (ReenterTool) {
+StringRef F = llvm::sys::path::filename(Executable);
+if (F.endswith_lower(".exe"))
+  F = F.drop_back(4);

This looks fragile as it will break when the user chooses to rename the 
executable (`clang-cuda`, `--driver-mode=...`,...). Why not moving the 
ClangDriverMain logic into the clangDriver module. Or, at least, pass it 
through a lambda. It would also make it easier to use clangDriver as a library 
(if that was ever an intended use-case?!?)



Comment at: clang/tools/driver/driver.cpp:337
+
+int ClangDriverMain(SmallVectorImpl ) {
+  static LLVM_THREAD_LOCAL bool EnterPE = true;

Could you add a comment for the purpose of this function? It's not obvious 
outside the context of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:184
+  if (IsFix) {
+if (Optional SizeStr = getSizeExprAsString(Call, CallC, C)) {
+  renameFunctionFix(UseSafeFunctions ? "gets_s" : "fgets", Call, *Report);

Charusso wrote:
> NoQ wrote:
> > Also, which is probably more important, you will never be able to provide a 
> > fixit for the malloced memory case, because there may be multiple execution 
> > paths that reach the current point with different size expressions (in 
> > fact, not necessarily all of them are malloced).
> > 
> > Eg.:
> > ```lang=c
> > char *x = 0;
> > char y[10];
> > 
> > if (coin()) {
> >   x = malloc(20);
> > } else {
> >   x = y;
> > }
> > 
> > gets(x);
> > ```
> > 
> > If you suggest replacing `gets(x)` with `gets_s(x, 20)`, you'll still have 
> > a buffer overflow on the else-branch on which `x` points to an array of 10 
> > bytes.
> This checker going to evolve a lot, and all of the checked function calls 
> have issues like that. I do not even think what else issues they have. I 
> would like to cover the false alarm suppression when we are about to alarm. 
> Is it would be okay? I really would like to see alarms first.
> 
> For example, I have seen stuff in the wild so that I can state out 8-param 
> allocators and we need to rely on the checkers provide information about 
> allocation.
*summons @Szelethus*

Apart from the obviously syntactic cases, you might actually be able to 
implement fixits for the situation when the reaching-definitions analysis 
displays exactly one definition for `x`, which additionally coincides with the 
allocation site. If that definition is a simple assignment, you'll be able to 
re-run the reaching definitions analysis for the RHS of that assignment. If 
that definition comes from a function call, you might be able to re-run the 
reaching definitions analysis on the return statement(s) of that function (note 
that this function must have been inlined during path-sensitive analysis, 
otherwise no definition in it would coincide with the allocation site). And so 
on.

This problem sheds some light on how much do we want to make the reaching 
definitions analysis inter-procedural. My current guess is that we probably 
don't need to; we'd rather have this guided by re-running the 
reaching-definitions analysis based on the path-sensitive report data, than 
have the reaching-definitions analysis be inter-procedural on our own.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69813



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2002
 def gno_codeview_ghash : Flag<["-"], "gno-codeview-ghash">, 
Flags<[CoreOption]>;
+
 def ginline_line_tables : Flag<["-"], "ginline-line-tables">, 
Flags<[CoreOption]>;

Unexpected blank line here.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3237
 
+// If the user specified a default DWARF version, that takes precedent over
+// the platform default.

precedence



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1151
+  || Value > 5
+  || Value < 1)
+TC.getDriver().Diag(diag::err_drv_invalid_int_value)

Clang doesn't support DWARF v1.  I expect nobody does at this point (DWARF 2 
came out in 1993).



Comment at: clang/test/CodeGen/debug-default-version.c:1
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER3

MaskRay wrote:
> This looks like a clangDriver change. Maybe move the test to test/Driver ?
+1.  It should be sufficient to use `-###` and check what gets passed as cc1 
options.



Comment at: clang/test/CodeGen/debug-default-version.c:24
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf 
-S -emit-llvm -o - %s \

Does that actually work?  Last I checked, DWARF and COFF didn't play nicely, 
but I admit that was quite a while ago.


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

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D69822#1733226 , @probinson wrote:

> > Want to decouple setting the DWARF version from enabling/disabling 
> > generation of debug info.
>
> Um, why?


If you've got a big build system with various users opting in/out of DWARF and 
you want to migrate to DWARFv5, say, but you can't add "-gdwarf-5" to your 
build system, because that'd turn on debug info in cases that don't need it - 
but it's easier to change the default than to modify all cases that enable 
dwarf across the codebase.

Open to discussion about whether this is a good/bad idea - but it'd be useful 
for Google at least & seemed low-cost enough to go this route.


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

https://reviews.llvm.org/D69822



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Michael Liao via Phabricator via cfe-commits]

hliao marked 2 inline comments as done.
hliao added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7719
+  // Arrary types.
+  if (auto ATy = dyn_cast(Ty)) {
+auto T = ATy->getElementType();

arsenm wrote:
> No tests with arrays or structs?
> 
> It's also not immediately obvious to me that this optimization is still valid 
> if the pointer is buried in a struct
the original generic kernel pointer promotion to a global one only handles the 
pointer directly passed. From a critical workload, I found quite a few cases 
where the global pointers are passed through a by-val struct. We didn't handle 
that yet. With this case, we could start to handle that.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7719
+  // Arrary types.
+  if (auto ATy = dyn_cast(Ty)) {
+auto T = ATy->getElementType();

hliao wrote:
> arsenm wrote:
> > No tests with arrays or structs?
> > 
> > It's also not immediately obvious to me that this optimization is still 
> > valid if the pointer is buried in a struct
> the original generic kernel pointer promotion to a global one only handles 
> the pointer directly passed. From a critical workload, I found quite a few 
> cases where the global pointers are passed through a by-val struct. We didn't 
> handle that yet. With this case, we could start to handle that.
struct tests are added. From test cases, it seems to me that arry is not passed 
by value. I need to double-confirm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 16 inline comments as done.
jdoerfert added a comment.

In D69785#1732951 , @rogfer01 wrote:

> I made a small experiment lowering a `taskwait` (which is even simpler than 
> `barrier` in "lowering" complexity). It was pretty straightforward after all.


Nice, can you share the code? Was it based on the OpenMPIRBuilder (this one or 
the old one) or somehow separate?

> I have a few questions just to reassure I fully understand your vision here:

Sure thing.

> - the parameter `Loc` is currently unused but my understanding from the 
> comment in `OpenMPIRBuilder::getSrcLocStr` eventually we will convert a 
> `clang::SourceLocation` to a `llvm::Constant*` that represents the location 
> as used by KMP, right?

It is not unused everywhere (emitOMPBarrier uses it) but you are right that 
some uses are missing. The idea is that it will combine information about the 
location in LLVM-IR (which it does already) and the location in the source. The 
latter is in Clang a `clang::SourceLocation` but I want a frontend agnostic 
interface that takes only the values we need. That is, we add something along 
the lines of `StringRef FileName; unsigned LineNo, ColumnNo;` to the struct and 
use these to create the string that goes into KMP.

> - emitting a `taskwait` however has this ```lang=cpp if (auto *Region = 
> dyn_cast_or_null(CGF.CapturedStmtInfo)) 
> Region->emitUntiedSwitch(CGF); ``` but my understanding is that we can apply 
> the same scheme to these hooks as well, recursively, i.e.: progressively 
> convert them to calls to `OpenMPIRBuilder` (if needed under the flag 
> suggested above by Alexey). Does this make sense?

Yes. Eventually, the logic to create "untied switches" should move, then the 
logic surrounding it, ...
Though, I doubt we should start there as it will cause too much interaction 
with the internals of Clang.
Instead, we port task created (tied ones only), then add code that keeps track 
of untied tasks, then add the switches, ...

Does that makes sense?




Comment at: llvm/include/llvm/IR/OpenMPIRBuilder.h:51
+  /// potentially other helpers into the underlying module. Must be called
+  /// before any other other method and only once!
+  void initialize();

fghanim wrote:
> ///before any other method and only once!
Agreed.



Comment at: llvm/include/llvm/IR/OpenMPIRBuilder.h:72
+  bool CheckCancelFlag = true);
+
+  ///}

fghanim wrote:
> Suggestion: Rename to createOMPBarrier( ... ); - similar naming scheme to 
> IRBuilder, MDBuilder, etc.
Fine with me.



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:24
+
+Function *OpenMPIRBuilder::getRuntimeFunction(RTLFnKind FnID) {
+  Function *Fn = nullptr;

fpetrogalli wrote:
> Mark this method as `const`? It doesn't seem to change any of the fields of 
> the instance.
Some methods could be made const (and I will) but most of them not (they modify 
maps and the builder).
Making them const has only little benefit but I'll add it where appropriate 
anyway. There is little benefit because we basically do not hand out a `const 
OpenMPIRBuilder` to anyone anyway.



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:29-48
+#define OMP_RTL(Enum, Str, IsVarArg, ReturnType, ...)  
\
+  case Enum:   
\
+Fn = M.getFunction(Str);   
\
+break;
+#include "llvm/IR/OpenMPKinds.def"
+  }
+

fpetrogalli wrote:
> Why not use `getorInsertFunction` directly instead of splitting the two cases?
because this way we can define attributes easily for a subset of RT 
declarations separate from the type definition. Arguably, we can merge 
everything into a big macro that defines name, type and attributes but I 
figured this is easier to maintain. (The cost is not important as we pay it 
once per function at most and the switches should be eliminated). Can be 
changed later if we determine attributes will become part of all runtime 
function declarations.



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:177
+void OpenMPIRBuilder::emitBarrierImpl(const LocationDescription ,
+  DirektiveKind Kind, bool CheckCancelFlag,
+  bool ForceSimpleCall) {

fghanim wrote:
> DirectiveKind
(No need to repeat such comments, I will change the type name everywhere once I 
update ;))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124
+  if (const SymbolicRegion *SR = DestMR->getSymbolicBase())
+if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR))
+  return exprToStr(SizeExpr, C);

Charusso wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Again, you will have to highlight the allocation site with a note. 
> > > Therefore you will have to write a bug visitor that traverses the size 
> > > expression at some point (or, equivalently, a note tag when the size 
> > > expression is evaluated). Therefore you don't need to store the 
> > > expression in the program state.
> > Yes, you have pointed out the necessary visitor, but it needs more thinking.
> > 
> > I have a memory region which could be any kind of "memory block region" 
> > therefore I have no idea where is the size expression. We are supporting 
> > ~20 different allocations, which is nothing compared to the wild with the 
> > not so uncommon 5+ parameter allocators. Therefore I still do not want to 
> > reverse engineer a small MallocChecker + ExprEngine + 
> > BuiltinFunctionChecker inside my checker. They provide the necessary 
> > `DynamicSizeInfo` easily, which could be used in at least 4 checkers at the 
> > moment (which I have pointed out earlier in D68725).
> > 
> > If I have the size expression in the dynamic size map, and I can clearly 
> > point out the destination buffer, it is a lot more simplified to traverse 
> > the graph where the buffer and its size comes from.
> Well, you really do not want to store `SizeExpr` of `malloc(SizeExpr)` and 
> you are right I will have to traverse from it to see whether the `SizeExpr` 
> is ambiguous or not, where it comes from.
> 
> I want to rely on the `trackExpressionValue()` as the `SizeExpr` is available 
> by `getDynamicSizeExpr()`, so it is one or two lines of code.
> 
> Would you create your own switch-case to see where is the size expression 
> goes in the allocation and use `trackExpressionValue()` on it? So that you do 
> not store information in the global state which results in better run-time / 
> less memory.
> 
> At first I really wanted to model `malloc()` and `realloc()` and stuff, then 
> I realized the `MallocChecker` provides every information I need. Would it be 
> a better idea to create my own tiny `MallocChecker` inside my checker which 
> does nothing but marks the size expression being interesting with `NoteTags`?
> 
> Also I am thinking of a switch-case on the `DefinedOrUnknownSVal Size` which 
> somewhere has an expression inside it which I could `trackExpressionValue()` 
> on.
> 
> Basically we are missing the rules what to use and I have picked the easiest 
> solution. Could you share please which would be the right direction for such 
> a simple task?
> I want to rely on the `trackExpressionValue()` as the `SizeExpr` is available 
> by `getDynamicSizeExpr()`, so it is one or two lines of code.

This won't work. `trackExpressionValue()` can only track an active expression 
(that has, or at least should have, a value in the bug-node's environment). 
You'll have to make a visitor or a note tag.

You can either make your own visitor (which will detect the node in which the 
extent information becomes present), or convert `MallocChecker` to use note 
tags and then inter-operate with those tags (though the interestingness map - 
"i mark the symbol as interesting so i'm interested in highlighting the 
allocation site" - or a similar mechanism). The second approach is more work 
because no such interoperation has ever been implemented yet, but it should be 
pretty rewarding for the future.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69813



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 227784.
hliao added a comment.

add the test case for struct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu

Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -x hip %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+
+// Coerced struct from `struct S` without all generic pointers lowered into
+// global ones.
+// CHECK: %struct.S.coerce = type { i32 addrspace(1)*, float addrspace(1)* }
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
+__global__ void kernel1(int *x) {
+  x[0]++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* dereferenceable(4) %x.coerce)
+__global__ void kernel2(int ) {
+  x++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
+__global__ void kernel3(__attribute__((address_space(2))) int *x,
+__attribute__((address_space(1))) int *y) {
+  y[0] = x[0];
+}
+
+// CHECK: define void @_Z4funcPi(i32* %x)
+__device__ void func(int *x) {
+  x[0]++;
+}
+
+struct S {
+  int *x;
+  float *y;
+};
+// `by-val` struct will be coerced into a similar struct with all generic
+// pointers lowerd into global ones.
+// CHECK: define amdgpu_kernel void @_Z7kernel41S(%struct.S.coerce %s.coerce)
+__global__ void kernel4(struct S s) {
+  s.x[0]++;
+  s.y[0] += 1.f;
+}
+
+// If a pointer to struct is passed, only the pointer itself is coerced into the global one.
+// CHECK: define amdgpu_kernel void @_Z7kernel5P1S(%struct.S addrspace(1)* %s.coerce)
+__global__ void kernel5(struct S *s) {
+  s->x[0]++;
+  s->y[0] += 1.f;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7685,6 +7685,53 @@
   bool isHomogeneousAggregateSmallEnough(const Type *Base,
  uint64_t Members) const override;
 
+  // Coercion type builder for lower HIP pointer argument from generic pointer
+  // to global pointer.
+  class CoerceGenericPointerTypeBuilder {
+llvm::LLVMContext 
+unsigned DefaultAS;
+unsigned GlobalAS;
+
+  public:
+CoerceGenericPointerTypeBuilder(llvm::LLVMContext , unsigned DAS,
+unsigned GAS)
+: Context(VMCtx), DefaultAS(DAS), GlobalAS(GAS) {}
+
+llvm::Type *coerce(llvm::Type *Ty) {
+  // Structure types.
+  if (auto STy = dyn_cast(Ty)) {
+SmallVector EltTys;
+bool Changed = false;
+for (auto T : STy->elements()) {
+  auto NT = coerce(T);
+  EltTys.push_back(NT);
+  Changed |= (NT != T);
+}
+// Skip if there is no change in element types.
+if (!Changed)
+  return STy;
+if (STy->hasName())
+  return llvm::StructType::create(
+  EltTys, (STy->getName() + ".coerce").str(), STy->isPacked());
+return llvm::StructType::get(Context, EltTys, STy->isPacked());
+  }
+  // Arrary types.
+  if (auto ATy = dyn_cast(Ty)) {
+auto T = ATy->getElementType();
+auto NT = coerce(T);
+// Skip if there is no change in that element type.
+if (NT == T)
+  return ATy;
+return llvm::ArrayType::get(NT, ATy->getNumElements());
+  }
+  // Single value types.
+  if (Ty->isPointerTy() && Ty->getPointerAddressSpace() == DefaultAS)
+return llvm::PointerType::get(
+cast(Ty)->getElementType(), GlobalAS);
+  return Ty;
+}
+  };
+
 public:
   explicit AMDGPUABIInfo(CodeGen::CodeGenTypes ) :
 DefaultABIInfo(CGT) {}
@@ -7812,14 +7859,23 @@
 
   // TODO: Can we omit empty structs?
 
-  // Coerce single element structs to its element.
+  llvm::Type *LTy = nullptr;
   if (const Type *SeltTy = isSingleElementStruct(Ty, getContext()))
-return ABIArgInfo::getDirect(CGT.ConvertType(QualType(SeltTy, 0)));
+LTy = CGT.ConvertType(QualType(SeltTy, 0));
+
+  if (getContext().getLangOpts().HIP) {
+if (!LTy)
+  LTy = CGT.ConvertType(Ty);
+CoerceGenericPointerTypeBuilder Builder(getVMContext(),
+getContext().getTargetAddressSpace(LangAS::Default),
+getContext().getTargetAddressSpace(LangAS::cuda_device));
+LTy = Builder.coerce(LTy);
+  }
 
   // If we set CanBeFlattened to true, CodeGen will expand the struct to its
   // individual elements, which confuses the Clover OpenCL backend; therefore we
   // have to set 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

> Want to decouple setting the DWARF version from enabling/disabling generation 
> of debug info.

Um, why?


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

https://reviews.llvm.org/D69822



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1308-1310
+  if (isa(SrcTy) &&
+  isa(DstTy) &&
+  SrcTy->getPointerAddressSpace() != DstTy->getPointerAddressSpace()) {

I would somewhat prefer 2 dyn_cast and getAddressSpace, this is essentially isa 
+ cast combo



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7719
+  // Arrary types.
+  if (auto ATy = dyn_cast(Ty)) {
+auto T = ATy->getElementType();

No tests with arrays or structs?

It's also not immediately obvious to me that this optimization is still valid 
if the pointer is buried in a struct


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7688
 
+  // Coercion type builder for lower HIP pointer argument from generic pointer
+  // to global pointer.

Nit: `for lower` -> `for lowering` or `that lowers`



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7690
+  // to global pointer.
+  class CoerceGenericPointerTypeBuilder {
+llvm::LLVMContext 

I don't think you need a class here -- it just complicates calling of coerce().
I'd just make `coerce()` a member function.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:7696
+  public:
+CoerceGenericPointerTypeBuilder(llvm::LLVMContext , unsigned DAS,
+unsigned GAS)

Nit: `VM` in `VMCtx` is not useful. `Ctx` or `LLVMCtx` would be better, IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826



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

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124
+  if (const SymbolicRegion *SR = DestMR->getSymbolicBase())
+if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR))
+  return exprToStr(SizeExpr, C);

Charusso wrote:
> NoQ wrote:
> > Again, you will have to highlight the allocation site with a note. 
> > Therefore you will have to write a bug visitor that traverses the size 
> > expression at some point (or, equivalently, a note tag when the size 
> > expression is evaluated). Therefore you don't need to store the expression 
> > in the program state.
> Yes, you have pointed out the necessary visitor, but it needs more thinking.
> 
> I have a memory region which could be any kind of "memory block region" 
> therefore I have no idea where is the size expression. We are supporting ~20 
> different allocations, which is nothing compared to the wild with the not so 
> uncommon 5+ parameter allocators. Therefore I still do not want to reverse 
> engineer a small MallocChecker + ExprEngine + BuiltinFunctionChecker inside 
> my checker. They provide the necessary `DynamicSizeInfo` easily, which could 
> be used in at least 4 checkers at the moment (which I have pointed out 
> earlier in D68725).
> 
> If I have the size expression in the dynamic size map, and I can clearly 
> point out the destination buffer, it is a lot more simplified to traverse the 
> graph where the buffer and its size comes from.
Well, you really do not want to store `SizeExpr` of `malloc(SizeExpr)` and you 
are right I will have to traverse from it to see whether the `SizeExpr` is 
ambiguous or not, where it comes from.

I want to rely on the `trackExpressionValue()` as the `SizeExpr` is available 
by `getDynamicSizeExpr()`, so it is one or two lines of code.

Would you create your own switch-case to see where is the size expression goes 
in the allocation and use `trackExpressionValue()` on it? So that you do not 
store information in the global state which results in better run-time / less 
memory.

At first I really wanted to model `malloc()` and `realloc()` and stuff, then I 
realized the `MallocChecker` provides every information I need. Would it be a 
better idea to create my own tiny `MallocChecker` inside my checker which does 
nothing but marks the size expression being interesting with `NoteTags`?

Also I am thinking of a switch-case on the `DefinedOrUnknownSVal Size` which 
somewhere has an expression inside it which I could `trackExpressionValue()` on.

Basically we are missing the rules what to use and I have picked the easiest 
solution. Could you share please which would be the right direction for such a 
simple task?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69813



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

[PATCH] D69743: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D69743#1733101 , @vsk wrote:

> In D69743#1732967 , @dblaikie wrote:
>
> > What's the extra DWARF that's emitted from the backend? Is there a 
> > corresponding LLVM code change/test, or is the functionality there but 
> > dormant in this particular case? (& already running/tested in other cases, 
> > such as when fn1 is defined in the same CU)
>
>
> The extra DWARF would look like:
>
>   DW_TAG_subprogram
>   DW_AT_name  ("fn1")
>   DW_AT_declaration   (true)
>
>
> This clang change has no corresponding llvm logic change. The functionality 
> to emit a subprogram for a declaration is already present & tested in llvm, 
> as is the logic to use such a subprogram to create a TAG_call_site 
> (llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll is a test, and the 
> logic is in `DwarfDebug::constructCallSiteEntryDIEs`). That test should cover 
> the case where a function has no subprogram, though.


OK, thanks for the context!


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

https://reviews.llvm.org/D69743



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

[PATCH] D69818: [HIP] Fix pointer type kernel arg for amdgpu

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl abandoned this revision.
yaxunl added a comment.

Please review Michael's patch https://reviews.llvm.org/D69826 which supersedes 
this one. Thanks.


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

https://reviews.llvm.org/D69818



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

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked 3 inline comments as done.
aganea added a comment.

A few remarks:




Comment at: clang/lib/Driver/Job.cpp:319
 
+LLVM_THREAD_LOCAL bool Command::ReenterTool = true;
+

See my other comment about `LLVM_THREAD_LOCAL`



Comment at: clang/tools/driver/driver.cpp:338
+int ClangDriverMain(SmallVectorImpl ) {
+  static LLVM_THREAD_LOCAL bool EnterPE = true;
+  if (EnterPE) {

`LLVM_THREAD_LOCAL` is here for the `llvm-buildozer` application I've presented 
at the LLVM conference. It's there because `main()` is entered from several 
threads, thus all clang needs to be thread-safe (I have several other patches 
to achieve that).
If you want this patch without `LLVM_THREAD_LOCAL` I'm fine with that.



Comment at: llvm/lib/Support/Windows/Signals.inc:814
+
+  return EXCEPTION_EXECUTE_HANDLER;
 }

When this function is entered through the global exception handler, returning 
`EXCEPTION_EXECUTE_HANDLER` lets the CRT do its own exit with the exception 
code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

we need a test for byval struct and array. better to have a struct containing 
an array which contain another struct which contains a pointer. thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826



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

[PATCH] D69818: [HIP] Fix pointer type kernel arg for amdgpu

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked an inline comment as done.
yaxunl added a comment.

BTW Michael Liao will create another patch which will supersede this patch. 
That patch contains similar changes and also handles pointers in a byval struct 
or array.




Comment at: clang/lib/CodeGen/CGCall.cpp:1172
 if (isa(Ty))
-  return CGF.Builder.CreateBitCast(Val, Ty, "coerce.val");
+  return CGF.Builder.CreatePointerCast(Val, Ty, "coerce.val");
 

tra wrote:
> What's the purpose of this change?
this is supposed to cast the coerced type to the original type. Previously, no 
coerced type changes the address space, therefore a bitcast is sufficient. Now 
we coerce a type to different address space, therefore we need a pointer cast.


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

https://reviews.llvm.org/D69818



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

It happens that Sam has a similar patch of this one. After discussion, we 
agreed that this patch addresses more cases found in the workloads. Thank Sam 
for the test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69826



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

[PATCH] D69826: [hip] Enable pointer argument lowering through coercing type.

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added reviewers: tra, rjmccall, yaxunl.
Herald added subscribers: cfe-commits, nhaehnle, jvesely.
Herald added a project: clang.
hliao added a comment.

It happens that Sam has a similar patch of this one. After discussion, we 
agreed that this patch addresses more cases found in the workloads. Thank Sam 
for the test case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69826

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu

Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -x hip %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+// CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
+__global__ void kernel1(int *x) {
+  x[0]++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* dereferenceable(4) %x.coerce)
+__global__ void kernel2(int ) {
+  x++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
+__global__ void kernel3(__attribute__((address_space(2))) int *x,
+__attribute__((address_space(1))) int *y) {
+  y[0] = x[0];
+}
+
+// CHECK: define void @_Z4funcPi(i32* %x)
+__device__ void func(int *x) {
+  x[0]++;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7685,6 +7685,53 @@
   bool isHomogeneousAggregateSmallEnough(const Type *Base,
  uint64_t Members) const override;
 
+  // Coercion type builder for lower HIP pointer argument from generic pointer
+  // to global pointer.
+  class CoerceGenericPointerTypeBuilder {
+llvm::LLVMContext 
+unsigned DefaultAS;
+unsigned GlobalAS;
+
+  public:
+CoerceGenericPointerTypeBuilder(llvm::LLVMContext , unsigned DAS,
+unsigned GAS)
+: Context(VMCtx), DefaultAS(DAS), GlobalAS(GAS) {}
+
+llvm::Type *coerce(llvm::Type *Ty) {
+  // Structure types.
+  if (auto STy = dyn_cast(Ty)) {
+SmallVector EltTys;
+bool Changed = false;
+for (auto T : STy->elements()) {
+  auto NT = coerce(T);
+  EltTys.push_back(NT);
+  Changed |= (NT != T);
+}
+// Skip if there is no change in element types.
+if (!Changed)
+  return STy;
+if (STy->hasName())
+  return llvm::StructType::create(
+  EltTys, (STy->getName() + ".coerce").str(), STy->isPacked());
+return llvm::StructType::get(Context, EltTys, STy->isPacked());
+  }
+  // Arrary types.
+  if (auto ATy = dyn_cast(Ty)) {
+auto T = ATy->getElementType();
+auto NT = coerce(T);
+// Skip if there is no change in that element type.
+if (NT == T)
+  return ATy;
+return llvm::ArrayType::get(NT, ATy->getNumElements());
+  }
+  // Single value types.
+  if (Ty->isPointerTy() && Ty->getPointerAddressSpace() == DefaultAS)
+return llvm::PointerType::get(
+cast(Ty)->getElementType(), GlobalAS);
+  return Ty;
+}
+  };
+
 public:
   explicit AMDGPUABIInfo(CodeGen::CodeGenTypes ) :
 DefaultABIInfo(CGT) {}
@@ -7812,14 +7859,23 @@
 
   // TODO: Can we omit empty structs?
 
-  // Coerce single element structs to its element.
+  llvm::Type *LTy = nullptr;
   if (const Type *SeltTy = isSingleElementStruct(Ty, getContext()))
-return ABIArgInfo::getDirect(CGT.ConvertType(QualType(SeltTy, 0)));
+LTy = CGT.ConvertType(QualType(SeltTy, 0));
+
+  if (getContext().getLangOpts().HIP) {
+if (!LTy)
+  LTy = CGT.ConvertType(Ty);
+CoerceGenericPointerTypeBuilder Builder(getVMContext(),
+getContext().getTargetAddressSpace(LangAS::Default),
+getContext().getTargetAddressSpace(LangAS::cuda_device));
+LTy = Builder.coerce(LTy);
+  }
 
   // If we set CanBeFlattened to true, CodeGen will expand the struct to its
   // individual elements, which confuses the Clover OpenCL backend; therefore we
   // have to set it to false here. Other args of getDirect() are just defaults.
-  return ABIArgInfo::getDirect(nullptr, 0, nullptr, false);
+  return ABIArgInfo::getDirect(LTy, 0, nullptr, false);
 }
 
 ABIArgInfo AMDGPUABIInfo::classifyArgumentType(QualType Ty,
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1305,6 +1305,14 @@
 DstTy = Dst.getType()->getElementType();
   }
 
+  if (isa(SrcTy) &&
+  isa(DstTy) &&
+  

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3247-3248
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;
 

cmtice wrote:
> dblaikie wrote:
> > Hmm, actually - why is this case necessary/what does it cover? I was hoping 
> > the case you added inside the "if (EmitDwarf)" case above would be all that 
> > was required (& the call to ParseDebugDefaultVersion would go inside there 
> > at the use, to reduce the variable scope/keep the code closer together).
> This covers the case where you are setting the default dwarf version without 
> actually turning on/off debug info at all (there's no -gdwarf or -g option, 
> so EmitDwarf is false).
Ah, perhaps I'm missing something - why should the DWARFVersion be set if 
EmitDwarf is false? No DWARF was requested, so the DWARFVersion probably 
shouldn't be set, I think?


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

https://reviews.llvm.org/D69822



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

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 227779.
aganea added a comment.
Herald added a subscriber: dexonsmith.

Fix missing `llvm::InitializeAllTargets()` in driver.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69825

Files:
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/tools/driver/driver.cpp
  llvm/include/llvm/Support/CrashRecoveryContext.h
  llvm/lib/Support/CrashRecoveryContext.cpp
  llvm/lib/Support/Windows/Signals.inc

Index: llvm/lib/Support/Windows/Signals.inc
===
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -187,7 +187,7 @@
 using namespace llvm;
 
 // Forward declare.
-static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
+LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType);
 
 // The function to call if ctrl-c is pressed.
@@ -521,10 +521,13 @@
 extern "C" VOID WINAPI RtlCaptureContext(PCONTEXT ContextRecord);
 #endif
 
-void llvm::sys::PrintStackTrace(raw_ostream ) {
-  STACKFRAME64 StackFrame = {};
-  CONTEXT Context = {};
-  ::RtlCaptureContext();
+static void LocalPrintStackTrace(raw_ostream , PCONTEXT C) {
+  STACKFRAME64 StackFrame{};
+  CONTEXT Context{};
+  if (!C) {
+::RtlCaptureContext();
+C = 
+  }
 #if defined(_M_X64)
   StackFrame.AddrPC.Offset = Context.Rip;
   StackFrame.AddrStack.Offset = Context.Rsp;
@@ -546,9 +549,12 @@
   StackFrame.AddrStack.Mode = AddrModeFlat;
   StackFrame.AddrFrame.Mode = AddrModeFlat;
   PrintStackTraceForThread(OS, GetCurrentProcess(), GetCurrentThread(),
-   StackFrame, );
+   StackFrame, C);
 }
 
+void llvm::sys::PrintStackTrace(raw_ostream ) {
+  LocalPrintStackTrace(OS, nullptr);
+}
 
 void llvm::sys::SetInterruptFunction(void (*IF)()) {
   RegisterHandler();
@@ -785,7 +791,7 @@
   return std::error_code();
 }
 
-static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
+LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
   Cleanup();
 
   // We'll automatically write a Minidump file here to help diagnose
@@ -803,42 +809,9 @@
<< "\n";
   }
 
-  // Initialize the STACKFRAME structure.
-  STACKFRAME64 StackFrame = {};
-
-#if defined(_M_X64)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Rip;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Rsp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Rbp;
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#elif defined(_M_IX86)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Eip;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Esp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Ebp;
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#elif defined(_M_ARM64) || defined(_M_ARM)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Pc;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Sp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-#if defined(_M_ARM64)
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Fp;
-#else
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->R11;
-#endif
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#endif
-
-  HANDLE hProcess = GetCurrentProcess();
-  HANDLE hThread = GetCurrentThread();
-  PrintStackTraceForThread(llvm::errs(), hProcess, hThread, StackFrame,
-   ep->ContextRecord);
-
-  _exit(ep->ExceptionRecord->ExceptionCode);
+  LocalPrintStackTrace(llvm::errs(), ep->ContextRecord);
+
+  return EXCEPTION_EXECUTE_HANDLER;
 }
 
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) {
Index: llvm/lib/Support/CrashRecoveryContext.cpp
===
--- llvm/lib/Support/CrashRecoveryContext.cpp
+++ llvm/lib/Support/CrashRecoveryContext.cpp
@@ -13,6 +13,9 @@
 #include "llvm/Support/ThreadLocal.h"
 #include 
 #include 
+#if defined(_WIN32)
+#include 
+#endif
 using namespace llvm;
 
 namespace {
@@ -54,7 +57,7 @@
 #endif
   }
 
-  void HandleCrash() {
+  void HandleCrash(int retCode) {
 // Eliminate the current context entry, to avoid re-entering in case the
 // cleanup code crashes.
 CurrentContext->set(Next);
@@ -62,6 +65,8 @@
 assert(!Failed && "Crash recovery context already failed!");
 Failed = true;
 
+CRC->RetCode = retCode;
+
 // FIXME: Stash the backtrace.
 
 // Jump back to the RunSafely we were called under.
@@ -171,6 +176,9 @@
 static void installExceptionOrSignalHandlers() {}
 static void uninstallExceptionOrSignalHandlers() {}
 
+// In Signals.inc
+LONG WINAPI 

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

Hi @jdoerfert , thank you for working on this!

I have added some minor comments.

Francesco




Comment at: clang/lib/CodeGen/CodeGenModule.h:593
+  llvm::OpenMPIRBuilder () {
+assert(OMPBuilder != nullptr);
+return *OMPBuilder;

Nit: wouldn't the following be more informative? (and, also, I thought it was 
preferred style for LLVM codebase)

```
assert(OMPBuilder && "Invalid OMPBuilder instance");
```



Comment at: llvm/include/llvm/IR/OpenMPIRBuilder.h:27
+  /// IDs for all omp runtime library (RTL) functions.
+  enum RTLFnKind {
+#define OMP_RTL(Enum, ...) Enum,

I'd prefer use `enum class` instead of enums. If needed in some interface, it 
make it easier to see the actual type instead of a plain `unsigned`. No strong 
opinion though.



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:24
+
+Function *OpenMPIRBuilder::getRuntimeFunction(RTLFnKind FnID) {
+  Function *Fn = nullptr;

Mark this method as `const`? It doesn't seem to change any of the fields of the 
instance.



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:29-48
+#define OMP_RTL(Enum, Str, IsVarArg, ReturnType, ...)  
\
+  case Enum:   
\
+Fn = M.getFunction(Str);   
\
+break;
+#include "llvm/IR/OpenMPKinds.def"
+  }
+

Why not use `getorInsertFunction` directly instead of splitting the two cases?



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:85
+
+Value *OpenMPIRBuilder::getIdent(Constant *SrcLocStr, unsigned LocFlags) {
+  // Enable "C-mode".

`const` method?



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:113
+
+Constant *OpenMPIRBuilder::getSrcLocStr(std::string LocStr) {
+  Constant * = SrcLocStrMap[LocStr];

`const` method?



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:131
+
+Constant *OpenMPIRBuilder::getDefaultSrcLocStr() {
+  return getSrcLocStr(";unknown;unknown;0;0;;");

`const` method?



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:140
+
+Value *OpenMPIRBuilder::getThreadID(Value *Ident) {
+  // TODO: It makes only so much sense to actually cache the global_thread_num

`const` method?



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:168
+
+void OpenMPIRBuilder::emitOMPBarrier(const LocationDescription ,
+ DirektiveKind DK, bool CheckCancelFlag) {

`const` method?



Comment at: llvm/lib/IR/OpenMPIRBuilder.cpp:173
+  return emitBarrierImpl(Loc, DK, CheckCancelFlag,
+ /* ForceSimpleCall */ false);
+}

Nit!

```
/*ForceSimpleCall=*/ false);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D68969: [clang-format] Remove the dependency on frontend

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D68969#1732489 , @thakis wrote:

> Since this was reverted: are you looking into relanding this?


I'm not totally sure how they reproduced the UBSAN issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68969



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

[PATCH] D68590: [clangd] Improve hover support for Objective-C

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

We'll also want to do something similar for `DocumentSymbols`, see here 
,
 which will lead to Objective-C categories showing up as either `(anonymous)` 
or ``


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68590



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

[PATCH] D69763: [Clang][Test]: Remaining "lld-link2" -> "lld-link"

[Saleem Abdulrasool via Phabricator via cfe-commits]

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

Seems like a good idea since lld link2 has been made into lld link for a while 
now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69763



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

[PATCH] D69322: [hip][cuda] Enable extended lambda support on Windows.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

Looks like holidays are approaching, :). Anyway, it's really appreciated that 
you could review this patch to enable CUDA/HIP applications on Windows.

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69322



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

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:4995
+  verifyFormat("c()->f();");
+  verifyFormat("x()->foo<1>;");
+  verifyFormat("x = p->foo<3>();");

curdeius wrote:
> What about:
> 
> ```
> verifyFormat("x()->x<1>;");
> ```
> i.e. a function `x` returning a pointer to a class having a template member 
> `x` (for instance a template variable).
I'm unsure if we are the limit of being able to differentiate between the two?


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

https://reviews.llvm.org/D69577



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice updated this revision to Diff 22.
cmtice added a comment.

Made requested formatting changes.


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

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/CodeGen/debug-default-version.c

Index: clang/test/CodeGen/debug-default-version.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-default-version.c
@@ -0,0 +1,47 @@
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER3
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=NODEBUGINFO,VER4
+
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -target x86_64-apple-macosx10.11 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-apple-darwin14 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-apple-macosx10.11 -fdebug-default-version=5 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-apple-darwin14 -g -fdebug-default-version=4 -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+
+// RUN: %clang -target powerpc-unknown-openbsd -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target powerpc-unknown-freebsd -g -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target i386-pc-solaris -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+
+int main (void) {
+  return 0;
+}
+
+// NODEBUGINFO-NOT: !llvm.dbg.cu = !{!0}
+
+// NOCODEVIEW-NOT: !"CodeView"
+
+// VER2: !{i32 2, !"Dwarf Version", i32 2}
+// VER3: !{i32 2, !"Dwarf Version", i32 3}
+// VER4: !{i32 2, !"Dwarf Version", i32 4}
+// VER5: !{i32 2, !"Dwarf Version", i32 5}
+
+// NODWARF-NOT: !"Dwarf Version"
+// CODEVIEW: !{i32 2, !"CodeView", i32 1}
+// NOCODEVIEW-NOT: !"CodeView"
+// NODWARF-NOT: !"Dwarf Version"
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+ const ArgList ) {
+  const Arg *A = Args.getLastArg(options::OPT_fdebug_default_version);
+
+  if (!A)
+return 0;
+
+  unsigned Value = 0;
+  if (StringRef(A->getValue()).getAsInteger(10, Value)
+  || Value > 5
+  || Value < 1)
+

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Alexandre Ganea via Phabricator via cfe-commits]

aganea created this revision.
aganea added reviewers: thakis, hans, rnk.
aganea added a project: clang.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added a reviewer: jfb.
Herald added a project: LLVM.

This patch is an optimization for speed: it will bypass the cc1 process 
invocation when possible, and instead will re-enter the driver process.
On Windows, this has a great impact on build times:

F10626773: clang-bypass-cc1-6-core.png 

F10626775: clang-bypass-cc1-36-core.png 

CFG (control flow guard) is disabled on Windows.
//Side-note: I've also observed an improvement when switching to Windows 10 
build 1903: all builds are faster, generally by ~2-3 min on the 6-core 
machine.//

The cc1 bypass mildly improves build times on Linux as well (although it's a 
old machine):

F10626777: clang-bypass-cc1-6-core-linux.png 


All tests pass (ninja check-all) on Windows with the following config:

  cmake -G"Ninja" %ROOT%/llvm -DCMAKE_BUILD_TYPE=Release 
-DLLVM_OPTIMIZED_TABLEGEN=true -DLLVM_ENABLE_ASSERTIONS=ON 
-DLLVM_ENABLE_LIBXML2=OFF -DLLVM_USE_CRT_RELEASE=MT 
-DCMAKE_C_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" 
-DCMAKE_CXX_COMPILER="C:/Program Files/LLVM/bin/clang-cl.exe" 
-DCMAKE_LINKER="c:/Program Files/LLVM/bin/lld-link.exe" -DLLVM_ENABLE_LLD=true 
-DCMAKE_CXX_FLAGS="/arch:AVX /GS- /D_ITERATOR_DEBUG_LEVEL=0" 
-DCMAKE_C_FLAGS="/arch:AVX /GS- /D_ITERATOR_DEBUG_LEVEL=0" 
-DLLVM_ENABLE_PDB=true 
-DLLVM_ENABLE_PROJECTS="clang;lld;clang-tools-extra;compiler-rt"

..then followed by a second stage build.

As for re-entering `main()` in the code:
In some cases, when using clang-as-a-library, we cannot re-enter `main()`; and 
we shouldn't take the address of `main()` as per [C++11: 3.6.1/3] (that's what 
I meant at the LLVM conf. by saying that "we shouldn't be doing that"). I 
therefore used `llvm::sys::DynamicLibrary::AddSymbol` to keep a weak link to 
`main()`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69825

Files:
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/tools/driver/driver.cpp
  llvm/include/llvm/Support/CrashRecoveryContext.h
  llvm/lib/Support/CrashRecoveryContext.cpp
  llvm/lib/Support/Windows/Signals.inc

Index: llvm/lib/Support/Windows/Signals.inc
===
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -187,7 +187,7 @@
 using namespace llvm;
 
 // Forward declare.
-static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
+LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType);
 
 // The function to call if ctrl-c is pressed.
@@ -521,10 +521,13 @@
 extern "C" VOID WINAPI RtlCaptureContext(PCONTEXT ContextRecord);
 #endif
 
-void llvm::sys::PrintStackTrace(raw_ostream ) {
-  STACKFRAME64 StackFrame = {};
-  CONTEXT Context = {};
-  ::RtlCaptureContext();
+static void LocalPrintStackTrace(raw_ostream , PCONTEXT C) {
+  STACKFRAME64 StackFrame{};
+  CONTEXT Context{};
+  if (!C) {
+::RtlCaptureContext();
+C = 
+  }
 #if defined(_M_X64)
   StackFrame.AddrPC.Offset = Context.Rip;
   StackFrame.AddrStack.Offset = Context.Rsp;
@@ -546,9 +549,12 @@
   StackFrame.AddrStack.Mode = AddrModeFlat;
   StackFrame.AddrFrame.Mode = AddrModeFlat;
   PrintStackTraceForThread(OS, GetCurrentProcess(), GetCurrentThread(),
-   StackFrame, );
+   StackFrame, C);
 }
 
+void llvm::sys::PrintStackTrace(raw_ostream ) {
+  LocalPrintStackTrace(OS, nullptr);
+}
 
 void llvm::sys::SetInterruptFunction(void (*IF)()) {
   RegisterHandler();
@@ -785,7 +791,7 @@
   return std::error_code();
 }
 
-static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
+LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
   Cleanup();
 
   // We'll automatically write a Minidump file here to help diagnose
@@ -803,42 +809,9 @@
<< "\n";
   }
 
-  // Initialize the STACKFRAME structure.
-  STACKFRAME64 StackFrame = {};
-
-#if defined(_M_X64)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Rip;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Rsp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Rbp;
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#elif defined(_M_IX86)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Eip;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Esp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Ebp;
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#elif defined(_M_ARM64) || defined(_M_ARM)
-  StackFrame.AddrPC.Offset = 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/CodeGen/debug-default-version.c:2
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER3
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER4

`VER3` -> `DWARF3` may be better, because you also check another debug info 
format codeview here.


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

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/CodeGen/debug-default-version.c:1
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S 
-emit-llvm -o - %s | FileCheck %s --check-prefix=VER3

This looks like a clangDriver change. Maybe move the test to test/Driver ?


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

https://reviews.llvm.org/D69822



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

[PATCH] D69818: [HIP] Fix pointer type kernel arg for amdgpu

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1172
 if (isa(Ty))
-  return CGF.Builder.CreateBitCast(Val, Ty, "coerce.val");
+  return CGF.Builder.CreatePointerCast(Val, Ty, "coerce.val");
 

What's the purpose of this change?



Comment at: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu:15-16
+// CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 
addrspace(2)* %x, i32 addrspace(1)* %y)
+__global__ void kernel3(__attribute__((address_space(2))) int *x,
+__attribute__((address_space(1))) int *y) {
+  y[0] = x[0];

Interesting. Clang used to crash on explicit address space attribute in the 
past. I'm glad it works now.


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

https://reviews.llvm.org/D69818



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

[PATCH] D69598: Work on cleaning up denormal mode handling

[Matt Arsenault via Phabricator via cfe-commits]

arsenm updated this revision to Diff 227775.
arsenm retitled this revision from "WIP: Work on cleaning up denormal mode 
handling" to "Work on cleaning up denormal mode handling".
arsenm added a comment.

Defer any behavior changes until a future patch, so all tests now pass


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

https://reviews.llvm.org/D69598

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/include/llvm/ADT/FloatingPointMode.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/CodeGen/SelectionDAG.h
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/unittests/ADT/CMakeLists.txt
  llvm/unittests/ADT/FloatingPointMode.cpp

Index: llvm/unittests/ADT/FloatingPointMode.cpp
===
--- /dev/null
+++ llvm/unittests/ADT/FloatingPointMode.cpp
@@ -0,0 +1,33 @@
+//===- llvm/unittest/ADT/FloatingPointMode.cpp ===//
+//
+// 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 "llvm/ADT/FloatingPointMode.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+TEST(FloatingPointModeTest, ParseSubnormalFPAttribute) {
+  EXPECT_EQ(SubnormalMode::IEEE, parseSubnormalFPAttribute("ieee"));
+  EXPECT_EQ(SubnormalMode::IEEE, parseSubnormalFPAttribute(""));
+  EXPECT_EQ(SubnormalMode::PreserveSign,
+parseSubnormalFPAttribute("preserve-sign"));
+  EXPECT_EQ(SubnormalMode::PositiveZero,
+parseSubnormalFPAttribute("positive-zero"));
+  EXPECT_EQ(SubnormalMode::Invalid, parseSubnormalFPAttribute("foo"));
+}
+
+TEST(FloatingPointModeTest, SubnormalAttributeName) {
+  EXPECT_EQ("ieee", subnormalModeName(SubnormalMode::IEEE));
+  EXPECT_EQ("preserve-sign", subnormalModeName(SubnormalMode::PreserveSign));
+  EXPECT_EQ("positive-zero", subnormalModeName(SubnormalMode::PositiveZero));
+  EXPECT_EQ("", subnormalModeName(SubnormalMode::Invalid));
+}
+
+}
Index: llvm/unittests/ADT/CMakeLists.txt
===
--- llvm/unittests/ADT/CMakeLists.txt
+++ llvm/unittests/ADT/CMakeLists.txt
@@ -20,6 +20,7 @@
   DirectedGraphTest.cpp
   EquivalenceClassesTest.cpp
   FallibleIteratorTest.cpp
+  FloatingPointMode.cpp
   FoldingSet.cpp
   FunctionExtrasTest.cpp
   FunctionRefTest.cpp
Index: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -20464,9 +20464,8 @@
 SDLoc DL(Op);
 EVT CCVT = getSetCCResultType(VT);
 ISD::NodeType SelOpcode = VT.isVector() ? ISD::VSELECT : ISD::SELECT;
-const Function  = DAG.getMachineFunction().getFunction();
-Attribute Denorms = F.getFnAttribute("denormal-fp-math");
-if (Denorms.getValueAsString().equals("ieee")) {
+SubnormalMode SubnormMode = DAG.getSubnormalMode(VT);
+if (SubnormMode == SubnormalMode::IEEE) {
   // fabs(X) < SmallestNormal ? 0.0 : Est
   const fltSemantics  = DAG.EVTToAPFloatSemantics(VT);
   APFloat SmallestNorm = APFloat::getSmallestNormalized(FltSem);
Index: llvm/lib/CodeGen/MachineFunction.cpp
===
--- llvm/lib/CodeGen/MachineFunction.cpp
+++ llvm/lib/CodeGen/MachineFunction.cpp
@@ -270,6 +270,21 @@
   return JumpTableInfo;
 }
 
+SubnormalMode MachineFunction::getSubnormalMode(const fltSemantics ) const {
+  // TODO: Should probably avoid the connection to the IR and store directly
+  // in the MachineFunction.
+  Attribute Attr = F.getFnAttribute("denormal-fp-math");
+
+  // FIXME: This should assume IEEE behavior on an unspecified
+  // attribute. However, the one current user incorrectly assumes a non-IEEE
+  // target by default.
+  StringRef Val = Attr.getValueAsString();
+  if (Val.empty())
+return SubnormalMode::Invalid;
+
+  return parseSubnormalFPAttribute(Val);
+}
+
 /// Should we be emitting segmented stack stuff for the function
 bool MachineFunction::shouldSplitStack() const {
   return getFunction().hasFnAttribute("split-stack");
Index: llvm/include/llvm/CodeGen/SelectionDAG.h
===
--- llvm/include/llvm/CodeGen/SelectionDAG.h
+++ llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -1711,6 +1711,12 @@
 return It->second.HeapAllocSite;
   }
 
+  /// Return the current function's default subnormal handling kind for the
+  /// given floating point type.
+  SubnormalMode getSubnormalMode(EVT VT) const {
+return 

[PATCH] D69743: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In D69743#1732967 , @dblaikie wrote:

> What's the extra DWARF that's emitted from the backend? Is there a 
> corresponding LLVM code change/test, or is the functionality there but 
> dormant in this particular case? (& already running/tested in other cases, 
> such as when fn1 is defined in the same CU)


The extra DWARF would look like:

  DW_TAG_subprogram
  DW_AT_name("fn1")
  DW_AT_declaration (true)

This clang change has no corresponding llvm logic change. The functionality to 
emit a subprogram for a declaration is already present & tested in llvm, as is 
the logic to use such a subprogram to create a TAG_call_site 
(llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll is a test, and the 
logic is in `DwarfDebug::constructCallSiteEntryDIEs`). That test should cover 
the case where a function has no subprogram, though.


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

https://reviews.llvm.org/D69743



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

[PATCH] D69743: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

[Vedant Kumar via Phabricator via cfe-commits]

vsk updated this revision to Diff 227774.
vsk edited the summary of this revision.
vsk added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

- Add coverage for a callee without a subprogram in 
llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll.


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

https://reviews.llvm.org/D69743

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-extern-call.c
  clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
  llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll


Index: llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
===
--- llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
+++ llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll
@@ -25,6 +25,14 @@
 
 @sink = global i32 0, align 4, !dbg !0
 
+define void @__has_no_subprogram() {
+entry:
+  %0 = load volatile i32, i32* @sink, align 4
+  %inc = add nsw i32 %0, 1
+  store volatile i32 %inc, i32* @sink, align 4
+  ret void
+}
+
 ; ASM: DW_TAG_subprogram
 ; ASM:   DW_AT_call_all_calls
 ; OBJ: [[bat_sp:.*]]: DW_TAG_subprogram
@@ -70,6 +78,7 @@
 ; OBJ: DW_AT_call_tail_call
 define void @_Z3foov() !dbg !25 {
 entry:
+  tail call void @__has_no_subprogram()
   tail call void @_Z3barv(), !dbg !26
   tail call void @_Z3batv(), !dbg !27
   tail call void @_Z3barv(), !dbg !26
Index: clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
===
--- clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
+++ clang/test/CodeGenCXX/dbg-info-all-calls-described.cpp
@@ -56,6 +56,7 @@
 
 // NO-ATTR-NOT: FlagAllCallsDescribed
 
+// HAS-ATTR-DAG: DISubprogram(name: "declaration1", {{.*}}, flags: 
DIFlagPrototyped
 // HAS-ATTR-DAG: DISubprogram(name: "declaration2", {{.*}}, flags: 
DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
 // HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: 
DIFlagPrototyped, spFlags: DISPFlagOptimized)
 // HAS-ATTR-DAG: DISubprogram(name: "struct1", {{.*}}, flags: DIFlagPrototyped 
| DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition
Index: clang/test/CodeGen/debug-info-extern-call.c
===
--- clang/test/CodeGen/debug-info-extern-call.c
+++ clang/test/CodeGen/debug-info-extern-call.c
@@ -1,8 +1,21 @@
-// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target 
x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s 
-check-prefix=CHECK-EXT
-// CHECK-EXT: !DISubprogram(name: "fn1"
+// When entry values are emitted, expect a subprogram for extern decls so that
+// the dwarf generator can describe call site parameters at extern call sites.
+//
+// RUN: %clang -Xclang -femit-debug-entry-values -g -O2 -target 
x86_64-none-linux-gnu -S -emit-llvm %s -o - | FileCheck %s 
-check-prefix=ENTRY-VAL
+// ENTRY-VAL: !DISubprogram(name: "fn1"
 
-// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -S -emit-llvm %s -o - | 
FileCheck %s
-// CHECK-NOT: !DISubprogram(name: "fn1"
+// Similarly, when the debugger tuning is gdb, expect a subprogram for extern
+// decls so that the dwarf generator can describe information needed for tail
+// call frame reconstrution.
+//
+// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -ggdb -S -emit-llvm %s -o 
- | FileCheck %s -check-prefix=GDB
+// GDB: !DISubprogram(name: "fn1"
+//
+// Do not emit a subprogram for extern decls when entry values are disabled and
+// the tuning is not set to gdb.
+//
+// RUN: %clang -g -O2 -target x86_64-none-linux-gnu -gsce -S -emit-llvm %s -o 
- | FileCheck %s -check-prefix=SCE
+// SCE-NOT: !DISubprogram(name: "fn1"
 
 extern int fn1(int a, int b);
 
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3731,9 +3731,7 @@
 void CGDebugInfo::EmitFuncDeclForCallSite(llvm::CallBase *CallOrInvoke,
   QualType CalleeType,
   const FunctionDecl *CalleeDecl) {
-  auto  = CGM.getCodeGenOpts();
-  if (!CGOpts.EnableDebugEntryValues || !CGM.getLangOpts().Optimize ||
-  !CallOrInvoke)
+  if (!CallOrInvoke || getCallSiteRelatedAttrs() == llvm::DINode::FlagZero)
 return;
 
   auto *Func = CallOrInvoke->getCalledFunction();
@@ -4791,10 +4789,10 @@
   bool SupportsDWARFv4Ext =
   CGM.getCodeGenOpts().DwarfVersion == 4 &&
   (CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::LLDB ||
-   (CGM.getCodeGenOpts().EnableDebugEntryValues &&
-   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB));
+   CGM.getCodeGenOpts().getDebuggerTuning() == llvm::DebuggerKind::GDB);
 
-  if (!SupportsDWARFv4Ext && CGM.getCodeGenOpts().DwarfVersion < 5)
+  if (!SupportsDWARFv4Ext && 

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

[Wenlei He via Phabricator via cfe-commits]

wenlei added inline comments.



Comment at: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp:614
   // during ThinLTO and perform the rest of the optimizations afterward.
   if (PrepareForThinLTO) {
 // Ensure we perform any last passes, but do so before renaming anonymous

this also need to be `PrepareForThinLTO || PrepareForLTO` for oldPM?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69732



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247
 DWARFVersion = ExplicitVersion;
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;

dblaikie wrote:
> Looks like this should be on a single line to conform to LLVM convention 
> (though might just be phabricator doing something weird)
> 
> If you can run clang-format over the change (not over the whole file) it 
> should fix up issues like this. (there's various clang-format editor 
> integrations - there's some google-internal documentation at go/clang-format 
> that'll explain how to setup an auto-save hook that'll clang-format the 
> changed lines so all your C++ code in the LLVM repository conforms to LLVM's 
> coding conventions (well, those that can be expressed by clang-format))
Oh, there's also clang/tools/clang-format/git-clang-format for formatting 
anything in a git revision range.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69822



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice marked 3 inline comments as done.
cmtice added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247
 DWARFVersion = ExplicitVersion;
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;

dblaikie wrote:
> dblaikie wrote:
> > Looks like this should be on a single line to conform to LLVM convention 
> > (though might just be phabricator doing something weird)
> > 
> > If you can run clang-format over the change (not over the whole file) it 
> > should fix up issues like this. (there's various clang-format editor 
> > integrations - there's some google-internal documentation at 
> > go/clang-format that'll explain how to setup an auto-save hook that'll 
> > clang-format the changed lines so all your C++ code in the LLVM repository 
> > conforms to LLVM's coding conventions (well, those that can be expressed by 
> > clang-format))
> Oh, there's also clang/tools/clang-format/git-clang-format for formatting 
> anything in a git revision range.
Ok, will do.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3247-3248
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;
 

dblaikie wrote:
> Hmm, actually - why is this case necessary/what does it cover? I was hoping 
> the case you added inside the "if (EmitDwarf)" case above would be all that 
> was required (& the call to ParseDebugDefaultVersion would go inside there at 
> the use, to reduce the variable scope/keep the code closer together).
This covers the case where you are setting the default dwarf version without 
actually turning on/off debug info at all (there's no -gdwarf or -g option, so 
EmitDwarf is false).



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6173-6175
+unsigned DefaultDwarfVersion = ParseDebugDefaultVersion(getToolChain(), 
Args);
+DwarfVersion = DefaultDwarfVersion ? DefaultDwarfVersion
+   :getToolChain().GetDefaultDwarfVersion();

dblaikie wrote:
> Some clang-formatting required, but also could probably be written as:
> 
>   if (DwarfVersion == 0)
> DwarfVersion = ParseDebugDefaultVersion(...);
> 
>   if (DwarfVersion == 0)
> DwarfVersion = getToolChain().GetDefaultDwarfVersion();
> 
> To make these more symmetric?
Ok will do.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69822



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

[clang] 403739b - [AST][NFC] Fixes a comment typo

[Mark de Wever via cfe-commits]

Author: Mark de Wever
Date: 2019-11-04T22:32:56+01:00
New Revision: 403739b2fdb64e90118017355bd01f88a0640b3f

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

LOG: [AST][NFC] Fixes a comment typo

Also a test for commit access.

Added: 


Modified: 
clang/lib/AST/DeclCXX.cpp

Removed: 




diff  --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 12ec44fa0279..58b50de944ed 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -413,7 +413,7 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const 
*Bases,
   data().HasIrrelevantDestructor = false;
 
 // C++11 [class.copy]p18:
-//   The implicitly-declared copy assignment oeprator for a class X will
+//   The implicitly-declared copy assignment operator for a class X will
 //   have the form 'X& X::operator=(const X&)' if each direct base class B
 //   of X has a copy assignment operator whose parameter is of type 'const
 //   B&', 'const volatile B&', or 'B' [...]



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

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Updated release notes in d11a9018b773c0359934a7989d886b02468112e4 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67983



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

[PATCH] D69756: [opaque pointer types] Add element type argument to IRBuilder CreatePreserveStructAccessIndex and CreatePreserveArrayAccessIndex

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Updated release notes in d11a9018b773c0359934a7989d886b02468112e4 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69756



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

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3246-3247
 DWARFVersion = ExplicitVersion;
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;

Looks like this should be on a single line to conform to LLVM convention 
(though might just be phabricator doing something weird)

If you can run clang-format over the change (not over the whole file) it should 
fix up issues like this. (there's various clang-format editor integrations - 
there's some google-internal documentation at go/clang-format that'll explain 
how to setup an auto-save hook that'll clang-format the changed lines so all 
your C++ code in the LLVM repository conforms to LLVM's coding conventions 
(well, those that can be expressed by clang-format))



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3247-3248
   }
+  else if (DefaultDWARFVersion != 0)
+DWARFVersion = DefaultDWARFVersion;
 

Hmm, actually - why is this case necessary/what does it cover? I was hoping the 
case you added inside the "if (EmitDwarf)" case above would be all that was 
required (& the call to ParseDebugDefaultVersion would go inside there at the 
use, to reduce the variable scope/keep the code closer together).



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6173-6175
+unsigned DefaultDwarfVersion = ParseDebugDefaultVersion(getToolChain(), 
Args);
+DwarfVersion = DefaultDwarfVersion ? DefaultDwarfVersion
+   :getToolChain().GetDefaultDwarfVersion();

Some clang-formatting required, but also could probably be written as:

  if (DwarfVersion == 0)
DwarfVersion = ParseDebugDefaultVersion(...);

  if (DwarfVersion == 0)
DwarfVersion = getToolChain().GetDefaultDwarfVersion();

To make these more symmetric?


Repository:
  rC Clang

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

https://reviews.llvm.org/D69822



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

[clang] 3eecd60 - [OPENMP][DOCS]Update list of implemented features, NFC.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2019-11-04T16:29:26-05:00
New Revision: 3eecd601ed803ed3dc15b054cdddee68702ba3dd

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

LOG: [OPENMP][DOCS]Update list of implemented features, NFC.

Added: 


Modified: 
clang/docs/OpenMPSupport.rst

Removed: 




diff  --git a/clang/docs/OpenMPSupport.rst b/clang/docs/OpenMPSupport.rst
index 5e88276794a6..f3922fa20837 100644
--- a/clang/docs/OpenMPSupport.rst
+++ b/clang/docs/OpenMPSupport.rst
@@ -126,11 +126,11 @@ implementation.
 
+--+--+--+---+
 | loop extension   | #pragma omp loop (directive)  
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+
-| loop extension   | collapse imperfectly nested loop  
   | :none:`unclaimed`| 
  |
+| loop extension   | collapse imperfectly nested loop  
   | :none:`done` | 
  |
 
+--+--+--+---+
 | loop extension   | collapse non-rectangular nested loop  
   | :good:`done` | 
  |
 
+--+--+--+---+
-| loop extension   | C++ range-base for loop   
   | :none:`unclaimed`| 
  |
+| loop extension   | C++ range-base for loop   
   | :none:`done` | 
  |
 
+--+--+--+---+
 | loop extension   | clause: nosimd for SIMD directives
   | :none:`unclaimed`| 
  |
 
+--+--+--+---+



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

[clang] d11a901 - Add release notes for commit ccc4d83cda16bea1d9dfd0967dc7d2cfb24b8e75.

[James Y Knight via cfe-commits]

Author: James Y Knight
Date: 2019-11-04T16:26:53-05:00
New Revision: d11a9018b773c0359934a7989d886b02468112e4

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

LOG: Add release notes for commit ccc4d83cda16bea1d9dfd0967dc7d2cfb24b8e75.

(Which was "[ObjC] Diagnose implicit type coercion from ObjC 'Class'
to object pointer types.")

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5363050ca69d..51bcd013df6d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -166,7 +166,49 @@ C++1z Feature Support
 Objective-C Language Changes in Clang
 -
 
-- ...
+- In both Objective-C and
+  Objective-C++, ``-Wcompare-distinct-pointer-types`` will now warn when
+  comparing ObjC ``Class`` with an ObjC instance type pointer.
+
+  .. code-block:: objc
+
+Class clz = ...;
+MyType *instance = ...;
+bool eq = (clz == instance); // Previously undiagnosed, now warns.
+
+- Objective-C++ now diagnoses conversions between ``Class`` and ObjC
+  instance type pointers. Such conversions already emitted an
+  on-by-default ``-Wincompatible-pointer-types`` warning in Objective-C
+  mode, but had inadvertently been missed entirely in
+  Objective-C++. This has been fixed, and they are now diagnosed as
+  errors, consistent with the usual C++ treatment for conversions
+  between unrelated pointer types.
+
+  .. code-block:: objc
+
+Class clz = ...;
+MyType *instance = ...;
+clz = instance; // Previously undiagnosed, now an error.
+instance = clz; // Previously undiagnosed, now an error.
+
+  One particular issue you may run into is attempting to use a class
+  as a key in a dictionary literal. This will now result in an error,
+  because ``Class`` is not convertable to ``id``. (Note that
+  this was already a warning in Objective-C mode.) While an arbitrary
+  ``Class`` object is not guaranteed to implement ``NSCopying``, the
+  default metaclass implementation does. Therefore, the recommended
+  solution is to insert an explicit cast to ``id``, which disables the
+  type-checking here.
+
+ .. code-block:: objc
+
+Class cls = ...;
+
+// Error: cannot convert from Class to id.
+NSDictionary* d = @{cls : @"Hello"};
+
+// Fix: add an explicit cast to 'id'.
+NSDictionary* d = @{(id)cls : @"Hello"};
 
 OpenCL C Language Changes in Clang
 --



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

[clang] 8bbf2e3 - [OPENMP50]Support for imperfectly nested loops.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2019-11-04T16:09:25-05:00
New Revision: 8bbf2e37167d9ac08fa9d3c772d48ca7d9a6f8f6

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

LOG: [OPENMP50]Support for imperfectly nested loops.

Added support for imperfectly nested loops introduced in OpenMP 5.0.

Added: 


Modified: 
clang/include/clang/AST/StmtOpenMP.h
clang/lib/AST/StmtOpenMP.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/for_ast_print.cpp
clang/test/OpenMP/for_codegen.cpp
clang/test/OpenMP/for_collapse_messages.cpp

Removed: 




diff  --git a/clang/include/clang/AST/StmtOpenMP.h 
b/clang/include/clang/AST/StmtOpenMP.h
index 722aa509b132..9807b3c8f680 100644
--- a/clang/include/clang/AST/StmtOpenMP.h
+++ b/clang/include/clang/AST/StmtOpenMP.h
@@ -1084,28 +1084,20 @@ class OMPLoopDirective : public OMPExecutableDirective {
 return const_cast(reinterpret_cast(
 *std::next(child_begin(), CombinedParForInDistConditionOffset)));
   }
+  /// Try to find the next loop sub-statement in the specified statement \p
+  /// CurStmt.
+  /// \param TryImperfectlyNestedLoops true, if we need to try to look for the
+  /// imperfectly nested loop.
+  static Stmt *tryToFindNextInnerLoop(Stmt *CurStmt,
+  bool TryImperfectlyNestedLoops);
+  static const Stmt *tryToFindNextInnerLoop(const Stmt *CurStmt,
+bool TryImperfectlyNestedLoops) {
+return tryToFindNextInnerLoop(const_cast(CurStmt),
+  TryImperfectlyNestedLoops);
+  }
+  Stmt *getBody();
   const Stmt *getBody() const {
-// This relies on the loop form is already checked by Sema.
-const Stmt *Body =
-getInnermostCapturedStmt()->getCapturedStmt()->IgnoreContainers();
-if (auto *For = dyn_cast(Body)) {
-  Body = For->getBody();
-} else {
-  assert(isa(Body) &&
- "Expected canonical for loop or range-based for loop.");
-  Body = cast(Body)->getBody();
-}
-for (unsigned Cnt = 1; Cnt < CollapsedNum; ++Cnt) {
-  Body = Body->IgnoreContainers();
-  if (auto *For = dyn_cast(Body)) {
-Body = For->getBody();
-  } else {
-assert(isa(Body) &&
-   "Expected canonical for loop or range-based for loop.");
-Body = cast(Body)->getBody();
-  }
-}
-return Body;
+return const_cast(this)->getBody();
   }
 
   ArrayRef counters() { return getCounters(); }

diff  --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp
index a93192b4857f..12201ef9ec28 100644
--- a/clang/lib/AST/StmtOpenMP.cpp
+++ b/clang/lib/AST/StmtOpenMP.cpp
@@ -41,6 +41,74 @@ const Stmt *OMPExecutableDirective::getStructuredBlock() 
const {
   return getInnermostCapturedStmt()->getCapturedStmt();
 }
 
+Stmt *OMPLoopDirective::tryToFindNextInnerLoop(Stmt *CurStmt,
+   bool TryImperfectlyNestedLoops) 
{
+  Stmt *OrigStmt = CurStmt;
+  CurStmt = CurStmt->IgnoreContainers();
+  // Additional work for imperfectly nested loops, introduced in OpenMP 5.0.
+  if (TryImperfectlyNestedLoops) {
+if (auto *CS = dyn_cast(CurStmt)) {
+  CurStmt = nullptr;
+  SmallVector Statements(1, CS);
+  SmallVector NextStatements;
+  while (!Statements.empty()) {
+CS = Statements.pop_back_val();
+if (!CS)
+  continue;
+for (Stmt *S : CS->body()) {
+  if (!S)
+continue;
+  if (isa(S) || isa(S)) {
+// Only single loop construct is allowed.
+if (CurStmt) {
+  CurStmt = OrigStmt;
+  break;
+}
+CurStmt = S;
+continue;
+  }
+  S = S->IgnoreContainers();
+  if (auto *InnerCS = dyn_cast_or_null(S))
+NextStatements.push_back(InnerCS);
+}
+if (Statements.empty()) {
+  // Found single inner loop or multiple loops - exit.
+  if (CurStmt)
+break;
+  Statements.swap(NextStatements);
+}
+  }
+  if (!CurStmt)
+CurStmt = OrigStmt;
+}
+  }
+  return CurStmt;
+}
+
+Stmt *OMPLoopDirective::getBody() {
+  // This relies on the loop form is already checked by Sema.
+  Stmt *Body =
+  getInnermostCapturedStmt()->getCapturedStmt()->IgnoreContainers();
+  if (auto *For = dyn_cast(Body)) {
+Body = For->getBody();
+  } else {
+assert(isa(Body) &&
+   "Expected canonical for loop or range-based for loop.");
+Body = cast(Body)->getBody();
+  }
+  for (unsigned Cnt = 1; Cnt < CollapsedNum; ++Cnt) {
+Body = tryToFindNextInnerLoop(Body, /*TryImperfectlyNestedLoops=*/true);
+if (auto *For = 

[PATCH] D69822: [clang] Add new -fdebug-default-version flag.

[Caroline Tice via Phabricator via cfe-commits]

cmtice created this revision.
cmtice added a reviewer: dblaikie.
Herald added subscribers: cfe-commits, fedor.sergeev, aprantl.
Herald added a project: clang.

Want to decouple setting the DWARF version from enabling/disabling generation 
of debug info.  This adds a new flag  '-fdebug-default-version=N' that sets the 
version of DWARF to be generated, but does not turn on/off debug info 
generation.


Repository:
  rC Clang

https://reviews.llvm.org/D69822

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/CodeGen/debug-default-version.c

Index: clang/test/CodeGen/debug-default-version.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-default-version.c
@@ -0,0 +1,47 @@
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -gdwarf-2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-3 -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER3
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-4 -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-linux-gnu -gdwarf-5 -S -fdebug-default-version=2 -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=5 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-linux-gnu -gdwarf -fdebug-default-version=2 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-linux-gnu -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefixes=NODEBUGINFO,VER4
+
+// The -isysroot is used as a hack to avoid LIT messing with the SDKROOT
+// environment variable which indirecty overrides the version in the target
+// triple used here.
+// RUN: %clang -target x86_64-apple-macosx10.11 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target x86_64-apple-darwin14 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER2
+// RUN: %clang -target x86_64-apple-macosx10.11 -fdebug-default-version=5 -g -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER5
+// RUN: %clang -target x86_64-apple-darwin14 -g -fdebug-default-version=4 -S -emit-llvm -o - %s -isysroot %t | FileCheck %s --check-prefix=VER4
+
+// RUN: %clang -target powerpc-unknown-openbsd -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target powerpc-unknown-freebsd -g -fdebug-default-version=4 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+// RUN: %clang -target i386-pc-solaris -fdebug-default-version=4 -g -S -emit-llvm -o - %s | FileCheck %s --check-prefix=VER4
+
+// Check which debug info formats we use on Windows. By default, in an MSVC
+// environment, we should use codeview. You can enable dwarf, which implicitly
+// disables codeview, of you can explicitly ask for both if you don't know how
+// the app will be debugged.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=2 -gdwarf -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER2,NOCODEVIEW
+// Explicitly request both.
+// RUN: %clang -target i686-pc-windows-msvc -fdebug-default-version=4 -gdwarf -gcodeview -S -emit-llvm -o - %s \
+// RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
+
+int main (void) {
+  return 0;
+}
+
+// NODEBUGINFO-NOT: !llvm.dbg.cu = !{!0}
+
+// NOCODEVIEW-NOT: !"CodeView"
+
+// VER2: !{i32 2, !"Dwarf Version", i32 2}
+// VER3: !{i32 2, !"Dwarf Version", i32 3}
+// VER4: !{i32 2, !"Dwarf Version", i32 4}
+// VER5: !{i32 2, !"Dwarf Version", i32 5}
+
+// NODWARF-NOT: !"Dwarf Version"
+// CODEVIEW: !{i32 2, !"CodeView", i32 1}
+// NOCODEVIEW-NOT: !"CodeView"
+// NODWARF-NOT: !"Dwarf Version"
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -68,6 +68,9 @@
 unsigned ParseFunctionAlignment(const ToolChain ,
 const llvm::opt::ArgList );
 
+unsigned ParseDebugDefaultVersion(const ToolChain ,
+  const llvm::opt::ArgList );
+
 void AddAssemblerKPIC(const ToolChain ,
   const llvm::opt::ArgList ,
   llvm::opt::ArgStringList );
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1138,6 +1138,22 @@
   return Value ? llvm::Log2_32_Ceil(std::min(Value, 65536u)) : Value;
 }
 
+unsigned tools::ParseDebugDefaultVersion(const ToolChain ,
+   

[PATCH] D51200: Introduce per-callsite inline intrinsics

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: include/clang/CodeGen/CGFunctionInfo.h:517
 
+  unsigned InlineCall : 2;
+

rjmccall wrote:
> Please don't propagate this information through the `CGFunctionInfo`.  I 
> really want this type to be *less* site-specific, not *more*.
I like this patch but sadly, it was abandoned.

What way of passing this info would you recommend?


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

https://reviews.llvm.org/D51200



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

[PATCH] D69818: [HIP] Fix pointer type kernel arg for amdgpu

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 227764.
yaxunl added a comment.

add a test for non-kernel function.


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

https://reviews.llvm.org/D69818

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu


Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -x hip %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+// CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
+__global__ void kernel1(int *x) {
+  x[0]++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* 
dereferenceable(4) %x.coerce)
+__global__ void kernel2(int ) {
+  x++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 
addrspace(2)* %x, i32 addrspace(1)* %y)
+__global__ void kernel3(__attribute__((address_space(2))) int *x,
+__attribute__((address_space(1))) int *y) {
+  y[0] = x[0];
+}
+
+// CHECK: define void @_Z4funcPi(i32* %x)
+__device__ void func(int *x) {
+  x[0]++;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7816,6 +7816,27 @@
   if (const Type *SeltTy = isSingleElementStruct(Ty, getContext()))
 return ABIArgInfo::getDirect(CGT.ConvertType(QualType(SeltTy, 0)));
 
+  // Coerce pointer type kernel arguments in default address space to
+  // device address space for HIP.
+  QualType PointeeTy;
+  if (getContext().getLangOpts().HIP) {
+if (auto *PT = Ty->getAs()) {
+  if (PT->getPointeeType().getAddressSpace() == LangAS::Default) {
+PointeeTy = PT->getPointeeType();
+  }
+} else if (auto *RT = Ty->getAs()) {
+  if (RT->getPointeeType().getAddressSpace() == LangAS::Default) {
+PointeeTy = RT->getPointeeType();
+  }
+}
+
+if (PointeeTy != QualType()) {
+  return ABIArgInfo::getDirect(
+CGT.ConvertType(PointeeTy)
+->getPointerTo(
+getContext().getTargetAddressSpace(LangAS::cuda_device)));
+}
+  }
   // If we set CanBeFlattened to true, CodeGen will expand the struct to its
   // individual elements, which confuses the Clover OpenCL backend; therefore 
we
   // have to set it to false here. Other args of getDirect() are just defaults.
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1169,7 +1169,7 @@
   if (isa(Val->getType())) {
 // If this is Pointer->Pointer avoid conversion to and from int.
 if (isa(Ty))
-  return CGF.Builder.CreateBitCast(Val, Ty, "coerce.val");
+  return CGF.Builder.CreatePointerCast(Val, Ty, "coerce.val");
 
 // Convert the pointer to an integer so we can play with its width.
 Val = CGF.Builder.CreatePtrToInt(Val, CGF.IntPtrTy, "coerce.val.pi");


Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -x hip %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+// CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
+__global__ void kernel1(int *x) {
+  x[0]++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* dereferenceable(4) %x.coerce)
+__global__ void kernel2(int ) {
+  x++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
+__global__ void kernel3(__attribute__((address_space(2))) int *x,
+__attribute__((address_space(1))) int *y) {
+  y[0] = x[0];
+}
+
+// CHECK: define void @_Z4funcPi(i32* %x)
+__device__ void func(int *x) {
+  x[0]++;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7816,6 +7816,27 @@
   if (const Type *SeltTy = isSingleElementStruct(Ty, getContext()))
 return ABIArgInfo::getDirect(CGT.ConvertType(QualType(SeltTy, 0)));
 
+  // Coerce pointer type kernel arguments in default address space to
+  // device address space for HIP.
+  QualType PointeeTy;
+  if (getContext().getLangOpts().HIP) {
+if (auto *PT = Ty->getAs()) {
+  if (PT->getPointeeType().getAddressSpace() == LangAS::Default) {
+PointeeTy = PT->getPointeeType();
+  }
+} else if (auto *RT = Ty->getAs()) {
+  if 

[PATCH] D69743: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

What's the extra DWARF that's emitted from the backend? Is there a 
corresponding LLVM code change/test, or is the functionality there but dormant 
in this particular case? (& already running/tested in other cases, such as when 
fn1 is defined in the same CU)


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

https://reviews.llvm.org/D69743



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

[PATCH] D69818: [HIP] Fix pointer type kernel arg for amdgpu

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall.
Herald added subscribers: t-tye, tpr, dstuttard, nhaehnle, wdng, jvesely, 
kzhuravl.

amdgpu target prefers pointer type kernel arg in default address space
to be coerced to device address space for better performance.

  

This patch fixes that.


https://reviews.llvm.org/D69818

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu


Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -x hip %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+// CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
+__global__ void kernel1(int *x) {
+  x[0]++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* 
dereferenceable(4) %x.coerce)
+__global__ void kernel2(int ) {
+  x++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 
addrspace(2)* %x, i32 addrspace(1)* %y)
+__global__ void kernel3(__attribute__((address_space(2))) int *x,
+__attribute__((address_space(1))) int *y) {
+  y[0] = x[0];
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7816,6 +7816,27 @@
   if (const Type *SeltTy = isSingleElementStruct(Ty, getContext()))
 return ABIArgInfo::getDirect(CGT.ConvertType(QualType(SeltTy, 0)));
 
+  // Coerce pointer type kernel arguments in default address space to
+  // device address space for HIP.
+  QualType PointeeTy;
+  if (getContext().getLangOpts().HIP) {
+if (auto *PT = Ty->getAs()) {
+  if (PT->getPointeeType().getAddressSpace() == LangAS::Default) {
+PointeeTy = PT->getPointeeType();
+  }
+} else if (auto *RT = Ty->getAs()) {
+  if (RT->getPointeeType().getAddressSpace() == LangAS::Default) {
+PointeeTy = RT->getPointeeType();
+  }
+}
+
+if (PointeeTy != QualType()) {
+  return ABIArgInfo::getDirect(
+CGT.ConvertType(PointeeTy)
+->getPointerTo(
+getContext().getTargetAddressSpace(LangAS::cuda_device)));
+}
+  }
   // If we set CanBeFlattened to true, CodeGen will expand the struct to its
   // individual elements, which confuses the Clover OpenCL backend; therefore 
we
   // have to set it to false here. Other args of getDirect() are just defaults.
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1169,7 +1169,7 @@
   if (isa(Val->getType())) {
 // If this is Pointer->Pointer avoid conversion to and from int.
 if (isa(Ty))
-  return CGF.Builder.CreateBitCast(Val, Ty, "coerce.val");
+  return CGF.Builder.CreatePointerCast(Val, Ty, "coerce.val");
 
 // Convert the pointer to an integer so we can play with its width.
 Val = CGF.Builder.CreatePtrToInt(Val, CGF.IntPtrTy, "coerce.val.pi");


Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -x hip %s -o - | FileCheck %s
+#include "Inputs/cuda.h"
+// CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
+__global__ void kernel1(int *x) {
+  x[0]++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* dereferenceable(4) %x.coerce)
+__global__ void kernel2(int ) {
+  x++;
+}
+
+// CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
+__global__ void kernel3(__attribute__((address_space(2))) int *x,
+__attribute__((address_space(1))) int *y) {
+  y[0] = x[0];
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7816,6 +7816,27 @@
   if (const Type *SeltTy = isSingleElementStruct(Ty, getContext()))
 return ABIArgInfo::getDirect(CGT.ConvertType(QualType(SeltTy, 0)));
 
+  // Coerce pointer type kernel arguments in default address space to
+  // device address space for HIP.
+  QualType PointeeTy;
+  if (getContext().getLangOpts().HIP) {
+if (auto *PT = Ty->getAs()) {
+  if (PT->getPointeeType().getAddressSpace() == LangAS::Default) {
+PointeeTy = PT->getPointeeType();
+  }
+} else if (auto *RT = Ty->getAs()) {
+  if (RT->getPointeeType().getAddressSpace() 

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124
+  if (const SymbolicRegion *SR = DestMR->getSymbolicBase())
+if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR))
+  return exprToStr(SizeExpr, C);

NoQ wrote:
> Again, you will have to highlight the allocation site with a note. Therefore 
> you will have to write a bug visitor that traverses the size expression at 
> some point (or, equivalently, a note tag when the size expression is 
> evaluated). Therefore you don't need to store the expression in the program 
> state.
Yes, you have pointed out the necessary visitor, but it needs more thinking.

I have a memory region which could be any kind of "memory block region" 
therefore I have no idea where is the size expression. We are supporting ~20 
different allocations, which is nothing compared to the wild with the not so 
uncommon 5+ parameter allocators. Therefore I still do not want to reverse 
engineer a small MallocChecker + ExprEngine + BuiltinFunctionChecker inside my 
checker. They provide the necessary `DynamicSizeInfo` easily, which could be 
used in at least 4 checkers at the moment (which I have pointed out earlier in 
D68725).

If I have the size expression in the dynamic size map, and I can clearly point 
out the destination buffer, it is a lot more simplified to traverse the graph 
where the buffer and its size comes from.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:184
+  if (IsFix) {
+if (Optional SizeStr = getSizeExprAsString(Call, CallC, C)) {
+  renameFunctionFix(UseSafeFunctions ? "gets_s" : "fgets", Call, *Report);

NoQ wrote:
> Also, which is probably more important, you will never be able to provide a 
> fixit for the malloced memory case, because there may be multiple execution 
> paths that reach the current point with different size expressions (in fact, 
> not necessarily all of them are malloced).
> 
> Eg.:
> ```lang=c
> char *x = 0;
> char y[10];
> 
> if (coin()) {
>   x = malloc(20);
> } else {
>   x = y;
> }
> 
> gets(x);
> ```
> 
> If you suggest replacing `gets(x)` with `gets_s(x, 20)`, you'll still have a 
> buffer overflow on the else-branch on which `x` points to an array of 10 
> bytes.
This checker going to evolve a lot, and all of the checked function calls have 
issues like that. I do not even think what else issues they have. I would like 
to cover the false alarm suppression when we are about to alarm. Is it would be 
okay? I really would like to see alarms first.

For example, I have seen stuff in the wild so that I can state out 8-param 
allocators and we need to rely on the checkers provide information about 
allocation.



Comment at: clang/test/Analysis/cert/str31-alloc.cpp:42
+  // expected-warning@-1 {{'gets' could write outside of 'buf3'}}
+  // CHECK-FIXES: if (gets_s(buf3 + 1, sizeof(buf3))) {}
+}

NoQ wrote:
> The fix is not correct. It should be `sizeof(buf3) - 1`, otherwise you still 
> have a buffer overflow.
Good catch, thanks! I was really into the pretty-printing, we should not fix-it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69813



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Hi @jdoerfert, thanks a lot for putting this up this initial skeleton and 
providing an example with `barrier`.

I made a small experiment lowering a `taskwait` (which is even simpler than 
`barrier` in "lowering" complexity). It was pretty straightforward after all.

I have a few questions just to reassure I fully understand your vision here:

- the parameter `Loc` is currently unused but my understanding from the comment 
in `OpenMPIRBuilder::getSrcLocStr` eventually we will convert a 
`clang::SourceLocation` to a `llvm::Constant*` that represents the location as 
used by KMP, right?
- emitting a `taskwait` however has this

  if (auto *Region = dyn_cast_or_null(CGF.CapturedStmtInfo))
Region->emitUntiedSwitch(CGF);

but my understanding is that we can apply the same scheme to these hooks as 
well, recursively, i.e.: progressively convert them to calls to 
`OpenMPIRBuilder` (if needed under the flag suggested above by Alexey). Does 
this make sense?

Kind regards,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D68590: [clangd] Improve hover support for Objective-C

[David Goldman via Phabricator via cfe-commits]

dgoldman marked 3 inline comments as done.
dgoldman added a comment.

Will revisit this once more critical fixes are in (crash fixes), I'm still not 
sure where this sort of stuff should belong




Comment at: clangd/XRefs.cpp:461
+
+  OS << (Method->isInstanceMethod() ? '-' : '+')
+ << '[' << Class->getName();

kadircet wrote:
> it looks like `DeclPrinter::VisitObjCMethodDecl` already has a similar 
> handling.
> would it provide value if you printed class and category names in the general 
> case? If so it might make sense to update the logic in `DeclPrinter`.
> If not, it looks like printing `selector` via `getAsString` is not enough, 
> there's some special handling in `DeclPrinter`. Maybe move it into a public
> place and make use of the same logic also in here to be consistent?
`DeclPrinter::VisitObjCMethodDecl` prints the declaration itself, e.g.:

`- (void)someMethod:(int)param;`

it wouldn't make sense to instead of that do

`-[MyClass someMethod:]` for the declaration itself



Comment at: clangd/XRefs.cpp:470
+
+static std::string getNameForObjCContainer(const ObjCContainerDecl *C) {
+  if (const ObjCCategoryDecl *Category = dyn_cast(C)) {

kadircet wrote:
> again there are printers in for `ObjCCategory{Impl}Decl`s that don't respect 
> `TerseOutput` option in printing policies. It might be sensible to update 
> them instead to print an output like what you propose in here, in the 
> presence of `TerseOutput` option. WDYT?
What method names are you looking at? I wasn't able to find them


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68590



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

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:184
+  if (IsFix) {
+if (Optional SizeStr = getSizeExprAsString(Call, CallC, C)) {
+  renameFunctionFix(UseSafeFunctions ? "gets_s" : "fgets", Call, *Report);

Also, which is probably more important, you will never be able to provide a 
fixit for the malloced memory case, because there may be multiple execution 
paths that reach the current point with different size expressions (in fact, 
not necessarily all of them are malloced).

Eg.:
```lang=c
char *x = 0;
char y[10];

if (coin()) {
  x = malloc(20);
} else {
  x = y;
}

gets(x);
```

If you suggest replacing `gets(x)` with `gets_s(x, 20)`, you'll still have a 
buffer overflow on the else-branch on which `x` points to an array of 10 bytes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69813



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

[PATCH] D69726: [analyzer] DynamicSize: Store the dynamic size

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h:29-31
+/// \returns The stored dynamic size expression for the region \p MR.
+const Expr *getDynamicSizeExpr(ProgramStateRef State, const MemRegion *MR);
+

Charusso wrote:
> NoQ wrote:
> > Why do we need this?
> I think as the checkers are growing and we push more-and-more allocation 
> modeling so that at some point the Git's 8-parameter allocator's size 
> expression could be retrieved so easily. This is the full arsenal of my 
> buffer-overflow checker's needs, so I have pushed it here. Also it made a 
> meaning to have a helper-class with two fields (one would be lame).
>>! In D68725#1722136, @NoQ wrote:
> any path-sensitive checker for which such region is "interesting" would have 
> to implement a bug visitor to display the allocation site. Such visitor 
> automatically knows the location of the `alloca()` expression and can emit 
> the necessary fixits.


>>! In D69813#inline-628182, @NoQ wrote:
> Again, you will have to highlight the allocation site with a note. Therefore 
> you will have to write a bug visitor that traverses the size expression at 
> some point (or, equivalently, a note tag when the size expression is 
> evaluated). Therefore you don't need to store the expression in the program 
> state.


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

https://reviews.llvm.org/D69726



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

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D61634#1682660 , @gchatelet wrote:

> In D61634#1679331 , @tejohnson wrote:
>
> > In D61634#1635595 , @tejohnson 
> > wrote:
> >
> > > I had some time to work on this finally late last week. I decided the 
> > > most straightforward thing was to implement the necessary interface 
> > > changes to the TLI analysis to make it require a Function (without any 
> > > changes yet to how that analysis operates). See D66428 
> > >  that I just mailed for review. That 
> > > takes care of the most widespread changes needed for this migration, and 
> > > afterwards we can change the analysis to look at the function attributes 
> > > and make a truly per-function TLI.
> >
> >
> > D66428  went in a few weeks ago at 
> > r371284, and I just mailed the follow on patch D67923 
> >  which will adds the support into the TLI 
> > analysis to use the Function to override the available builtins (with some 
> > of the code stubbed out since we don't yet have those per-Function 
> > attributes finalized).
> >
> > @gchatelet where are you at on finalizing this patch? Also, I mentioned 
> > this offline but to follow up here: I think we will want an attribute to 
> > represent -fno-builtins (so that it doesn't need to be expanded out into 
> > the full list of individual no-builtin-{func} attributes, which would be 
> > both more verbose and less efficient, as well as being less backward 
> > compatible when new builtin funcs are added).
>
>
> I'll break this patch in several pieces. The first one is to add the 
> `no_builtin` attribute, see https://reviews.llvm.org/D61634.


Are you planning to add a follow on patch that translates the various 
-fno-builtin* options to these attributes? Once that is done I can refine 
D67923  to actually set the TLI availability 
from the attributes and remove the clang functionality that does this from the 
options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634



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

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:124
+  if (const SymbolicRegion *SR = DestMR->getSymbolicBase())
+if (const Expr *SizeExpr = getDynamicSizeExpr(C.getState(), SR))
+  return exprToStr(SizeExpr, C);

Again, you will have to highlight the allocation site with a note. Therefore 
you will have to write a bug visitor that traverses the size expression at some 
point (or, equivalently, a note tag when the size expression is evaluated). 
Therefore you don't need to store the expression in the program state.



Comment at: clang/test/Analysis/cert/str31-alloc.cpp:42
+  // expected-warning@-1 {{'gets' could write outside of 'buf3'}}
+  // CHECK-FIXES: if (gets_s(buf3 + 1, sizeof(buf3))) {}
+}

The fix is not correct. It should be `sizeof(buf3) - 1`, otherwise you still 
have a buffer overflow.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69813



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

[clang] 8112a42 - clang/Modules: Bring back optimization lost in 31e14f41a21f

[Duncan P. N. Exon Smith via cfe-commits]

Author: Duncan P. N. Exon Smith
Date: 2019-11-04T11:40:03-08:00
New Revision: 8112a423a8ede9bce64b6553e6451bf10995105c

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

LOG: clang/Modules: Bring back optimization lost in 31e14f41a21f

31e14f41a21f9016050a20f07d5da03db2e8c13e accidentally dropped caching of
failed module loads.  This brings it back by making
ModuleMap::getCachedModuleLoad return an Optional.

Added: 


Modified: 
clang/include/clang/Lex/ModuleMap.h
clang/lib/Frontend/CompilerInstance.cpp

Removed: 




diff  --git a/clang/include/clang/Lex/ModuleMap.h 
b/clang/include/clang/Lex/ModuleMap.h
index 3110ead86010..1e6b28d4aa3d 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -696,8 +696,11 @@ class ModuleMap {
   }
 
   /// Return a cached module load.
-  Module *getCachedModuleLoad(const IdentifierInfo ) {
-return CachedModuleLoads.lookup();
+  llvm::Optional getCachedModuleLoad(const IdentifierInfo ) {
+auto I = CachedModuleLoads.find();
+if (I == CachedModuleLoads.end())
+  return None;
+return I->second;
   }
 };
 

diff  --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index cc3d848c1e02..a0663217453a 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -1633,10 +1633,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   }
 
   // If we don't already have information on this module, load the module now.
+  Module *Module = nullptr;
   ModuleMap  = getPreprocessor().getHeaderSearchInfo().getModuleMap();
-  clang::Module *Module = MM.getCachedModuleLoad(*Path[0].first);
-  if (Module) {
-// Nothing to do here, we found it.
+  if (auto MaybeModule = MM.getCachedModuleLoad(*Path[0].first)) {
+// Use the cached result, which may be nullptr.
+Module = *MaybeModule;
   } else if (ModuleName == getLangOpts().CurrentModule) {
 // This is the module we're building.
 Module = PP->getHeaderSearchInfo().lookupModule(



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

[PATCH] D67763: [Clang FE] Recognize -mnop-mcount CL option

[Ulrich Weigand via Phabricator via cfe-commits]

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

This looks good to me now.  Given that this patch implements an option only 
relevant on SystemZ, this should be OK.


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

https://reviews.llvm.org/D67763



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

[PATCH] D69813: [analyzer][WIP] CERTStrChecker: Model gets()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny.
Charusso added a parent revision: D69746: [analyzer] FixItHint: Apply and test 
hints with the Clang Tidy's script.

This checker warn on `gets()` based on the following rules:
https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator

It also tries to rewrite the bad code with the help of `FixItHints`.


Repository:
  rC Clang

https://reviews.llvm.org/D69813

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/cert/str31-alloc.cpp
  clang/test/Analysis/cert/str31-safe.cpp
  clang/test/Analysis/cert/str31-unsafe.cpp

Index: clang/test/Analysis/cert/str31-unsafe.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str31-unsafe.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_analyzer_fixit %s %t \
+// RUN:  -analyzer-checker=core,unix,security.cert.Str \
+// RUN:  -analyzer-config security.cert.Str:WantToUseSafeFunctions=true \
+// RUN:  -I %S
+
+// See the examples on the page of STR31:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator
+
+#include "../Inputs/system-header-simulator.h"
+
+// The following is not defined therefore the safe functions are unavailable.
+// #define __STDC_LIB_EXT1__ 1
+#define __STDC_WANT_LIB_EXT1__ 1
+
+namespace test_gets_bad {
+#define BUFFER_SIZE 1024
+
+void func(void) {
+  char buf[BUFFER_SIZE];
+  if (gets(buf)) {}
+  // expected-warning@-1 {{'gets' could write outside of 'buf'}}
+  // CHECK-FIXES: if (fgets(buf, sizeof(buf), stdin)) {}
+}
+} // namespace test_gets_bad
+
+namespace test_gets_good {
+enum { BUFFERSIZE = 32 };
+
+void func(void) {
+  char buff[BUFFERSIZE];
+
+  if (fgets(buff, sizeof(buff), stdin)) {}
+}
+} // namespace test_gets_good
Index: clang/test/Analysis/cert/str31-safe.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str31-safe.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_analyzer_fixit %s %t \
+// RUN:  -analyzer-checker=core,unix,security.cert.Str \
+// RUN:  -analyzer-config security.cert.Str:WantToUseSafeFunctions=true \
+// RUN:  -I %S
+
+// See the examples on the page of STR31:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator
+
+#include "../Inputs/system-header-simulator.h"
+
+#define __STDC_LIB_EXT1__ 1
+#define __STDC_WANT_LIB_EXT1__ 1
+
+namespace test_gets_bad {
+#define BUFFER_SIZE 1024
+
+void func(void) {
+  char buf[BUFFER_SIZE];
+  if (gets(buf)) {}
+  // expected-warning@-1 {{'gets' could write outside of 'buf'}}
+  // CHECK-FIXES: if (gets_s(buf, sizeof(buf))) {}
+}
+} // namespace test_gets_bad
+
+namespace test_gets_good {
+enum { BUFFERSIZE = 32 };
+
+void func(void) {
+  char buff[BUFFERSIZE];
+
+  if (gets_s(buff, sizeof(buff))) {}
+}
+} // namespace test_gets_good
Index: clang/test/Analysis/cert/str31-alloc.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str31-alloc.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_analyzer_fixit %s %t \
+// RUN:  -analyzer-checker=core,unix,security.cert.Str \
+// RUN:  -analyzer-config security.cert.Str:WantToUseSafeFunctions=true \
+// RUN:  -I %S
+
+// See the examples on the page of STR31:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator
+
+#include "../Inputs/system-header-simulator.h"
+
+void *malloc(size_t size);
+void free(void *memblock);
+
+#define __STDC_LIB_EXT1__ 1
+#define __STDC_WANT_LIB_EXT1__ 1
+
+void test_ambiguous_parameter(char *buf) {
+  if (gets(buf)) {}
+  // expected-warning@-1 {{'gets' could write outside of 'buf'}}
+  // CHECK-FIXES: if (gets(buf))
+}
+
+void test_malloc(size_t size) {
+  char *buf1 = (char *)malloc(size);
+  if (gets(buf1)) {}
+  // expected-warning@-1 {{'gets' could write outside of 'buf1'}}
+  // CHECK-FIXES: if (gets_s(buf1, size)) {}
+  free(buf1);
+}
+
+void test_variable_array_ty(size_t size) {
+  char buf2[size];
+  if (gets(buf2)) {}
+  // expected-warning@-1 {{'gets' could write outside of 'buf2'}}
+  // CHECK-FIXES: if 

[PATCH] D57829: [CUDA][HIP] Disable emitting llvm.linker.options in device compilation

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenCUDA/linker-options.cu:1-5
+// RUN: %clang_cc1 -emit-llvm -o - -fcuda-is-device -fms-extensions -x hip %s \
+// RUN:   -fno-autolink -triple amdgcn-amd-amdhsa \
+// RUN:   | FileCheck -check-prefix=DEV %s
+// RUN: %clang_cc1 -emit-llvm -o - -fms-extensions -x hip %s -triple \
+// RUN:x86_64-pc-windows-msvc | FileCheck -check-prefix=HOST %s

tra wrote:
> This appears to be specific to HIP on windows. If that's intended, then the 
> file should be renamed to something like `hip-ms-linker-options.cu`. If the 
> changes in functionality apply on other platforms, or with CUDA, the file 
> name can remain as is, but it would be great to add some test runs for the 
> other uses cases.
the change in functionality applies to HIP and CUDA on windows only. will 
rename it to ms-linker-options.cu and add test for CUDA when committing.


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

https://reviews.llvm.org/D57829



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

[PATCH] D69786: [X86] Add support for -mvzeroupper and -mno-vzeroupper to match gcc.

[Craig Topper via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb2b6a54f847f: [X86] Add support for -mvzeroupper and 
-mno-vzeroupper to match gcc (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69786

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/test/Driver/x86-target-features.c
  llvm/docs/ReleaseNotes.rst
  llvm/lib/Target/X86/X86.td
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/lib/Target/X86/X86TargetTransformInfo.h
  llvm/lib/Target/X86/X86VZeroUpper.cpp
  llvm/test/CodeGen/X86/avx-vzeroupper.ll

Index: llvm/test/CodeGen/X86/avx-vzeroupper.ll
===
--- llvm/test/CodeGen/X86/avx-vzeroupper.ll
+++ llvm/test/CodeGen/X86/avx-vzeroupper.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -x86-use-vzeroupper -mtriple=x86_64-unknown-unknown -mattr=+avx | FileCheck %s --check-prefix=ALL --check-prefix=VZ --check-prefix=AVX
 ; RUN: llc < %s -x86-use-vzeroupper -mtriple=x86_64-unknown-unknown -mattr=+avx512f | FileCheck %s --check-prefix=ALL --check-prefix=VZ --check-prefix=AVX512
-; RUN: llc < %s -x86-use-vzeroupper -mtriple=x86_64-unknown-unknown -mattr=+avx,+fast-partial-ymm-or-zmm-write | FileCheck %s --check-prefix=ALL --check-prefix=NO-VZ --check-prefix=FAST-ymm-zmm
+; RUN: llc < %s -x86-use-vzeroupper -mtriple=x86_64-unknown-unknown -mattr=+avx,-vzeroupper | FileCheck %s --check-prefix=ALL --check-prefix=NO-VZ --check-prefix=DISABLE-VZ
 ; RUN: llc < %s -x86-use-vzeroupper -mtriple=x86_64-unknown-unknown -mcpu=bdver2 | FileCheck %s --check-prefix=ALL --check-prefix=NO-VZ --check-prefix=BDVER2
 ; RUN: llc < %s -x86-use-vzeroupper -mtriple=x86_64-unknown-unknown -mcpu=btver2 | FileCheck %s --check-prefix=ALL --check-prefix=NO-VZ --check-prefix=BTVER2
 
@@ -44,18 +44,18 @@
 ; VZ-NEXT:addq $56, %rsp
 ; VZ-NEXT:retq
 ;
-; FAST-ymm-zmm-LABEL: test01:
-; FAST-ymm-zmm:   # %bb.0:
-; FAST-ymm-zmm-NEXT:subq $56, %rsp
-; FAST-ymm-zmm-NEXT:vmovups %ymm2, (%rsp) # 32-byte Spill
-; FAST-ymm-zmm-NEXT:vmovaps {{.*}}(%rip), %xmm0
-; FAST-ymm-zmm-NEXT:callq do_sse
-; FAST-ymm-zmm-NEXT:vmovaps %xmm0, {{.*}}(%rip)
-; FAST-ymm-zmm-NEXT:callq do_sse
-; FAST-ymm-zmm-NEXT:vmovaps %xmm0, {{.*}}(%rip)
-; FAST-ymm-zmm-NEXT:vmovups (%rsp), %ymm0 # 32-byte Reload
-; FAST-ymm-zmm-NEXT:addq $56, %rsp
-; FAST-ymm-zmm-NEXT:retq
+; DISABLE-VZ-LABEL: test01:
+; DISABLE-VZ:   # %bb.0:
+; DISABLE-VZ-NEXT:subq $56, %rsp
+; DISABLE-VZ-NEXT:vmovups %ymm2, (%rsp) # 32-byte Spill
+; DISABLE-VZ-NEXT:vmovaps {{.*}}(%rip), %xmm0
+; DISABLE-VZ-NEXT:callq do_sse
+; DISABLE-VZ-NEXT:vmovaps %xmm0, {{.*}}(%rip)
+; DISABLE-VZ-NEXT:callq do_sse
+; DISABLE-VZ-NEXT:vmovaps %xmm0, {{.*}}(%rip)
+; DISABLE-VZ-NEXT:vmovups (%rsp), %ymm0 # 32-byte Reload
+; DISABLE-VZ-NEXT:addq $56, %rsp
+; DISABLE-VZ-NEXT:retq
 ;
 ; BDVER2-LABEL: test01:
 ; BDVER2:   # %bb.0:
@@ -83,6 +83,7 @@
 ; BTVER2-NEXT:vmovups (%rsp), %ymm0 # 32-byte Reload
 ; BTVER2-NEXT:addq $56, %rsp
 ; BTVER2-NEXT:retq
+; DISABLE-VZ   # %bb.0:
   %tmp = load <4 x float>, <4 x float>* @x, align 16
   %call = tail call <4 x float> @do_sse(<4 x float> %tmp) nounwind
   store <4 x float> %call, <4 x float>* @x, align 16
@@ -100,10 +101,10 @@
 ; VZ-NEXT:vzeroupper
 ; VZ-NEXT:jmp do_sse # TAILCALL
 ;
-; FAST-ymm-zmm-LABEL: test02:
-; FAST-ymm-zmm:   # %bb.0:
-; FAST-ymm-zmm-NEXT:vaddps %xmm1, %xmm0, %xmm0
-; FAST-ymm-zmm-NEXT:jmp do_sse # TAILCALL
+; DISABLE-VZ-LABEL: test02:
+; DISABLE-VZ:   # %bb.0:
+; DISABLE-VZ-NEXT:vaddps %xmm1, %xmm0, %xmm0
+; DISABLE-VZ-NEXT:jmp do_sse # TAILCALL
 ;
 ; BDVER2-LABEL: test02:
 ; BDVER2:   # %bb.0:
@@ -154,34 +155,34 @@
 ; VZ-NEXT:popq %rbx
 ; VZ-NEXT:retq
 ;
-; FAST-ymm-zmm-LABEL: test03:
-; FAST-ymm-zmm:   # %bb.0: # %entry
-; FAST-ymm-zmm-NEXT:pushq %rbx
-; FAST-ymm-zmm-NEXT:subq $16, %rsp
-; FAST-ymm-zmm-NEXT:vaddps %xmm1, %xmm0, %xmm0
-; FAST-ymm-zmm-NEXT:vmovaps %xmm0, (%rsp) # 16-byte Spill
-; FAST-ymm-zmm-NEXT:.p2align 4, 0x90
-; FAST-ymm-zmm-NEXT:  .LBB3_1: # %while.cond
-; FAST-ymm-zmm-NEXT:# =>This Inner Loop Header: Depth=1
-; FAST-ymm-zmm-NEXT:callq foo
-; FAST-ymm-zmm-NEXT:testl %eax, %eax
-; FAST-ymm-zmm-NEXT:jne .LBB3_1
-; FAST-ymm-zmm-NEXT:  # %bb.2: # %for.body.preheader
-; FAST-ymm-zmm-NEXT:movl $4, %ebx
-; FAST-ymm-zmm-NEXT:vmovaps (%rsp), %xmm0 # 16-byte Reload
-; FAST-ymm-zmm-NEXT:.p2align 4, 0x90
-; FAST-ymm-zmm-NEXT:  .LBB3_3: # %for.body
-; FAST-ymm-zmm-NEXT:# =>This Inner Loop Header: Depth=1
-; FAST-ymm-zmm-NEXT:callq do_sse
-; FAST-ymm-zmm-NEXT:callq do_sse
-; FAST-ymm-zmm-NEXT:vmovaps g+{{.*}}(%rip), %xmm0
-;