[PATCH] D97000: [clang] Increase the bitness of data length in ASTDeclContextNameLookup

[Danila Kutenin via Phabricator via cfe-commits]

danlark abandoned this revision.
danlark added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97000

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

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

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

steakhal added a comment.

In D96090#2572069 , @ASDenysPetrov 
wrote:

> In D96090#2568431 , @steakhal wrote:
>
>> Could you please rebase this?
>
> I'll update and rebase this patch soon.
>
>> If it depends on any parent revisions please document them as well.
>
> It does. You can see this in the stack. Do I need to mention this somewhere 
> else?

No, I probably missed that. My bad.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:101-103
   // FIXME: Make these protected again once RegionStoreManager correctly
   // handles loads from different bound value types.
   virtual SVal dispatchCast(SVal val, QualType castTy) = 0;

ASDenysPetrov wrote:
> steakhal wrote:
> > I'm not sure if this FIXME is still applicable.
> > 
> > I'm also confused about having two functions doing effectively the same 
> > thing.
> > `SimpleSValBuilder::dispatchCast` is a virtual function, which just invokes 
> > a non-virtual function `SValBuilder::evalCast`.
> > 
> > Why should it be virtual in the first place?
> I want to make the patch smaller avoiding things that could be changed 
> individually. We can remove `dispatchCast` in the next post-cleanup patch.
Seems reasonable to me.


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

https://reviews.llvm.org/D96090

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

[PATCH] D96866: [AArch64] Add some missing Neoverse features

[Sjoerd Meijer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG260f90bb3d1a: [AArch64] Add some missing Neoverse features 
(authored by SjoerdMeijer).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D96866?vs=324615&id=324910#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96866

Files:
  clang/test/Driver/aarch64-cpus.c
  llvm/lib/Target/AArch64/AArch64.td
  llvm/test/CodeGen/AArch64/misched-fusion-aes.ll

Index: llvm/test/CodeGen/AArch64/misched-fusion-aes.ll
===
--- llvm/test/CodeGen/AArch64/misched-fusion-aes.ll
+++ llvm/test/CodeGen/AArch64/misched-fusion-aes.ll
@@ -10,6 +10,10 @@
 ; RUN: llc %s -o - -mtriple=aarch64-unknown -mcpu=cortex-a78 | FileCheck %s
 ; RUN: llc %s -o - -mtriple=aarch64-unknown -mcpu=cortex-a78c| FileCheck %s
 ; RUN: llc %s -o - -mtriple=aarch64-unknown -mcpu=cortex-x1  | FileCheck %s
+; RUN: llc %s -o - -mtriple=aarch64-unknown -mcpu=neoverse-e1 | FileCheck %s
+; RUN: llc %s -o - -mtriple=aarch64-unknown -mcpu=neoverse-n1 | FileCheck %s
+; RUN: llc %s -o - -mtriple=aarch64-unknown -mcpu=neoverse-n2 | FileCheck %s
+; RUN: llc %s -o - -mtriple=aarch64-unknown -mcpu=neoverse-v1 | FileCheck %s
 ; RUN: llc %s -o - -mtriple=aarch64-unknown -mcpu=exynos-m3  | FileCheck %s
 ; RUN: llc %s -o - -mtriple=aarch64-unknown -mcpu=exynos-m4  | FileCheck %s
 ; RUN: llc %s -o - -mtriple=aarch64-unknown -mcpu=exynos-m5  | FileCheck %s
Index: llvm/lib/Target/AArch64/AArch64.td
===
--- llvm/lib/Target/AArch64/AArch64.td
+++ llvm/lib/Target/AArch64/AArch64.td
@@ -594,7 +594,9 @@
FeatureFullFP16,
FeatureDotProd,
FeatureRCPC,
-   FeaturePerfMon
+   FeaturePerfMon,
+   FeaturePostRAScheduler,
+   FeatureUseAA
]>;
 
 def ProcA57 : SubtargetFeature<"a57", "ARMProcFamily", "CortexA57",
@@ -728,7 +730,7 @@
"CortexR82",
"Cortex-R82 ARM Processors", [
FeaturePostRAScheduler,
-   // TODO: crypto and FuseAES
+   FeatureUseAA,
// All other features are implied by v8_0r ops:
HasV8_0rOps,
]>;
@@ -977,6 +979,9 @@
   FeatureNEON,
   FeatureRCPC,
   FeatureSSBS,
+  FeaturePostRAScheduler,
+  FeatureUseAA,
+  FeatureFuseAES,
   ]>;
 
 def ProcNeoverseN1 : SubtargetFeature<"neoversen1", "ARMProcFamily",
@@ -991,6 +996,9 @@
   FeatureRCPC,
   FeatureSPE,
   FeatureSSBS,
+  FeaturePostRAScheduler,
+  FeatureUseAA,
+  FeatureFuseAES,
   ]>;
 
 def ProcNeoverseN2 : SubtargetFeature<"neoversen2", "ARMProcFamily",
@@ -1003,7 +1011,12 @@
   FeatureMTE,
   FeatureSVE2,
   FeatureSVE2BitPerm,
-  FeatureTRBE]>;
+  FeatureTRBE,
+  FeaturePostRAScheduler,
+  FeatureUseAA,
+  FeatureCrypto,
+  FeatureFuseAES,
+  ]>;
 
 def ProcNeoverseV1 : SubtargetFeature<"neoversev1", "ARMProcFamily",
   "NeoverseV1",
@@ -1020,6 +1033,7 @@
   FeatureNEON,
   FeaturePerfMon,
   FeaturePostRAScheduler,
+  FeatureUseAA,
   FeatureRandGen,
   FeatureSPE,
   FeatureSSBS,
Index: clang/test/Driver/aarch64-cpus.c
===
--- clang/test/Driver/aarch64-cpus.c
+++ clang/test/Driver/aarch64-cpus.c
@@ -185,8 +185,14 @@
 // CORTEXA78: "-cc1"

[clang] 260f90b - [AArch64] Add some missing Neoverse features

[Sjoerd Meijer via cfe-commits]

Author: Sjoerd Meijer
Date: 2021-02-19T09:18:35Z
New Revision: 260f90bb3d1aef90764de3506f86dedd1339e37c

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

LOG: [AArch64] Add some missing Neoverse features

This enables AES fusion and the post RA scheduler for the Neoverse cores.
And while we are it also for the A55 that we had missed earlier.

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

Added: 


Modified: 
clang/test/Driver/aarch64-cpus.c
llvm/lib/Target/AArch64/AArch64.td
llvm/test/CodeGen/AArch64/misched-fusion-aes.ll

Removed: 




diff  --git a/clang/test/Driver/aarch64-cpus.c 
b/clang/test/Driver/aarch64-cpus.c
index 7ac2473915e8..5b9bd5207792 100644
--- a/clang/test/Driver/aarch64-cpus.c
+++ b/clang/test/Driver/aarch64-cpus.c
@@ -185,8 +185,14 @@
 // CORTEXA78: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a78"
 // RUN: %clang -target aarch64 -mcpu=cortex-a78c  -### -c %s 2>&1 | FileCheck 
-check-prefix=CORTEX-A78C %s
 // CORTEX-A78C: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" 
"cortex-a78c"
+// RUN: %clang -target aarch64 -mcpu=neoverse-e1  -### -c %s 2>&1 | FileCheck 
-check-prefix=NEOVERSE-E1 %s
+// NEOVERSE-E1: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" 
"neoverse-e1"
 // RUN: %clang -target aarch64 -mcpu=neoverse-v1  -### -c %s 2>&1 | FileCheck 
-check-prefix=NEOVERSE-V1 %s
 // NEOVERSE-V1: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" 
"neoverse-v1"
+// RUN: %clang -target aarch64 -mcpu=neoverse-n1 -### -c %s 2>&1 | FileCheck 
-check-prefix=NEOVERSE-N1 %s
+// NEOVERSE-N1: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" 
"neoverse-n1"
+// RUN: %clang -target aarch64 -mcpu=neoverse-n2 -### -c %s 2>&1 | FileCheck 
-check-prefix=NEOVERSE-N2 %s
+// NEOVERSE-N2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" 
"neoverse-n2"
 
 // RUN: %clang -target aarch64 -mcpu=cortex-r82  -### -c %s 2>&1 | FileCheck 
-check-prefix=CORTEXR82 %s
 // CORTEXR82: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-r82"
@@ -788,9 +794,6 @@
 // RUN: %clang -target aarch64 -march=armv8-a+ras -### -c %s 2>&1 | FileCheck 
-check-prefix=V8ARAS -check-prefix=GENERIC %s
 // V8ARAS: "-target-feature" "+ras"
 
-// RUN: %clang -target aarch64 -mcpu=neoverse-n2 -### -c %s 2>&1 | FileCheck 
-check-prefix=NEOVERSE-N2 %s
-// NEOVERSE-N2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" 
"neoverse-n2"
-
 // == Check whether -march accepts mixed-case values.
 // RUN: %clang -target aarch64_be -march=ARMV8.1A -### -c %s 2>&1 | FileCheck 
-check-prefix=GENERICV81A-BE %s
 // RUN: %clang -target aarch64_be -march=ARMV8.1-A -### -c %s 2>&1 | FileCheck 
-check-prefix=GENERICV81A-BE %s

diff  --git a/llvm/lib/Target/AArch64/AArch64.td 
b/llvm/lib/Target/AArch64/AArch64.td
index 762855207d2b..bdf2e517deda 100644
--- a/llvm/lib/Target/AArch64/AArch64.td
+++ b/llvm/lib/Target/AArch64/AArch64.td
@@ -594,7 +594,9 @@ def ProcA55 : SubtargetFeature<"a55", "ARMProcFamily", 
"CortexA55",
FeatureFullFP16,
FeatureDotProd,
FeatureRCPC,
-   FeaturePerfMon
+   FeaturePerfMon,
+   FeaturePostRAScheduler,
+   FeatureUseAA
]>;
 
 def ProcA57 : SubtargetFeature<"a57", "ARMProcFamily", "CortexA57",
@@ -728,7 +730,7 @@ def ProcR82 : SubtargetFeature<"cortex-r82", 
"ARMProcFamily",
"CortexR82",
"Cortex-R82 ARM Processors", [
FeaturePostRAScheduler,
-   // TODO: crypto and FuseAES
+   FeatureUseAA,
// All other features are implied by v8_0r ops:
HasV8_0rOps,
]>;
@@ -977,6 +979,9 @@ def ProcNeoverseE1 : SubtargetFeature<"neoversee1", 
"ARMProcFamily",
   FeatureNEON,
   FeatureRCPC,
   FeatureSSBS,
+  FeaturePostRAScheduler,
+  FeatureUseAA,
+  FeatureFuseAES,
   ]>;
 
 def ProcNeoverseN1 : SubtargetFeature<"neoversen1", "ARMProcFamily",
@@ -991,6 +996,9 @@ def ProcNeoverseN1 : SubtargetFeature<"neoversen1", 
"ARMProcFamily",
   FeatureRCPC,
   FeatureSPE,
   Feature

[PATCH] D96381: [AArch64] Adding SHA3 Intrinsics support

[David Spickett via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D96381

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

[PATCH] D96884: [flang][driver] Add more -fdebug options

[Faris Rehman via Phabricator via cfe-commits]

FarisRehman updated this revision to Diff 324926.
FarisRehman added a comment.

Remove -fdebug-parsing-log

Remove `-fdebug-parsing-log` from this patch as it requires additional work 
that can be implemented in a separate patch.
Also address review comments by @awarzynski


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96884

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Flang-Driver/debug-measure-parse-tree.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Lower/pre-fir-tree01.f90
  flang/test/Lower/pre-fir-tree02.f90
  flang/test/Lower/pre-fir-tree03.f90
  flang/test/Lower/pre-fir-tree05.f90

Index: flang/test/Lower/pre-fir-tree05.f90
===
--- flang/test/Lower/pre-fir-tree05.f90
+++ flang/test/Lower/pre-fir-tree05.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fdebug-pre-fir-tree -fsyntax-only -fopenacc %s | FileCheck %s
+! RUN: %flang_fc1 -fsyntax-only -fdebug-pre-fir-tree -fopenacc %s | FileCheck %s
 
 ! Test structure of the Pre-FIR tree with OpenACC construct
 
Index: flang/test/Lower/pre-fir-tree03.f90
===
--- flang/test/Lower/pre-fir-tree03.f90
+++ flang/test/Lower/pre-fir-tree03.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fdebug-pre-fir-tree -fsyntax-only -fopenmp %s | FileCheck %s
+! RUN: %flang_fc1 -fsyntax-only -fdebug-pre-fir-tree -fopenmp %s | FileCheck %s
 
 ! Test Pre-FIR Tree captures OpenMP related constructs
 
Index: flang/test/Lower/pre-fir-tree02.f90
===
--- flang/test/Lower/pre-fir-tree02.f90
+++ flang/test/Lower/pre-fir-tree02.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fdebug-pre-fir-tree -fsyntax-only %s | FileCheck %s
+! RUN: %flang_fc1 -fsyntax-only -fdebug-pre-fir-tree %s | FileCheck %s
 
 ! Test Pre-FIR Tree captures all the intended nodes from the parse-tree
 ! Coarray and OpenMP related nodes are tested in other files.
Index: flang/test/Lower/pre-fir-tree01.f90
===
--- flang/test/Lower/pre-fir-tree01.f90
+++ flang/test/Lower/pre-fir-tree01.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fdebug-pre-fir-tree -fsyntax-only %s | FileCheck %s
+! RUN: %flang_fc1 -fsyntax-only -fdebug-pre-fir-tree %s | FileCheck %s
 
 ! Test structure of the Pre-FIR tree
 
Index: flang/test/Flang-Driver/driver-help.f90
===
--- flang/test/Flang-Driver/driver-help.f90
+++ flang/test/Flang-Driver/driver-help.f90
@@ -59,6 +59,9 @@
 ! HELP-FC1-NEXT: -fdebug-dump-parse-tree Dump the parse tree
 ! HELP-FC1-NEXT: -fdebug-dump-provenance Dump provenance
 ! HELP-FC1-NEXT: -fdebug-dump-symbolsDump symbols after the semantic analysis
+! HELP-FC1-NEXT: -fdebug-measure-parse-tree
+! HELP-FC1-NEXT: Measure the parse tree
+! HELP-FC1-NEXT: -fdebug-pre-fir-treeDump the pre-FIR tree
 ! HELP-FC1-NEXT: -fdebug-unparse-with-symbols
 ! HELP-FC1-NEXT:Unparse and stop.
 ! HELP-FC1-NEXT: -fdebug-unparseUnparse and stop.
Index: flang/test/Flang-Driver/debug-measure-parse-tree.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/debug-measure-parse-tree.f90
@@ -0,0 +1,26 @@
+! Ensure argument -fdebug-measure-parse-tree works as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: not %flang-new -fdebug-measure-parse-tree %s  2>&1 | FileCheck %s --check-prefix=FLANG
+
+!
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!
+! RUN: %flang-new -fc1 -fdebug-measure-parse-tree %s  2>&1 | FileCheck %s --check-prefix=FRONTEND
+
+!--
+! EXPECTED OUTPUT WITH `flang-new`
+!--
+! FLANG:warning: argument unused during compilation: '-fdebug-measure-parse-tree'
+
+!---
+! EXPECTED OUTPUT WITH `flang-new -fc1`
+!---
+! FRONTEND:Parse tree comprises {{[0-9]+}} objects and occupies {{[0-9]+}} total bytes.
+
+program A
+end
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -52,6 +52,12 @@
   case DebugDumpProvenance:
 return std::make_unique();
 break;
+  case DebugMeasureParseTree:
+retu

[PATCH] D97041: [clang][cli] Pass '-Wspir-compat' to cc1 from driver

[Jan Svoboda via Phabricator via cfe-commits]

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

This patch moves the creation of the '-Wspir-compat' argument from cc1 to the 
driver.

Without this change, generating command line arguments from 
`CompilerInvocation` cannot be done reliably: there's no way to distinguish 
whether '-Wspir-compat' was passed to cc1 on the command line (should be 
generated), or if it was created within `CompilerInvocation::CreateFromArgs` 
(should not be generated).

This is also in line with how other '-Wxxx' flags are handled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97041

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/opencl.cl
  clang/test/SemaOpenCL/sampler_t.cl


Index: clang/test/SemaOpenCL/sampler_t.cl
===
--- clang/test/SemaOpenCL/sampler_t.cl
+++ clang/test/SemaOpenCL/sampler_t.cl
@@ -1,9 +1,9 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE 
-Wspir-compat -triple amdgcn--amdhsa
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE 
-triple spir-unknown-unknown
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE 
-Wspir-compat -triple spir-unknown-unknown
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only 
-DCHECK_SAMPLER_VALUE -Wspir-compat -triple amdgcn--amdhsa
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only 
-DCHECK_SAMPLER_VALUE -triple spir-unknown-unknown
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only 
-DCHECK_SAMPLER_VALUE -Wspir-compat -triple spir-unknown-unknown
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE   2
 #define CLK_NORMALIZED_COORDS_TRUE  1
Index: clang/test/Driver/opencl.cl
===
--- clang/test/Driver/opencl.cl
+++ clang/test/Driver/opencl.cl
@@ -19,6 +19,8 @@
 // RUN: %clang -S -### -cl-uniform-work-group-size %s 2>&1 | FileCheck 
--check-prefix=CHECK-UNIFORM-WG %s
 // RUN: not %clang -cl-std=c99 -DOPENCL %s 2>&1 | FileCheck 
--check-prefix=CHECK-C99 %s
 // RUN: not %clang -cl-std=invalid -DOPENCL %s 2>&1 | FileCheck 
--check-prefix=CHECK-INVALID %s
+// RUN: %clang -S -### -target spir-unknown-unknown %s 2>&1 | FileCheck 
--check-prefix=CHECK-W-SPIR-COMPAT %s
+// RUN: %clang -S -### -target amdgcn-amd-amdhsa-opencl %s 2>&1 | FileCheck 
--check-prefix=CHECK-NO-W-SPIR-COMPAT %s
 
 // CHECK-CL: "-cc1" {{.*}} "-cl-std=CL"
 // CHECK-CL10: "-cc1" {{.*}} "-cl-std=CL1.0"
@@ -45,4 +47,7 @@
 // CHECK-C99: error: invalid value 'c99' in '-cl-std=c99'
 // CHECK-INVALID: error: invalid value 'invalid' in '-cl-std=invalid'
 
+// CHECK-W-SPIR-COMPAT: "-Wspir-compat"
+// CHECK-NO-W-SPIR-COMPAT-NOT: "-Wspir-compat"
+
 kernel void func(void);
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4634,10 +4634,6 @@
 Success = false;
   }
 
-  // Turn on -Wspir-compat for SPIR target.
-  if (T.isSPIR())
-Res.getDiagnosticOpts().Warnings.push_back("spir-compat");
-
   // If sanitizer is enabled, disable OPT_ffine_grained_bitfield_accesses.
   if (Res.getCodeGenOpts().FineGrainedBitfieldAccesses &&
   !Res.getLangOpts()->Sanitize.empty()) {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4264,6 +4264,9 @@
   // are provided.
   TC.addClangWarningOptions(CmdArgs);
 
+  if (Triple.isSPIR())
+CmdArgs.push_back("-Wspir-compat");
+
   // Select the appropriate action.
   RewriteKind rewriteKind = RK_None;
 


Index: clang/test/SemaOpenCL/sampler_t.cl
===
--- clang/test/SemaOpenCL/sampler_t.cl
+++ clang/test/SemaOpenCL/sampler_t.cl
@@ -1,9 +1,9 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple amdgcn--amdhsa
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -triple spir-unknown-unknown
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple spir-unknown-unknown
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple amdgcn--amdhsa
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -triple 

[PATCH] D96884: [flang][driver] Add debug measure-parse-tree and pre-fir-tree options

[Andrzej Warzynski via Phabricator via cfe-commits]

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

Thank you for updating this @FarisRehman !

The original implementation of ``-fdebug-parsing-log` submitted here was not 
100% compatible with `-fdebug-instrumented-parse` from `f18` as we originally 
thought. I think that that's fine, but we need something that gives us such 
compatibility. We probably need another flag for that, e.g. 
`-finstrumented-parse`, that would toggle `Fortran::parser::instrumentedParse`. 
This way we will have this:

- `-fdebug-parsing-log` (`flang-new -fc1`) != `-fdebug-instrumented-parse` 
(`f18`)
- `-fdebug-parsing-log -finstrumented-parse` (`flang-new -fc1`) == 
`-fdebug-instrumented-parse` (`f18`)

But that's quite a few new scenarios and it's worth submitting as a separate 
patch.

You've also changed the order of flags in tests. This is also fine and 
guarantees that tests work with both `f18` and `flang-new -fc1`:

- in `flang-new -fc1` only the last _action_ option is take into account, so 
`-fsyntax-only` is effectively ignored (and not needed)
- in `f18` we still need `-fsyntax-only` to make sure that code-generation is 
not run (e.g. to make `f18` exit _after_ running the semantic checks), but the 
order is irrelevant

All this makes a lot of sense, thank you for adding these! LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96884

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

[PATCH] D97042: [clang][cli] Stop creating '-Wno-stdlibcxx-not-found' in cc1

[Jan Svoboda via Phabricator via cfe-commits]

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

This patch stops creating the '-Wno-stdlibcxx-not-found' argument in 
`CompilerInvocation::CreateFromArgs`.

The code was added in 2e7ab55e657f (a follow-up to D48297 
). However, D61963 
 removes relevant tests and starts explicitly 
passing '-Wno-stdlibcxx-not-found' to the driver. I think it's fair to assume 
this is a dead code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97042

Files:
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4588,12 +4588,6 @@
  Res.getPreprocessorOpts().Includes, Diags);
 if (Res.getFrontendOpts().ProgramAction == frontend::RewriteObjC)
   LangOpts.ObjCExceptions = 1;
-if (T.isOSDarwin() && DashX.isPreprocessed()) {
-  // Supress the darwin-specific 'stdlibcxx-not-found' diagnostic for
-  // preprocessed input as we don't expect it to be used with -std=libc++
-  // anyway.
-  Res.getDiagnosticOpts().Warnings.push_back("no-stdlibcxx-not-found");
-}
   }
 
   if (LangOpts.CUDA) {


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4588,12 +4588,6 @@
  Res.getPreprocessorOpts().Includes, Diags);
 if (Res.getFrontendOpts().ProgramAction == frontend::RewriteObjC)
   LangOpts.ObjCExceptions = 1;
-if (T.isOSDarwin() && DashX.isPreprocessed()) {
-  // Supress the darwin-specific 'stdlibcxx-not-found' diagnostic for
-  // preprocessed input as we don't expect it to be used with -std=libc++
-  // anyway.
-  Res.getDiagnosticOpts().Warnings.push_back("no-stdlibcxx-not-found");
-}
   }
 
   if (LangOpts.CUDA) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96280: [clang][cli] Round-trip the whole CompilerInvocation

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 324933.
jansvoboda11 added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96280

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Frontend/CompilerInvocation.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/round-trip-cc1-args.c

Index: clang/test/Frontend/round-trip-cc1-args.c
===
--- clang/test/Frontend/round-trip-cc1-args.c
+++ clang/test/Frontend/round-trip-cc1-args.c
@@ -4,4 +4,4 @@
 
 // CHECK-WITHOUT-ROUND-TRIP-NOT: remark:
 // CHECK-ROUND-TRIP-WITHOUT-REMARKS-NOT: remark:
-// CHECK-ROUND-TRIP-WITH-REMARKS: remark: Generated arguments {{.*}} in {{.*}} round-trip: {{.*}}
+// CHECK-ROUND-TRIP-WITH-REMARKS: remark: Generated arguments #{{.*}} in round-trip: {{.*}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -423,9 +423,11 @@
 
 static const StringRef GetInputKindName(InputKind IK);
 
-static void FixupInvocation(CompilerInvocation &Invocation,
-DiagnosticsEngine &Diags, const InputArgList &Args,
+static bool FixupInvocation(CompilerInvocation &Invocation,
+DiagnosticsEngine &Diags, const ArgList &Args,
 InputKind IK) {
+  unsigned NumErrorsBefore = Diags.getNumErrors();
+
   LangOptions &LangOpts = *Invocation.getLangOpts();
   CodeGenOptions &CodeGenOpts = Invocation.getCodeGenOpts();
   TargetOptions &TargetOpts = Invocation.getTargetOpts();
@@ -502,6 +504,8 @@
 Diags.Report(diag::err_drv_argument_only_allowed_with)
 << Args.getLastArg(OPT_fprofile_remapping_file_EQ)->getAsString(Args)
 << "-fno-legacy-pass-manager";
+
+  return Diags.getNumErrors() == NumErrorsBefore;
 }
 
 //===--===//
@@ -569,27 +573,29 @@
 Opt.getKind(), 0, Value);
 }
 
-// Parse subset of command line arguments into a member of CompilerInvocation.
-using ParseFn = llvm::function_ref;
+// Parse command line arguments into CompilerInvocation.
+using ParseFn =
+llvm::function_ref,
+DiagnosticsEngine &, const char *)>;
 
-// Generate part of command line arguments from a member of CompilerInvocation.
+// Generate command line arguments from CompilerInvocation.
 using GenerateFn = llvm::function_ref &,
 CompilerInvocation::StringAllocator)>;
 
-// Swap between dummy/real instance of a CompilerInvocation member.
-using SwapOptsFn = llvm::function_ref;
-
-// Performs round-trip of command line arguments if OriginalArgs contain
-// "-round-trip-args". Effectively runs the Parse function for a part of
-// CompilerInvocation on command line arguments that were already once parsed
-// and generated. This is used to check the Generate function produces arguments
-// that are semantically equivalent to those that were used to create
-// CompilerInvocation.
-static bool RoundTrip(ParseFn Parse, GenerateFn Generate, SwapOptsFn SwapOpts,
-  CompilerInvocation &Res, ArgList &OriginalArgs,
-  DiagnosticsEngine &Diags, StringRef OptsName) {
+// May perform round-trip of command line arguments. By default, the round-trip
+// is enabled if CLANG_ROUND_TRIP_CC1_ARGS was defined during build. This can be
+// overwritten at run-time via the "-round-trip-args" and "-no-round-trip-args"
+// command line flags.
+// During round-trip, the command line arguments are parsed into a dummy
+// instance of CompilerInvocation which is used to generate the command line
+// arguments again. The real CompilerInvocation instance is then created by
+// parsing the generated arguments, not the original ones.
+static bool RoundTrip(ParseFn Parse, GenerateFn Generate,
+  CompilerInvocation &RealInvocation,
+  CompilerInvocation &DummyInvocation,
+  ArrayRef CommandLineArgs,
+  DiagnosticsEngine &Diags, const char *Argv0) {
   // FIXME: Switch to '#ifndef NDEBUG' when possible.
 #ifdef CLANG_ROUND_TRIP_CC1_ARGS
   bool DoRoundTripDefault = true;
@@ -597,16 +603,21 @@
   bool DoRoundTripDefault = false;
 #endif
 
-  bool DoRoundTrip = OriginalArgs.hasFlag(
-  OPT_round_trip_args, OPT_no_round_trip_args, DoRoundTripDefault);
+  bool DoRoundTrip = DoRoundTripDefault;
+  for (const auto *Arg : CommandLineArgs) {
+if (Arg == StringRef("-round-trip-args"))
+  DoRoundTrip = true;
+if (Arg == StringRef("-no-round-trip-args"))
+  DoRoundTrip = false;
+  }
 
-  // If round-trip was not requested, simply run the parser with the original
-  // options and diagnostics.
+  // If round-trip was not req

[PATCH] D96848: [clang][cli] Don't emit manufactured warnings

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 abandoned this revision.
jansvoboda11 added a comment.

Abandoning this in favor of D97041  & D97042 
.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2347-2353
+// This warning was manufactured, don't put it on the command line.
+if (Warning == "no-stdlibcxx-not-found" && T.isOSDarwin() &&
+DashX.isPreprocessed())
+  continue;
+// This warning was manufactured, don't put it on the command line.
+if (Warning == "spir-compat" && T.isSPIR())
+  continue;

dexonsmith wrote:
> It seems reasonable to skip generating them if they're implied by other 
> command-line options, but I'm not sure "manufactured" is the right word to 
> use as a distinguishing characteristic. The entire CompilerInvocation could 
> have been created programmatically. I suggest instead saying the warning flag 
> is implied by the other command-line options.
> 
> Also, note that when created programmatically, one could have pushed 
> `stdlibcxx-not-found` *after* pushing `no-stdlibcxx-not-found`. Since that's 
> impossible to recreate, maybe there should be an assertion to catch this? 
> Alternatively, should this kind of imply-diagnostic-options logic be moved to 
> the driver?
> 
> Relatedly, unlike most command-line options, GenerateDiagnosticArgs is not 
> canonicalizing the options. For example, if `-Wabc` implies `-Wdef`, it'd be 
> nice to generate just `-Wabc` from initial command-lines of either `-Wabc 
> -Wdef` or `-Wdef -Wabc` / to drop the first of `-Wno-abc -Wabc` / etc.
> 
> IMO, something akin to an initial DiagnosticsEngine::DiagState (likely 
> renamed) could be stored in DiagnosticOptions (effectively, the resulting 
> state from calling ProcessWarningOptions). Parsing could translate 
> command-line options to this initial state. The state could be modified 
> programmatically; it'd also be used to initialize DiagnosticsEngine. 
> Generating command-line options would emit a canonical set of options that 
> would recreate the state. But that's a pretty big refactoring, and I think 
> it's okay to make progress without that.
> 
> As an initial fix, this is probably fine, but I think the comments and/or 
> FIXMEs should acknowledge that it's a bit fragile and point in a more sound / 
> less fragile direction (doesn't have to be my suggestion).
Using "implied" would be fitting as well. I guess I wanted to distinguish this 
from other implied options that don't need any explicit special-casing.

You're right the way of handling warnings is fragile. Normalizing the arguments 
into `DiagState` would be more robust, but as you say, it's a bit more 
involved. I'm putting that on my back-burner.

In the end, I looked into the history of `-Wno-stdlibcxx-not-found` and 
`-Wspir-compat` more closely and found ways to remove them from 
`CompilerInvocation` entirely: D97041 & D97042.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96848

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

[clang] 529f718 - [flang][driver] Add debug measure-parse-tree and pre-fir-tree options

[Faris Rehman via cfe-commits]

Author: Faris Rehman
Date: 2021-02-19T11:27:54Z
New Revision: 529f71811b0475995f2d9cf766f18d897eec574c

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

LOG: [flang][driver] Add debug measure-parse-tree and pre-fir-tree options

Add the following options:
* -fdebug-measure-parse-tree
* -fdebug-pre-fir-tree

Summary of changes:
- Add 2 new frontend actions: DebugMeasureParseTreeAction and 
DebugPreFIRTreeAction
- Add MeasurementVisitor to FrontendActions.h
- Make reportFatalSemanticErrors return true if there are any fatal errors
- Port most of the `-fdebug-pre-fir-tree` tests to use the new driver if built, 
otherwise use f18.

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

Added: 
flang/test/Flang-Driver/debug-measure-parse-tree.f90

Modified: 
clang/include/clang/Driver/Options.td
flang/include/flang/Frontend/FrontendActions.h
flang/include/flang/Frontend/FrontendOptions.h
flang/lib/Frontend/CMakeLists.txt
flang/lib/Frontend/CompilerInvocation.cpp
flang/lib/Frontend/FrontendActions.cpp
flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
flang/test/Flang-Driver/driver-help.f90
flang/test/Lower/pre-fir-tree01.f90
flang/test/Lower/pre-fir-tree02.f90
flang/test/Lower/pre-fir-tree03.f90
flang/test/Lower/pre-fir-tree05.f90

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 17eb0ade2493..ade330bd7ba4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4288,6 +4288,10 @@ def fdebug_dump_parse_tree : Flag<["-"], 
"fdebug-dump-parse-tree">, Group;
 def fdebug_dump_provenance : Flag<["-"], "fdebug-dump-provenance">, 
Group,
   HelpText<"Dump provenance">;
+def fdebug_measure_parse_tree : Flag<["-"], "fdebug-measure-parse-tree">, 
Group,
+  HelpText<"Measure the parse tree">;
+def fdebug_pre_fir_tree : Flag<["-"], "fdebug-pre-fir-tree">, 
Group,
+  HelpText<"Dump the pre-FIR tree">;
 
 }
 

diff  --git a/flang/include/flang/Frontend/FrontendActions.h 
b/flang/include/flang/Frontend/FrontendActions.h
index ebcb695cca72..f6dfdd2dc09b 100644
--- a/flang/include/flang/Frontend/FrontendActions.h
+++ b/flang/include/flang/Frontend/FrontendActions.h
@@ -15,6 +15,17 @@
 
 namespace Fortran::frontend {
 
+// TODO: This is a copy from f18.cpp. It doesn't really belong here and should
+// be moved to a more suitable place in future.
+struct MeasurementVisitor {
+  template  bool Pre(const A &) { return true; }
+  template  void Post(const A &) {
+++objects;
+bytes += sizeof(A);
+  }
+  size_t objects{0}, bytes{0};
+};
+
 
//===--===//
 // Custom Consumer Actions
 
//===--===//
@@ -43,6 +54,10 @@ class DebugDumpProvenanceAction : public PrescanAction {
   void ExecuteAction() override;
 };
 
+class DebugMeasureParseTreeAction : public PrescanAction {
+  void ExecuteAction() override;
+};
+
 
//===--===//
 // PrescanAndSema Actions
 
//===--===//
@@ -77,6 +92,10 @@ class DebugDumpParseTreeAction : public PrescanAndSemaAction 
{
   void ExecuteAction() override;
 };
 
+class DebugPreFIRTreeAction : public PrescanAndSemaAction {
+  void ExecuteAction() override;
+};
+
 class ParseSyntaxOnlyAction : public PrescanAndSemaAction {
   void ExecuteAction() override;
 };

diff  --git a/flang/include/flang/Frontend/FrontendOptions.h 
b/flang/include/flang/Frontend/FrontendOptions.h
index a26e1e3d84c7..98a717dfa6ec 100644
--- a/flang/include/flang/Frontend/FrontendOptions.h
+++ b/flang/include/flang/Frontend/FrontendOptions.h
@@ -48,7 +48,14 @@ enum ActionKind {
   DebugDumpParseTree,
 
   /// Dump provenance
-  DebugDumpProvenance
+  DebugDumpProvenance,
+
+  /// Parse then output the number of objects in the parse tree and the overall
+  /// size
+  DebugMeasureParseTree,
+
+  /// Parse, run semantics and then output the pre-FIR tree
+  DebugPreFIRTree
 
   /// TODO: RunPreprocessor, EmitLLVM, EmitLLVMOnly,
   /// EmitCodeGenOnly, EmitAssembly, (...)

diff  --git a/flang/lib/Frontend/CMakeLists.txt 
b/flang/lib/Frontend/CMakeLists.txt
index 53c2518d2912..abaa77f6af54 100644
--- a/flang/lib/Frontend/CMakeLists.txt
+++ b/flang/lib/Frontend/CMakeLists.txt
@@ -16,6 +16,7 @@ add_flang_library(flangFrontend
   FortranSemantics
   FortranEvaluate
   FortranCommon
+  FortranLower
   clangBasic
   clangDriver
 

diff  --git a/flang/lib/Frontend/CompilerInvocation.cpp 
b/flang/lib/Frontend/CompilerInvocation.cpp
index ecc0fda4546b..822bf26f3577 100644
--- a/flang/lib/Frontend/Compil

[PATCH] D96884: [flang][driver] Add debug measure-parse-tree and pre-fir-tree options

[Faris Rehman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG529f71811b04: [flang][driver] Add debug measure-parse-tree 
and pre-fir-tree options (authored by FarisRehman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96884

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Flang-Driver/debug-measure-parse-tree.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Lower/pre-fir-tree01.f90
  flang/test/Lower/pre-fir-tree02.f90
  flang/test/Lower/pre-fir-tree03.f90
  flang/test/Lower/pre-fir-tree05.f90

Index: flang/test/Lower/pre-fir-tree05.f90
===
--- flang/test/Lower/pre-fir-tree05.f90
+++ flang/test/Lower/pre-fir-tree05.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fdebug-pre-fir-tree -fsyntax-only -fopenacc %s | FileCheck %s
+! RUN: %flang_fc1 -fsyntax-only -fdebug-pre-fir-tree -fopenacc %s | FileCheck %s
 
 ! Test structure of the Pre-FIR tree with OpenACC construct
 
Index: flang/test/Lower/pre-fir-tree03.f90
===
--- flang/test/Lower/pre-fir-tree03.f90
+++ flang/test/Lower/pre-fir-tree03.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fdebug-pre-fir-tree -fsyntax-only -fopenmp %s | FileCheck %s
+! RUN: %flang_fc1 -fsyntax-only -fdebug-pre-fir-tree -fopenmp %s | FileCheck %s
 
 ! Test Pre-FIR Tree captures OpenMP related constructs
 
Index: flang/test/Lower/pre-fir-tree02.f90
===
--- flang/test/Lower/pre-fir-tree02.f90
+++ flang/test/Lower/pre-fir-tree02.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fdebug-pre-fir-tree -fsyntax-only %s | FileCheck %s
+! RUN: %flang_fc1 -fsyntax-only -fdebug-pre-fir-tree %s | FileCheck %s
 
 ! Test Pre-FIR Tree captures all the intended nodes from the parse-tree
 ! Coarray and OpenMP related nodes are tested in other files.
Index: flang/test/Lower/pre-fir-tree01.f90
===
--- flang/test/Lower/pre-fir-tree01.f90
+++ flang/test/Lower/pre-fir-tree01.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fdebug-pre-fir-tree -fsyntax-only %s | FileCheck %s
+! RUN: %flang_fc1 -fsyntax-only -fdebug-pre-fir-tree %s | FileCheck %s
 
 ! Test structure of the Pre-FIR tree
 
Index: flang/test/Flang-Driver/driver-help.f90
===
--- flang/test/Flang-Driver/driver-help.f90
+++ flang/test/Flang-Driver/driver-help.f90
@@ -59,6 +59,9 @@
 ! HELP-FC1-NEXT: -fdebug-dump-parse-tree Dump the parse tree
 ! HELP-FC1-NEXT: -fdebug-dump-provenance Dump provenance
 ! HELP-FC1-NEXT: -fdebug-dump-symbolsDump symbols after the semantic analysis
+! HELP-FC1-NEXT: -fdebug-measure-parse-tree
+! HELP-FC1-NEXT: Measure the parse tree
+! HELP-FC1-NEXT: -fdebug-pre-fir-treeDump the pre-FIR tree
 ! HELP-FC1-NEXT: -fdebug-unparse-with-symbols
 ! HELP-FC1-NEXT:Unparse and stop.
 ! HELP-FC1-NEXT: -fdebug-unparseUnparse and stop.
Index: flang/test/Flang-Driver/debug-measure-parse-tree.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/debug-measure-parse-tree.f90
@@ -0,0 +1,26 @@
+! Ensure argument -fdebug-measure-parse-tree works as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: not %flang-new -fdebug-measure-parse-tree %s  2>&1 | FileCheck %s --check-prefix=FLANG
+
+!
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!
+! RUN: %flang-new -fc1 -fdebug-measure-parse-tree %s  2>&1 | FileCheck %s --check-prefix=FRONTEND
+
+!--
+! EXPECTED OUTPUT WITH `flang-new`
+!--
+! FLANG:warning: argument unused during compilation: '-fdebug-measure-parse-tree'
+
+!---
+! EXPECTED OUTPUT WITH `flang-new -fc1`
+!---
+! FRONTEND:Parse tree comprises {{[0-9]+}} objects and occupies {{[0-9]+}} total bytes.
+
+program A
+end
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -52,6 +52,12 @@
   case DebugDumpProvenance:
 return std::make_unique();
 break;
+  case DebugMeasureParseTree:
+return std::make_u

[PATCH] D96905: [libclang] Add missing fixed point literal CursorKind to cindex.py

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

Polite ping. This is a small and simple change, my only doubt is the test 
coverage. But frankly I see none of the literals are covered in any of the test 
cases (unless I'm missing something). Could I get some guidance on required 
test coverage for this simple change, or a LGTM?

Thanks - Vince


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96905

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

[PATCH] D97043: [clang][DeclPrinter] Pass Context into StmtPrinter whenever possible

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

ASTContext were only passed to the StmtPrinter in some places, while it
is always available in DeclPrinter. The context is used by StmtPrinter to better
print statements in some cases, like printing constants as written.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97043

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/unittests/AST/DeclPrinterTest.cpp

Index: clang/unittests/AST/DeclPrinterTest.cpp
===
--- clang/unittests/AST/DeclPrinterTest.cpp
+++ clang/unittests/AST/DeclPrinterTest.cpp
@@ -20,6 +20,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
@@ -1438,3 +1439,11 @@
   namedDecl(hasName("Extension")).bind("id"),
   "@implementation <>(Extension)\n@end", /*AllowError=*/true));
 }
+
+TEST(DeclPrinter, VarDeclWithInitializer) {
+  ASSERT_TRUE(PrintedDeclCXX17Matches(
+  "int a = 0x15;", namedDecl(hasName("a")).bind("id"), "int a = 21"));
+  ASSERT_TRUE(PrintedDeclCXX17Matches(
+  "int a = 0x15;", namedDecl(hasName("a")).bind("id"), "int a = 0x15",
+  [](PrintingPolicy &Policy) { Policy.ConstantsAsWritten = true; }));
+}
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -341,7 +341,8 @@
 SimpleInit = Init;
 
   if (SimpleInit)
-SimpleInit->printPretty(Out, nullptr, Policy, Indentation);
+SimpleInit->printPretty(Out, nullptr, Policy, Indentation, "\n",
+&Context);
   else {
 for (unsigned I = 0; I != NumArgs; ++I) {
   assert(Args[I] != nullptr && "Expected non-null Expr");
@@ -350,7 +351,8 @@
 
   if (I)
 Out << ", ";
-  Args[I]->printPretty(Out, nullptr, Policy, Indentation);
+  Args[I]->printPretty(Out, nullptr, Policy, Indentation, "\n",
+   &Context);
 }
   }
 }
@@ -568,13 +570,14 @@
 }
 
 static void printExplicitSpecifier(ExplicitSpecifier ES, llvm::raw_ostream &Out,
-   PrintingPolicy &Policy,
-   unsigned Indentation) {
+   PrintingPolicy &Policy, unsigned Indentation,
+   const ASTContext &Context) {
   std::string Proto = "explicit";
   llvm::raw_string_ostream EOut(Proto);
   if (ES.getExpr()) {
 EOut << "(";
-ES.getExpr()->printPretty(EOut, nullptr, Policy, Indentation);
+ES.getExpr()->printPretty(EOut, nullptr, Policy, Indentation, "\n",
+  &Context);
 EOut << ")";
   }
   EOut << " ";
@@ -616,7 +619,7 @@
 if (D->isConsteval())Out << "consteval ";
 ExplicitSpecifier ExplicitSpec = ExplicitSpecifier::getFromDecl(D);
 if (ExplicitSpec.isSpecified())
-  printExplicitSpecifier(ExplicitSpec, Out, Policy, Indentation);
+  printExplicitSpecifier(ExplicitSpec, Out, Policy, Indentation, Context);
   }
 
   PrintingPolicy SubPolicy(Policy);
@@ -720,7 +723,7 @@
 Proto += "(";
 llvm::raw_string_ostream EOut(Proto);
 FT->getNoexceptExpr()->printPretty(EOut, nullptr, SubPolicy,
-   Indentation);
+   Indentation, "\n", &Context);
 EOut.flush();
 Proto += EOut.str();
 Proto += ")";
@@ -744,7 +747,8 @@
 
 if (Expr *TrailingRequiresClause = D->getTrailingRequiresClause()) {
   Out << " requires ";
-  TrailingRequiresClause->printPretty(Out, nullptr, SubPolicy, Indentation);
+  TrailingRequiresClause->printPretty(Out, nullptr, SubPolicy, Indentation,
+  "\n", &Context);
 }
   } else {
 Ty.print(Out, Policy, Proto);
@@ -776,7 +780,8 @@
 Out << ' ';
 
   if (D->getBody())
-D->getBody()->printPretty(Out, nullptr, SubPolicy, Indentation);
+D->getBody()->printPretty(Out, nullptr, SubPolicy, Indentation, "\n",
+  &Context);
 } else {
   if (!Policy.TerseOutput && isa(*D))
 Out << " {}";
@@ -821,7 +826,8 @@
 
   if (D->isBitField()) {
 Out << " : ";
-D->getBitWidth()->printPretty(Out, nullptr, Policy, Indentation);
+D->getBitWidth()->printPretty(Out, nullptr, Policy, Indentation, "\n",
+  &Context);
   }
 
   Expr *Init = D->getInClassInitializer();
@@ -830,7 +836,7 

[PATCH] D97043: [clang][DeclPrinter] Pass Context into StmtPrinter whenever possible

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:526
  HI.Name = "result";
- HI.Definition = "static constexpr int result = 1 + 2";
+ HI.Definition = "static constexpr int result = a + b";
  HI.Kind = index::SymbolKind::StaticProperty;

not sure if this is a regression or not, as in theory we print the definition 
as written.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97043

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

[PATCH] D96950: [clang][CodeComplete] Ensure there are no crashes when completing with ParenListExprs as LHS

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 324945.
kadircet marked 4 inline comments as done.
kadircet added a comment.

- Pull unwrapping of parenlistexprs into a function
- Merge comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96950

Files:
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5245,6 +5245,19 @@
   return Unresolved;
 }
 
+// If \p Base is ParenListExpr, assume a chain of comma operators and pick the
+// last expr. We expect other ParenListExprs to be resolved to e.g. constructor
+// calls before here. (So the ParenListExpr should be nonempty, but check just
+// in case)
+Expr *unwrapParenList(Expr *Base) {
+  if (auto *PLE = llvm::dyn_cast_or_null(Base)) {
+if (PLE->getNumExprs() == 0)
+  return nullptr;
+Base = PLE->getExpr(PLE->getNumExprs() - 1);
+  }
+  return Base;
+}
+
 } // namespace
 
 void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
@@ -5252,18 +5265,11 @@
SourceLocation OpLoc, bool IsArrow,
bool IsBaseExprStatement,
QualType PreferredType) {
+  Base = unwrapParenList(Base);
+  OtherOpBase = unwrapParenList(OtherOpBase);
   if (!Base || !CodeCompleter)
 return;
 
-  // Peel off the ParenListExpr by chosing the last one, as they don't have a
-  // predefined type.
-  if (auto *PLE = llvm::dyn_cast(Base))
-Base = PLE->getExpr(PLE->getNumExprs() - 1);
-  if (OtherOpBase) {
-if (auto *PLE = llvm::dyn_cast(OtherOpBase))
-  OtherOpBase = PLE->getExpr(PLE->getNumExprs() - 1);
-  }
-
   ExprResult ConvertedBase = PerformMemberExprBaseConversion(Base, IsArrow);
   if (ConvertedBase.isInvalid())
 return;
@@ -5693,14 +5699,10 @@
 QualType Sema::ProduceCallSignatureHelp(Scope *S, Expr *Fn,
 ArrayRef Args,
 SourceLocation OpenParLoc) {
+  Fn = unwrapParenList(Fn);
   if (!CodeCompleter || !Fn)
 return QualType();
 
-  // If we have a ParenListExpr for LHS, peel it off by chosing the last expr.
-  // As ParenListExprs don't have a predefined type.
-  if (auto *PLE = llvm::dyn_cast(Fn))
-Fn = PLE->getExpr(PLE->getNumExprs() - 1);
-
   // FIXME: Provide support for variadic template functions.
   // Ignore type-dependent call expressions entirely.
   if (Fn->isTypeDependent() || anyNullArguments(Args))


Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5245,6 +5245,19 @@
   return Unresolved;
 }
 
+// If \p Base is ParenListExpr, assume a chain of comma operators and pick the
+// last expr. We expect other ParenListExprs to be resolved to e.g. constructor
+// calls before here. (So the ParenListExpr should be nonempty, but check just
+// in case)
+Expr *unwrapParenList(Expr *Base) {
+  if (auto *PLE = llvm::dyn_cast_or_null(Base)) {
+if (PLE->getNumExprs() == 0)
+  return nullptr;
+Base = PLE->getExpr(PLE->getNumExprs() - 1);
+  }
+  return Base;
+}
+
 } // namespace
 
 void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
@@ -5252,18 +5265,11 @@
SourceLocation OpLoc, bool IsArrow,
bool IsBaseExprStatement,
QualType PreferredType) {
+  Base = unwrapParenList(Base);
+  OtherOpBase = unwrapParenList(OtherOpBase);
   if (!Base || !CodeCompleter)
 return;
 
-  // Peel off the ParenListExpr by chosing the last one, as they don't have a
-  // predefined type.
-  if (auto *PLE = llvm::dyn_cast(Base))
-Base = PLE->getExpr(PLE->getNumExprs() - 1);
-  if (OtherOpBase) {
-if (auto *PLE = llvm::dyn_cast(OtherOpBase))
-  OtherOpBase = PLE->getExpr(PLE->getNumExprs() - 1);
-  }
-
   ExprResult ConvertedBase = PerformMemberExprBaseConversion(Base, IsArrow);
   if (ConvertedBase.isInvalid())
 return;
@@ -5693,14 +5699,10 @@
 QualType Sema::ProduceCallSignatureHelp(Scope *S, Expr *Fn,
 ArrayRef Args,
 SourceLocation OpenParLoc) {
+  Fn = unwrapParenList(Fn);
   if (!CodeCompleter || !Fn)
 return QualType();
 
-  // If we have a ParenListExpr for LHS, peel it off by chosing the last expr.
-  // As ParenListExprs don't have a predefined type.
-  if (auto *PLE = llvm::dyn_cast(Fn))
-Fn = PLE->getExpr(PLE->getNumExprs() - 1);
-
   // FIXME: Provide support for variadic template functions.
   // Ignore type-dependent call expressions entirely.
   if (Fn->isTypeDependent() || anyNullArg

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 324946.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Update comment.
- Get rid of a parent_path usage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96123

Files:
  clang-tools-extra/clangd/ConfigProvider.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h

Index: clang-tools-extra/clangd/support/Path.h
===
--- clang-tools-extra/clangd/support/Path.h
+++ clang-tools-extra/clangd/support/Path.h
@@ -28,6 +28,9 @@
 std::string maybeCaseFoldPath(PathRef Path);
 bool pathEqual(PathRef, PathRef);
 
+/// Variant of parent_path that operates only on absolute paths.
+/// Unlike parent_path doesn't consider C: a parent of C:\.
+PathRef absoluteParent(PathRef Path);
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/support/Path.cpp
===
--- clang-tools-extra/clangd/support/Path.cpp
+++ clang-tools-extra/clangd/support/Path.cpp
@@ -7,9 +7,25 @@
 //===--===//
 
 #include "support/Path.h"
+#include "llvm/Support/Path.h"
+
 namespace clang {
 namespace clangd {
 
+PathRef absoluteParent(PathRef Path) {
+  assert(llvm::sys::path::is_absolute(Path));
+#if defined(_WIN32)
+  // llvm::sys says "C:\" is absolute, and its parent is "C:" which is relative.
+  // This unhelpful behavior seems to have been inherited from boost.
+  if (llvm::sys::path::relative_path(Path).empty()) {
+return PathRef();
+  }
+#endif
+  PathRef Result = llvm::sys::path::parent_path(Path);
+  assert(Result.empty() || llvm::sys::path::is_absolute(Result));
+  return Result;
+}
+
 std::string maybeCaseFoldPath(PathRef Path) {
 #if defined(_WIN32) || defined(__APPLE__)
   return Path.lower();
Index: clang-tools-extra/clangd/TidyProvider.cpp
===
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -11,6 +11,7 @@
 #include "Config.h"
 #include "support/FileCache.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "support/ThreadsafeFS.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -102,23 +103,12 @@
 
 // Compute absolute paths to all ancestors (substrings of P.Path).
 // Ensure cache entries for each ancestor exist in the map.
-llvm::StringRef Parent = path::parent_path(AbsPath);
 llvm::SmallVector Caches;
 {
   std::lock_guard Lock(Mu);
-  for (auto I = path::rbegin(Parent), E = path::rend(Parent); I != E; ++I) {
-assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
-   "Canonical path components should be substrings");
-llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin());
-#ifdef _WIN32
-// C:\ is an ancestor, but skip its (relative!) parent C:.
-if (Ancestor.size() == 2 && Ancestor.back() == ':')
-  continue;
-#endif
-assert(path::is_absolute(Ancestor));
-
+  for (auto Ancestor = absoluteParent(AbsPath); !Ancestor.empty();
+   Ancestor = absoluteParent(Ancestor)) {
 auto It = Cache.find(Ancestor);
-
 // Assemble the actual config file path only if needed.
 if (It == Cache.end()) {
   llvm::SmallString<256> ConfigPath = Ancestor;
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -43,21 +43,6 @@
 namespace clangd {
 namespace {
 
-// Variant of parent_path that operates only on absolute paths.
-PathRef absoluteParent(PathRef Path) {
-  assert(llvm::sys::path::is_absolute(Path));
-#if defined(_WIN32)
-  // llvm::sys says "C:\" is absolute, and its parent is "C:" which is relative.
-  // This unhelpful behavior seems to have been inherited from boost.
-  if (llvm::sys::path::relative_path(Path).empty()) {
-return PathRef();
-  }
-#endif
-  PathRef Result = llvm::sys::path::parent_path(Path);
-  assert(Result.empty() || llvm::sys::path::is_absolute(Result));
-  return Result;
-}
-
 // Runs the given action on all parent directories of filename, starting from
 // deepest directory and going up to root. Stops whenever action succeeds.
 void actOnAllParentDirectories(PathRef FileName,
Index: clang-tools-extra/clangd/ConfigProvider.cpp
===
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -10,8 +10,10 @@
 #include "Config.h"
 #include 

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 324947.
kadircet added a comment.

- Get rid of parent_path usage for real


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96123

Files:
  clang-tools-extra/clangd/ConfigProvider.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h

Index: clang-tools-extra/clangd/support/Path.h
===
--- clang-tools-extra/clangd/support/Path.h
+++ clang-tools-extra/clangd/support/Path.h
@@ -28,6 +28,9 @@
 std::string maybeCaseFoldPath(PathRef Path);
 bool pathEqual(PathRef, PathRef);
 
+/// Variant of parent_path that operates only on absolute paths.
+/// Unlike parent_path doesn't consider C: a parent of C:\.
+PathRef absoluteParent(PathRef Path);
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/support/Path.cpp
===
--- clang-tools-extra/clangd/support/Path.cpp
+++ clang-tools-extra/clangd/support/Path.cpp
@@ -7,9 +7,25 @@
 //===--===//
 
 #include "support/Path.h"
+#include "llvm/Support/Path.h"
+
 namespace clang {
 namespace clangd {
 
+PathRef absoluteParent(PathRef Path) {
+  assert(llvm::sys::path::is_absolute(Path));
+#if defined(_WIN32)
+  // llvm::sys says "C:\" is absolute, and its parent is "C:" which is relative.
+  // This unhelpful behavior seems to have been inherited from boost.
+  if (llvm::sys::path::relative_path(Path).empty()) {
+return PathRef();
+  }
+#endif
+  PathRef Result = llvm::sys::path::parent_path(Path);
+  assert(Result.empty() || llvm::sys::path::is_absolute(Result));
+  return Result;
+}
+
 std::string maybeCaseFoldPath(PathRef Path) {
 #if defined(_WIN32) || defined(__APPLE__)
   return Path.lower();
Index: clang-tools-extra/clangd/TidyProvider.cpp
===
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -11,6 +11,7 @@
 #include "Config.h"
 #include "support/FileCache.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "support/ThreadsafeFS.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -102,23 +103,12 @@
 
 // Compute absolute paths to all ancestors (substrings of P.Path).
 // Ensure cache entries for each ancestor exist in the map.
-llvm::StringRef Parent = path::parent_path(AbsPath);
 llvm::SmallVector Caches;
 {
   std::lock_guard Lock(Mu);
-  for (auto I = path::rbegin(Parent), E = path::rend(Parent); I != E; ++I) {
-assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
-   "Canonical path components should be substrings");
-llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin());
-#ifdef _WIN32
-// C:\ is an ancestor, but skip its (relative!) parent C:.
-if (Ancestor.size() == 2 && Ancestor.back() == ':')
-  continue;
-#endif
-assert(path::is_absolute(Ancestor));
-
+  for (auto Ancestor = absoluteParent(AbsPath); !Ancestor.empty();
+   Ancestor = absoluteParent(Ancestor)) {
 auto It = Cache.find(Ancestor);
-
 // Assemble the actual config file path only if needed.
 if (It == Cache.end()) {
   llvm::SmallString<256> ConfigPath = Ancestor;
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -43,21 +43,6 @@
 namespace clangd {
 namespace {
 
-// Variant of parent_path that operates only on absolute paths.
-PathRef absoluteParent(PathRef Path) {
-  assert(llvm::sys::path::is_absolute(Path));
-#if defined(_WIN32)
-  // llvm::sys says "C:\" is absolute, and its parent is "C:" which is relative.
-  // This unhelpful behavior seems to have been inherited from boost.
-  if (llvm::sys::path::relative_path(Path).empty()) {
-return PathRef();
-  }
-#endif
-  PathRef Result = llvm::sys::path::parent_path(Path);
-  assert(Result.empty() || llvm::sys::path::is_absolute(Result));
-  return Result;
-}
-
 // Runs the given action on all parent directories of filename, starting from
 // deepest directory and going up to root. Stops whenever action succeeds.
 void actOnAllParentDirectories(PathRef FileName,
Index: clang-tools-extra/clangd/ConfigProvider.cpp
===
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -10,8 +10,10 @@
 #include "Config.h"
 #include "ConfigFragment.h"
 #include "support/FileCache.h"
+#in

[PATCH] D96906: [AMDGPU] gfx90a support

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199
+  MCRegister RepReg;
+  for (MCRegister R : *MRI->getRegClass(Reg)) {
+if (!MRI->isReserved(R)) {
+  RepReg = R;
+  break;
+}
+  }

This is a problem because I've removed forAllLanes.

This is a hack, we should be using a different register class for cases that 
don't support a given subregister index not scanning for an example 
non-reserved register


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:191-199
+  MCRegister RepReg;
+  for (MCRegister R : *MRI->getRegClass(Reg)) {
+if (!MRI->isReserved(R)) {
+  RepReg = R;
+  break;
+}
+  }

arsenm wrote:
> This is a problem because I've removed forAllLanes.
> 
> This is a hack, we should be using a different register class for cases that 
> don't support a given subregister index not scanning for an example 
> non-reserved register
This would be massive duplication of all instructions with such operands, isn't 
it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added a comment.

> The point is that nobody upstream even got a chance to chime in.

We are and will be taking care of any feedback provided in this review 
post-commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Mark Searles via Phabricator via cfe-commits]

msearles added a comment.

In D96906#2572749 , @kzhuravl wrote:

>> The point is that nobody upstream even got a chance to chime in.
>
> We are and will be taking care of any feedback provided in this review 
> post-commit.

To be fair to @rampitec , it was not his desire to push this up in 1 big patch. 
We needed this upstreamed and no time was given to him to break it up into 
reasonably sized pieces. If it appears to be his doing/his intent, well, it 
should not. There have been a couple comments; I believe most addressed; 
comments will continue to be addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D96906#2572842 , @msearles wrote:

> In D96906#2572749 , @kzhuravl wrote:
>
>>> The point is that nobody upstream even got a chance to chime in.
>>
>> We are and will be taking care of any feedback provided in this review 
>> post-commit.
>
> To be fair to @rampitec , it was not his desire to push this up in 1 big 
> patch. We needed this upstreamed and no time was given to him to break it up 
> into reasonably sized pieces. If it appears to be his doing/his intent, well, 
> it should not. There have been a couple comments; I believe most addressed; 
> comments will continue to be addressed.

Who landed the change is not particularly important.

What does matter is to make sure that shortcutting the review does not become a 
regular occurrence.

Stuff happens, Sometimes the standard rules may need to be bent. However, it 
should not be a unilateral decision by the committing side.

A better way to handle that would be to send the patch for review few days 
early (you presumably did have most/all of these changes made by then), provide 
details describing the changes (single subject line falls a bit short), outline 
your situation explaining why the patch can't be split and reviewed pre-commit. 
If the changes are indeed well-reviewed downstream, that would probably not 
pose much of a challenge to get the patch approved.  If the patch does need 
further cleanups, at least we would have a reasonable idea how invasive they 
would be and could make an informed call. "Commit now, ask for forgiveness 
later" is not among the LLVM contribution guidelines, not for large patches. At 
the very minimum it should've been publicly discussed before the fact.




Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+

We have `BoolFOption` to generate `-fsomething` and `-fno-something`



Comment at: llvm/include/llvm/BinaryFormat/ELF.h:737
   EF_AMDGPU_MACH_AMDGCN_FIRST = EF_AMDGPU_MACH_AMDGCN_GFX600,
-  EF_AMDGPU_MACH_AMDGCN_LAST = EF_AMDGPU_MACH_AMDGCN_GFX805,
+  EF_AMDGPU_MACH_AMDGCN_LAST = EF_AMDGPU_MACH_AMDGCN_GFX90A,
 

Nit: This looks odd. GFX90A does not need to be in the middle of the list. It 
makes it somewhat confusing to tell which ID is really the last. The `_LAST` 
enum says it's GFX90A, but it's not the last item of the list. There are 
already out-of-name-order GPUs at the end of the list, so putting 90A at the 
end would probably be a better choice. At least we'd still have the numeric 
values in order. Right now the list is ordered neither by the name nor by the 
value.

There's also a question of whether something needs to be done about the missing 
values 0x3c..0x3e. Presumably the `_FIRST`..`_LAST` enums specify the range 
we'll use to iterate over the GPU IDs. Do we handle the missing values 
correctly?

Looks like it's benign at the moment as we're only using it to return amdgcn 
triple in ELFObjectFile.h. I'd add the placeholder enums for the 
reserved/unused values within the range.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

In D96906#2572842 , @msearles wrote:

> In D96906#2572749 , @kzhuravl wrote:
>
>>> The point is that nobody upstream even got a chance to chime in.
>>
>> We are and will be taking care of any feedback provided in this review 
>> post-commit.
>
> To be fair to @rampitec , it was not his desire to push this up in 1 big 
> patch. We needed this upstreamed and no time was given to him to break it up 
> into reasonably sized pieces. If it appears to be his doing/his intent, well, 
> it should not. There have been a couple comments; I believe most addressed; 
> comments will continue to be addressed.

"we needed this upstream" is a business issue on AMD's side, not an issue for 
the llvm project. In general the expectation is that code is reviewed according 
to the guidelines and a single reviewer with one (small) patch that wasn't a 
revert doesn't feel like sufficient review for something of this size. For 
something this size I'd have expected Matt to at least be on the reviewer line 
and that also wasn't done. This feels like an abuse of the review system and 
probably should be reverted.

Thanks.

-eric


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added inline comments.



Comment at: llvm/include/llvm/BinaryFormat/ELF.h:737
   EF_AMDGPU_MACH_AMDGCN_FIRST = EF_AMDGPU_MACH_AMDGCN_GFX600,
-  EF_AMDGPU_MACH_AMDGCN_LAST = EF_AMDGPU_MACH_AMDGCN_GFX805,
+  EF_AMDGPU_MACH_AMDGCN_LAST = EF_AMDGPU_MACH_AMDGCN_GFX90A,
 

tra wrote:
> Nit: This looks odd. GFX90A does not need to be in the middle of the list. It 
> makes it somewhat confusing to tell which ID is really the last. The `_LAST` 
> enum says it's GFX90A, but it's not the last item of the list. There are 
> already out-of-name-order GPUs at the end of the list, so putting 90A at the 
> end would probably be a better choice. At least we'd still have the numeric 
> values in order. Right now the list is ordered neither by the name nor by the 
> value.
> 
> There's also a question of whether something needs to be done about the 
> missing values 0x3c..0x3e. Presumably the `_FIRST`..`_LAST` enums specify the 
> range we'll use to iterate over the GPU IDs. Do we handle the missing values 
> correctly?
> 
> Looks like it's benign at the moment as we're only using it to return amdgcn 
> triple in ELFObjectFile.h. I'd add the placeholder enums for the 
> reserved/unused values within the range.
> 
https://reviews.llvm.org/D97010


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+

tra wrote:
> We have `BoolFOption` to generate `-fsomething` and `-fno-something`
The reason we use m_amdgpu_Features_Group is it is getting transformed to 
target features (-mtgsplit to +tgsplit, mno-tgsplit to -tgsplit. For example, 
tgsplit target feature in AMDGPU backend:

https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPU.td#L157

Does BoolFOption get translated to target features as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+

kzhuravl wrote:
> tra wrote:
> > We have `BoolFOption` to generate `-fsomething` and `-fno-something`
> The reason we use m_amdgpu_Features_Group is it is getting transformed to 
> target features (-mtgsplit to +tgsplit, mno-tgsplit to -tgsplit. For example, 
> tgsplit target feature in AMDGPU backend:
> 
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPU.td#L157
> 
> Does BoolFOption get translated to target features as well?
We could probably create similar BoolMOption. This is not only tgsplit, there 
are plenty of such binary options around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+

kzhuravl wrote:
> tra wrote:
> > We have `BoolFOption` to generate `-fsomething` and `-fno-something`
> The reason we use m_amdgpu_Features_Group is it is getting transformed to 
> target features (-mtgsplit to +tgsplit, mno-tgsplit to -tgsplit. For example, 
> tgsplit target feature in AMDGPU backend:
> 
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPU.td#L157
> 
> Does BoolFOption get translated to target features as well?
Quickly glancing over Options.td, BoolFOption is in f_group, and does not get 
automatically converted to target-features


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+

kzhuravl wrote:
> rampitec wrote:
> > kzhuravl wrote:
> > > kzhuravl wrote:
> > > > tra wrote:
> > > > > We have `BoolFOption` to generate `-fsomething` and `-fno-something`
> > > > The reason we use m_amdgpu_Features_Group is it is getting transformed 
> > > > to target features (-mtgsplit to +tgsplit, mno-tgsplit to -tgsplit. For 
> > > > example, tgsplit target feature in AMDGPU backend:
> > > > 
> > > > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPU.td#L157
> > > > 
> > > > Does BoolFOption get translated to target features as well?
> > > Quickly glancing over Options.td, BoolFOption is in f_group, and does not 
> > > get automatically converted to target-features
> > We could probably create similar BoolMOption. This is not only tgsplit, 
> > there are plenty of such binary options around.
> agreed, seems like a good choice given there is BoolFOption, BoolGOption
but it will still need to be in m_amdgpu_Features_Group because 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/AMDGPU.cpp#L403
 unless we want to switch away from that. so maybe make a group an optional, 
last template parameter to BoolMOption?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+

kzhuravl wrote:
> kzhuravl wrote:
> > rampitec wrote:
> > > kzhuravl wrote:
> > > > kzhuravl wrote:
> > > > > tra wrote:
> > > > > > We have `BoolFOption` to generate `-fsomething` and `-fno-something`
> > > > > The reason we use m_amdgpu_Features_Group is it is getting 
> > > > > transformed to target features (-mtgsplit to +tgsplit, mno-tgsplit to 
> > > > > -tgsplit. For example, tgsplit target feature in AMDGPU backend:
> > > > > 
> > > > > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPU.td#L157
> > > > > 
> > > > > Does BoolFOption get translated to target features as well?
> > > > Quickly glancing over Options.td, BoolFOption is in f_group, and does 
> > > > not get automatically converted to target-features
> > > We could probably create similar BoolMOption. This is not only tgsplit, 
> > > there are plenty of such binary options around.
> > agreed, seems like a good choice given there is BoolFOption, BoolGOption
> but it will still need to be in m_amdgpu_Features_Group because 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/AMDGPU.cpp#L403
>  unless we want to switch away from that. so maybe make a group an optional, 
> last template parameter to BoolMOption?
Other targets also have its own corresponding groups: 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Driver/Options.td#L151


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+

rampitec wrote:
> kzhuravl wrote:
> > kzhuravl wrote:
> > > tra wrote:
> > > > We have `BoolFOption` to generate `-fsomething` and `-fno-something`
> > > The reason we use m_amdgpu_Features_Group is it is getting transformed to 
> > > target features (-mtgsplit to +tgsplit, mno-tgsplit to -tgsplit. For 
> > > example, tgsplit target feature in AMDGPU backend:
> > > 
> > > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPU.td#L157
> > > 
> > > Does BoolFOption get translated to target features as well?
> > Quickly glancing over Options.td, BoolFOption is in f_group, and does not 
> > get automatically converted to target-features
> We could probably create similar BoolMOption. This is not only tgsplit, there 
> are plenty of such binary options around.
agreed, seems like a good choice given there is BoolFOption, BoolGOption


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+

rampitec wrote:
> kzhuravl wrote:
> > kzhuravl wrote:
> > > kzhuravl wrote:
> > > > rampitec wrote:
> > > > > kzhuravl wrote:
> > > > > > kzhuravl wrote:
> > > > > > > tra wrote:
> > > > > > > > We have `BoolFOption` to generate `-fsomething` and 
> > > > > > > > `-fno-something`
> > > > > > > The reason we use m_amdgpu_Features_Group is it is getting 
> > > > > > > transformed to target features (-mtgsplit to +tgsplit, 
> > > > > > > mno-tgsplit to -tgsplit. For example, tgsplit target feature in 
> > > > > > > AMDGPU backend:
> > > > > > > 
> > > > > > > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPU.td#L157
> > > > > > > 
> > > > > > > Does BoolFOption get translated to target features as well?
> > > > > > Quickly glancing over Options.td, BoolFOption is in f_group, and 
> > > > > > does not get automatically converted to target-features
> > > > > We could probably create similar BoolMOption. This is not only 
> > > > > tgsplit, there are plenty of such binary options around.
> > > > agreed, seems like a good choice given there is BoolFOption, BoolGOption
> > > but it will still need to be in m_amdgpu_Features_Group because 
> > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/AMDGPU.cpp#L403
> > >  unless we want to switch away from that. so maybe make a group an 
> > > optional, last template parameter to BoolMOption?
> > Other targets also have its own corresponding groups: 
> > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Driver/Options.td#L151
> Looks like. It is not just the same thing as BoolFOption or BoolGOption.
@tra , do you have any suggestion based on the comments above? thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Jay Foad via Phabricator via cfe-commits]

foad added a comment.

In D96906#2573265 , @echristo wrote:

> In D96906#2572842 , @msearles wrote:
>
>> In D96906#2572749 , @kzhuravl wrote:
>>
 The point is that nobody upstream even got a chance to chime in.
>>>
>>> We are and will be taking care of any feedback provided in this review 
>>> post-commit.
>>
>> To be fair to @rampitec , it was not his desire to push this up in 1 big 
>> patch. We needed this upstreamed and no time was given to him to break it up 
>> into reasonably sized pieces. If it appears to be his doing/his intent, 
>> well, it should not. There have been a couple comments; I believe most 
>> addressed; comments will continue to be addressed.
>
> "we needed this upstream" is a business issue on AMD's side, not an issue for 
> the llvm project. In general the expectation is that code is reviewed 
> according to the guidelines and a single reviewer with one (small) patch that 
> wasn't a revert doesn't feel like sufficient review for something of this 
> size. For something this size I'd have expected Matt to at least be on the 
> reviewer line and that also wasn't done. This feels like an abuse of the 
> review system and probably should be reverted.
>
> Thanks.
>
> -eric

I'd appreciate it if you could find a solution that does not involve reverting 
and reapplying later, as this will triple the amount of churn we get 
downstream. (I realise LLVM policy is not to care about downstream but I 
thought I'd plead my case anyway!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96906: [AMDGPU] gfx90a support

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+

kzhuravl wrote:
> kzhuravl wrote:
> > kzhuravl wrote:
> > > rampitec wrote:
> > > > kzhuravl wrote:
> > > > > kzhuravl wrote:
> > > > > > tra wrote:
> > > > > > > We have `BoolFOption` to generate `-fsomething` and 
> > > > > > > `-fno-something`
> > > > > > The reason we use m_amdgpu_Features_Group is it is getting 
> > > > > > transformed to target features (-mtgsplit to +tgsplit, mno-tgsplit 
> > > > > > to -tgsplit. For example, tgsplit target feature in AMDGPU backend:
> > > > > > 
> > > > > > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPU.td#L157
> > > > > > 
> > > > > > Does BoolFOption get translated to target features as well?
> > > > > Quickly glancing over Options.td, BoolFOption is in f_group, and does 
> > > > > not get automatically converted to target-features
> > > > We could probably create similar BoolMOption. This is not only tgsplit, 
> > > > there are plenty of such binary options around.
> > > agreed, seems like a good choice given there is BoolFOption, BoolGOption
> > but it will still need to be in m_amdgpu_Features_Group because 
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/AMDGPU.cpp#L403
> >  unless we want to switch away from that. so maybe make a group an 
> > optional, last template parameter to BoolMOption?
> Other targets also have its own corresponding groups: 
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Driver/Options.td#L151
Looks like. It is not just the same thing as BoolFOption or BoolGOption.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[clang-tools-extra] 6329ce7 - [clangd] Expose absoluteParent helper

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-02-19T13:40:21+01:00
New Revision: 6329ce75da7a44c40d4406bf48e973ef5fdb

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

LOG: [clangd] Expose absoluteParent helper

Will be used in other components that need ancestor traversal.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/ConfigProvider.cpp
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/TidyProvider.cpp
clang-tools-extra/clangd/support/Path.cpp
clang-tools-extra/clangd/support/Path.h

Removed: 




diff  --git a/clang-tools-extra/clangd/ConfigProvider.cpp 
b/clang-tools-extra/clangd/ConfigProvider.cpp
index 05b2ba50566d..3b705d8f4a82 100644
--- a/clang-tools-extra/clangd/ConfigProvider.cpp
+++ b/clang-tools-extra/clangd/ConfigProvider.cpp
@@ -10,8 +10,10 @@
 #include "Config.h"
 #include "ConfigFragment.h"
 #include "support/FileCache.h"
+#include "support/Path.h"
 #include "support/ThreadsafeFS.h"
 #include "support/Trace.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -96,16 +98,10 @@ Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef 
RelPath,
 return {};
 
   // Compute absolute paths to all ancestors (substrings of P.Path).
-  llvm::StringRef Parent = path::parent_path(P.Path);
   llvm::SmallVector Ancestors;
-  for (auto I = path::begin(Parent, path::Style::posix),
-E = path::end(Parent);
-   I != E; ++I) {
-// Avoid weird non-substring cases like phantom "." components.
-// In practice, Component is a substring for all "normal" ancestors.
-if (I->end() < Parent.begin() || I->end() > Parent.end())
-  continue;
-Ancestors.emplace_back(Parent.begin(), I->end() - Parent.begin());
+  for (auto Ancestor = absoluteParent(P.Path); !Ancestor.empty();
+   Ancestor = absoluteParent(Ancestor)) {
+Ancestors.emplace_back(Ancestor);
   }
   // Ensure corresponding cache entries exist in the map.
   llvm::SmallVector Caches;
@@ -127,7 +123,7 @@ Provider::fromAncestorRelativeYAMLFiles(llvm::StringRef 
RelPath,
   // Finally query each individual file.
   // This will take a (per-file) lock for each file that actually exists.
   std::vector Result;
-  for (FileConfigCache *Cache : Caches)
+  for (FileConfigCache *Cache : llvm::reverse(Caches))
 Cache->get(FS, DC, P.FreshTime, Result);
   return Result;
 };

diff  --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp 
b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index a38c8a57d161..d830190bfdfd 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -43,21 +43,6 @@ namespace clang {
 namespace clangd {
 namespace {
 
-// Variant of parent_path that operates only on absolute paths.
-PathRef absoluteParent(PathRef Path) {
-  assert(llvm::sys::path::is_absolute(Path));
-#if defined(_WIN32)
-  // llvm::sys says "C:\" is absolute, and its parent is "C:" which is 
relative.
-  // This unhelpful behavior seems to have been inherited from boost.
-  if (llvm::sys::path::relative_path(Path).empty()) {
-return PathRef();
-  }
-#endif
-  PathRef Result = llvm::sys::path::parent_path(Path);
-  assert(Result.empty() || llvm::sys::path::is_absolute(Result));
-  return Result;
-}
-
 // Runs the given action on all parent directories of filename, starting from
 // deepest directory and going up to root. Stops whenever action succeeds.
 void actOnAllParentDirectories(PathRef FileName,

diff  --git a/clang-tools-extra/clangd/TidyProvider.cpp 
b/clang-tools-extra/clangd/TidyProvider.cpp
index bcf1cd5a6183..7573645eeefd 100644
--- a/clang-tools-extra/clangd/TidyProvider.cpp
+++ b/clang-tools-extra/clangd/TidyProvider.cpp
@@ -11,6 +11,7 @@
 #include "Config.h"
 #include "support/FileCache.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "support/ThreadsafeFS.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -102,23 +103,12 @@ class DotClangTidyTree {
 
 // Compute absolute paths to all ancestors (substrings of P.Path).
 // Ensure cache entries for each ancestor exist in the map.
-llvm::StringRef Parent = path::parent_path(AbsPath);
 llvm::SmallVector Caches;
 {
   std::lock_guard Lock(Mu);
-  for (auto I = path::rbegin(Parent), E = path::rend(Parent); I != E; ++I) 
{
-assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
-   "Canonical path components should be substrings");
-llvm::StringR

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6329ce75da7a: [clangd] Expose absoluteParent helper 
(authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D96123?vs=324947&id=324948#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96123

Files:
  clang-tools-extra/clangd/ConfigProvider.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h

Index: clang-tools-extra/clangd/support/Path.h
===
--- clang-tools-extra/clangd/support/Path.h
+++ clang-tools-extra/clangd/support/Path.h
@@ -40,6 +40,10 @@
 bool pathStartsWith(
 PathRef Ancestor, PathRef Path,
 llvm::sys::path::Style Style = llvm::sys::path::Style::native);
+
+/// Variant of parent_path that operates only on absolute paths.
+/// Unlike parent_path doesn't consider C: a parent of C:\.
+PathRef absoluteParent(PathRef Path);
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/support/Path.cpp
===
--- clang-tools-extra/clangd/support/Path.cpp
+++ clang-tools-extra/clangd/support/Path.cpp
@@ -19,6 +19,20 @@
 bool pathEqual(PathRef A, PathRef B) { return A == B; }
 #endif // CLANGD_PATH_CASE_INSENSITIVE
 
+PathRef absoluteParent(PathRef Path) {
+  assert(llvm::sys::path::is_absolute(Path));
+#if defined(_WIN32)
+  // llvm::sys says "C:\" is absolute, and its parent is "C:" which is relative.
+  // This unhelpful behavior seems to have been inherited from boost.
+  if (llvm::sys::path::relative_path(Path).empty()) {
+return PathRef();
+  }
+#endif
+  PathRef Result = llvm::sys::path::parent_path(Path);
+  assert(Result.empty() || llvm::sys::path::is_absolute(Result));
+  return Result;
+}
+
 bool pathStartsWith(PathRef Ancestor, PathRef Path,
 llvm::sys::path::Style Style) {
   assert(llvm::sys::path::is_absolute(Ancestor) &&
Index: clang-tools-extra/clangd/TidyProvider.cpp
===
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -11,6 +11,7 @@
 #include "Config.h"
 #include "support/FileCache.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "support/ThreadsafeFS.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -102,23 +103,12 @@
 
 // Compute absolute paths to all ancestors (substrings of P.Path).
 // Ensure cache entries for each ancestor exist in the map.
-llvm::StringRef Parent = path::parent_path(AbsPath);
 llvm::SmallVector Caches;
 {
   std::lock_guard Lock(Mu);
-  for (auto I = path::rbegin(Parent), E = path::rend(Parent); I != E; ++I) {
-assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
-   "Canonical path components should be substrings");
-llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin());
-#ifdef _WIN32
-// C:\ is an ancestor, but skip its (relative!) parent C:.
-if (Ancestor.size() == 2 && Ancestor.back() == ':')
-  continue;
-#endif
-assert(path::is_absolute(Ancestor));
-
+  for (auto Ancestor = absoluteParent(AbsPath); !Ancestor.empty();
+   Ancestor = absoluteParent(Ancestor)) {
 auto It = Cache.find(Ancestor);
-
 // Assemble the actual config file path only if needed.
 if (It == Cache.end()) {
   llvm::SmallString<256> ConfigPath = Ancestor;
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -43,21 +43,6 @@
 namespace clangd {
 namespace {
 
-// Variant of parent_path that operates only on absolute paths.
-PathRef absoluteParent(PathRef Path) {
-  assert(llvm::sys::path::is_absolute(Path));
-#if defined(_WIN32)
-  // llvm::sys says "C:\" is absolute, and its parent is "C:" which is relative.
-  // This unhelpful behavior seems to have been inherited from boost.
-  if (llvm::sys::path::relative_path(Path).empty()) {
-return PathRef();
-  }
-#endif
-  PathRef Result = llvm::sys::path::parent_path(Path);
-  assert(Result.empty() || llvm::sys::path::is_absolute(Result));
-  return Result;
-}
-
 // Runs the given action on all parent directories of filename, starting from
 // deepest directory and going up to root. Stops whenever action succeeds.
 void actOnAllParentDirectories(PathRef FileName,
Index: clang-tools-extra/clangd/ConfigProvider.cpp
===

[PATCH] D96875: [flang][driver] Add -fdebug-module-writer option

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 updated this revision to Diff 324949.
arnamoy10 added a comment.

Set the option as a frontend-only option, also modified the test case 
accordingly.


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

https://reviews.llvm.org/D96875

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Semantics/mod-file-rewriter.f90

Index: flang/test/Semantics/mod-file-rewriter.f90
===
--- flang/test/Semantics/mod-file-rewriter.f90
+++ flang/test/Semantics/mod-file-rewriter.f90
@@ -1,8 +1,8 @@
 ! RUN: rm -fr %t && mkdir %t && cd %t
-! RUN: %f18 -fsyntax-only -fdebug-module-writer %s 2>&1 | FileCheck %s --check-prefix CHECK_CHANGED
-! RUN: %f18 -fsyntax-only -fdebug-module-writer %s 2>&1 | FileCheck %s --check-prefix CHECK_UNCHANGED
-! RUN: %f18 -fsyntax-only -fdebug-module-writer %p/Inputs/mod-file-unchanged.f90 2>&1 | FileCheck %s --check-prefix CHECK_UNCHANGED
-! RUN: %f18 -fsyntax-only -fdebug-module-writer %p/Inputs/mod-file-changed.f90 2>&1 | FileCheck %s --check-prefix CHECK_CHANGED
+! RUN: %flang_fc1 -fsyntax-only -fdebug-module-writer %s 2>&1 | FileCheck %s --check-prefix CHECK_CHANGED
+! RUN: %flang_fc1 -fsyntax-only -fdebug-module-writer %s 2>&1 | FileCheck %s --check-prefix CHECK_UNCHANGED
+! RUN: %flang_fc1 -fsyntax-only -fdebug-module-writer %p/Inputs/mod-file-unchanged.f90 2>&1 | FileCheck %s --check-prefix CHECK_UNCHANGED
+! RUN: %flang_fc1 -fsyntax-only -fdebug-module-writer %p/Inputs/mod-file-changed.f90 2>&1 | FileCheck %s --check-prefix CHECK_CHANGED
 
 module m
   real :: x(10)
Index: flang/test/Flang-Driver/driver-help.f90
===
--- flang/test/Flang-Driver/driver-help.f90
+++ flang/test/Flang-Driver/driver-help.f90
@@ -59,6 +59,7 @@
 ! HELP-FC1-NEXT: -fdebug-dump-parse-tree Dump the parse tree
 ! HELP-FC1-NEXT: -fdebug-dump-provenance Dump provenance
 ! HELP-FC1-NEXT: -fdebug-dump-symbolsDump symbols after the semantic analysis
+! HELP-FC1-NEXT: -fdebug-module-writer   Enables showing debug messages while writing module files.
 ! HELP-FC1-NEXT: -fdebug-unparse-with-symbols
 ! HELP-FC1-NEXT:Unparse and stop.
 ! HELP-FC1-NEXT: -fdebug-unparseUnparse and stop.
Index: flang/lib/Frontend/FrontendActions.cpp
===
--- flang/lib/Frontend/FrontendActions.cpp
+++ flang/lib/Frontend/FrontendActions.cpp
@@ -116,7 +116,7 @@
   // Prepare semantics
   setSemantics(std::make_unique(
   ci.invocation().semanticsContext(), parseTree,
-  ci.parsing().cooked().AsCharBlock()));
+  ci.parsing().cooked().AsCharBlock(), ci.invocation().debugModuleDir()));
   auto &semantics = this->semantics();
 
   // Run semantic checks
Index: flang/lib/Frontend/CompilerInvocation.cpp
===
--- flang/lib/Frontend/CompilerInvocation.cpp
+++ flang/lib/Frontend/CompilerInvocation.cpp
@@ -292,9 +292,10 @@
 
 /// Parses all semantic related arguments and populates the variables
 /// options accordingly.
-static void parseSemaArgs(std::string &moduleDir, llvm::opt::ArgList &args,
+static void parseSemaArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
 clang::DiagnosticsEngine &diags) {
-
+  
+  // -J/module-dir option
   auto moduleDirList =
   args.getAllArgValues(clang::driver::options::OPT_module_dir);
   // User can only specify -J/-module-dir once
@@ -306,7 +307,12 @@
 diags.Report(diagID);
   }
   if (moduleDirList.size() == 1)
-moduleDir = moduleDirList[0];
+res.SetModuleDir(moduleDirList[0]);
+
+  // -fdebug-module-writer option
+  if (args.hasArg(clang::driver::options::OPT_fdebug_module_writer)) {
+res.SetDebugModuleDir(true);
+  }
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation &res,
@@ -339,7 +345,7 @@
   // Parse the preprocessor args
   parsePreprocessorArgs(res.preprocessorOpts(), args);
   // Parse semantic args
-  parseSemaArgs(res.moduleDir(), args, diags);
+  parseSemaArgs(res, args, diags);
 
   return success;
 }
@@ -451,7 +457,6 @@
   *(new Fortran::common::IntrinsicTypeDefaultKinds()),
   fortranOptions.features, allCookedSources);
 
-  auto &moduleDirJ = moduleDir();
-  semanticsContext_->set_moduleDirectory(moduleDirJ)
+  semanticsContext_->set_moduleDirectory(moduleDir())
   .set_searchDirectories(fortranOptions.searchDirectories);
 }
Index: flang/include/flang/Frontend/CompilerInvocation.h
===
--- flang/include/flang/Frontend/CompilerInvocation.h
+++ flang/include/flang/Frontend/CompilerInvocation.h
@@ -69,6 +69,9 @@
   // of options.
   std::string modul

[PATCH] D96944: [RecoveryAST] Add design doc to clang internal manual.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added inline comments.



Comment at: clang/docs/InternalsManual.rst:1882
+  example.
+- representing invalid node: the invalid node is preserved in the AST in some
+  form, e.g. when the "declaration" part of the declaration contains semantic

hokein wrote:
> kadircet wrote:
> > rather than `some form` maybe say `while indicating the error` ?
> I'm not sure this will be better. `indicating error` seems to be a bit strong.
> 
> e.g. `if () {}`, this ill-formed statement, the AST node  is like below, 
> which doesn't have an obvious error signal (we could argue the 
> `OpaqueValueExpr` is )
> 
> ```
> `-IfStmt
>|-OpaqueValueExpr 'bool'
>`-CompoundStmt 
> ```
oh okay didn't know that. i thought we would always have a node which can 
indicate the error somehow (not having that is still surprising though).



Comment at: clang/docs/InternalsManual.rst:1971
+considered value-dependent, because its value isn't well-defined until the 
error
+is resolved. Among other things, this means that clang doesn't emit more errors
+where a RecoveryExpr is used as a constant (e.g. array size), but also won't 
try

sammccall wrote:
> hokein wrote:
> > kadircet wrote:
> > > IIRC, there were other problems with clang trying to emit secondary diags 
> > > on such cases. It might be worth mentioning those too, again to warn 
> > > future travellers about the side effects of making this 
> > > non-value-dependent.
> > I think the existing `doesn't emit more errors` texts already indicate we 
> > suppress the secondary diags etc.
> Do you have an example? I chatted with Haojian offline about this and we 
> couldn't find a good one apart from constant contexts. (And the wording of 
> the standard suggests that value-dependence is a concept that's only 
> interesting in constant contexts)
> Do you have an example?

unfortunately not :( but something tells me the particular bug was somehow 
about typo correction happening inside recoveryexprs ?

> I think the existing doesn't emit more errors texts already indicate we 
> suppress the secondary diags etc.

yeah i guess you are right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96944

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

[PATCH] D96856: [clangd] Narrow and document a loophole in blockUntilIdle

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96856

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

[PATCH] D96847: [clang][cli] Store additional optimization remarks info

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 324956.
jansvoboda11 added a comment.

Put `RemarkKind` in `RemarkPattern`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96847

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1162,20 +1162,70 @@
<< "a filename";
 }
 
-/// Create a new Regex instance out of the string value in \p RpassArg.
-/// It returns the string and a pointer to the newly generated Regex instance.
-static CodeGenOptions::RemarkPattern
-GenerateOptimizationRemarkRegex(DiagnosticsEngine &Diags, ArgList &Args,
-Arg *RpassArg) {
-  StringRef Val = RpassArg->getValue();
-  std::string RegexError;
-  std::shared_ptr Pattern = std::make_shared(Val);
-  if (!Pattern->isValid(RegexError)) {
-Diags.Report(diag::err_drv_optimization_remark_pattern)
-<< RegexError << RpassArg->getAsString(Args);
-Pattern.reset();
-  }
-  return {std::string(Val), Pattern};
+/// Generate a remark argument. This is an inverse of `ParseOptimizationRemark`.
+static void
+GenerateOptimizationRemark(SmallVectorImpl &Args,
+   CompilerInvocation::StringAllocator SA,
+   OptSpecifier OptEQ, StringRef Name,
+   const CodeGenOptions::RemarkPattern &Pattern) {
+  if (Pattern) {
+GenerateArg(Args, OptEQ, Pattern.Value, SA);
+  } else if (Pattern.Kind == CodeGenOptions::RK_Enabled) {
+GenerateArg(Args, OPT_R_Joined, Name, SA);
+  } else if (Pattern.Kind == CodeGenOptions::RK_Disabled) {
+GenerateArg(Args, OPT_R_Joined, StringRef("no-") + Name, SA);
+  }
+};
+
+/// Parse a remark command line argument. It may be missing, disabled via
+/// '-Rno-group', enabled via '-Rgroup' or specified with a regular expression
+/// via '-Rgroup=regexp'. On top of that, it can be disabled/enabled globally by
+/// '-R[no-]everything'. The \p Kind and \p Pattern are initialized accordingly.
+static void ParseOptimizationRemark(DiagnosticsEngine &Diags, ArgList &Args,
+OptSpecifier OptEQ, StringRef Name,
+CodeGenOptions::RemarkPattern &Pattern) {
+  CodeGenOptions::RemarkPattern Result{CodeGenOptions::RK_Missing, "", nullptr};
+
+  auto InitializePattern =
+  [&Diags, &Args](const Arg *A, CodeGenOptions::RemarkPattern &Result) {
+Result.Value = A->getValue();
+
+std::string RegexError;
+Result.Regex = std::make_shared(Result.Value);
+if (!Result.Regex->isValid(RegexError)) {
+  Diags.Report(diag::err_drv_optimization_remark_pattern)
+  << RegexError << A->getAsString(Args);
+  return false;
+}
+
+return true;
+  };
+
+  for (Arg *A : Args) {
+if (A->getOption().matches(OPT_R_Joined)) {
+  StringRef Value = A->getValue();
+
+  if (Value == Name)
+Result.Kind = CodeGenOptions::RK_Enabled;
+  else if (Value == "everything")
+Result.Kind = CodeGenOptions::RK_EnabledEverything;
+  else if (Value.split('-') == std::make_pair(StringRef("no"), Name))
+Result.Kind = CodeGenOptions::RK_Disabled;
+  else if (Value == "no-everything")
+Result.Kind = CodeGenOptions::RK_DisabledEverything;
+} else if (A->getOption().matches(OptEQ)) {
+  Result.Kind = CodeGenOptions::RK_WithPattern;
+  if (!InitializePattern(A, Result))
+return;
+}
+  }
+
+  if (Result.Kind == CodeGenOptions::RK_Disabled ||
+  Result.Kind == CodeGenOptions::RK_DisabledEverything) {
+Result.Value = "";
+Result.Regex = nullptr;
+  }
+  Pattern = Result;
 }
 
 static bool parseDiagnosticLevelMask(StringRef FlagName,
@@ -1483,16 +1533,14 @@
   if (!Opts.OptRecordFormat.empty())
 GenerateArg(Args, OPT_opt_record_format, Opts.OptRecordFormat, SA);
 
-  if (Opts.OptimizationRemarkPattern)
-GenerateArg(Args, OPT_Rpass_EQ, Opts.OptimizationRemarkPattern.Pattern, SA);
+  GenerateOptimizationRemark(Args, SA, OPT_Rpass_EQ, "pass",
+ Opts.OptimizationRemarkPattern);
 
-  if (Opts.OptimizationRemarkMissedPattern)
-GenerateArg(Args, OPT_Rpass_missed_EQ,
-Opts.OptimizationRemarkMissedPattern.Pattern, SA);
+  GenerateOptimizationRemark(Args, SA, OPT_Rpass_missed_EQ, "pass-missed",
+ Opts.OptimizationRemarkMissedPattern);
 
-  if (Opts.OptimizationRemarkAnalysisPattern)
-GenerateArg(Args, OPT_Rpass_analysis_EQ,
-Opts.OptimizationRemarkAnalysisPattern.Pattern, SA);
+  GenerateOptimizationRemark(Args, SA, OPT_Rpass_analysis_EQ, "pass-analysis",

[PATCH] D96616: [OpenCL][Docs] Change decription for the OpenCL standard headers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/docs/UsersManual.rst:3016
 
.. code-block:: console
 

svenvh wrote:
> Anastasia wrote:
> > svenvh wrote:
> > > I wonder if we need to keep this example?  You're not referring to it 
> > > from the text and it's not demonstrating any special flags anymore.
> > it's still demonstrating the use of default clang driver invocation i.e. 
> > there are no flags needed to compile the code that uses BIFs.
> Okay, if you want to make that explicit then I would suggest to add a line 
> like:
> ```The following example shows how an OpenCL program that calls various 
> standard builtin functions can be compiled without the need for any explicit 
> includes or compiler flags.```
Ok, good idea. I will add this in my final commit.


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

https://reviews.llvm.org/D96616

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

[PATCH] D96905: [libclang] Add missing fixed point literal CursorKind to cindex.py

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

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

LGTM.

In D96905#2574322 , @vabridgers wrote:

> Polite ping. This is a small and simple change, my only doubt is the test 
> coverage. But frankly I see none of the literals are covered in any of the 
> test cases (unless I'm missing something). Could I get some guidance on 
> required test coverage for this simple change, or a LGTM?

I think if you really wish to test this, I think you should do something 
similar to `c-index-getCursor-pp`.

You could call it like `c-index-getCursor-fixed-point.c`:

  long _Fract longfractliteral = 0.lr;
  unsigned _Accum uliteral1 = 2.5uk;
  _Accum literal8 = -2.5k;
  
  // RUN: c-index-test -cursor-at=%s:1:1 -ffixed-point %s | FileCheck 
-check-prefix=CHECK-1 %s
  // CHECK-1: 1:13 VarDecl=longfractliteral:1:13 (Definition) Extent=[1:1 - 
1:44] Spelling=longfractliteral ([1:13 - 1:29])
  // CHECK-1-NEXT: Completion string: {ResultType long _Fract}{TypedText 
longfractliteral}
  
  // RUN: c-index-test -cursor-at=%s:2:1 -ffixed-point %s | FileCheck 
-check-prefix=CHECK-2 %s
  // CHECK-2: 2:17 VarDecl=uliteral1:2:17 (Definition) Extent=[2:1 - 2:34] 
Spelling=uliteral1 ([2:17 - 2:26])
  // CHECK-2-NEXT: Completion string: {ResultType unsigned _Accum}{TypedText 
uliteral1}
  
  // RUN: c-index-test -cursor-at=%s:3:1 -ffixed-point %s | FileCheck 
-check-prefix=CHECK-3 %s
  // CHECK-3: 3:8 VarDecl=literal8:3:8 (Definition) Extent=[3:1 - 3:24] 
Spelling=literal8 ([3:8 - 3:16])
  // CHECK-3-NEXT: Completion string: {ResultType _Accum}{TypedText literal8}

But I haven't used this binding stuff ever. Take my comments with a pinch of 
salt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96905

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

[PATCH] D96847: [clang][cli] Store additional optimization remarks info

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

That's a bit nicer.

Not sure if `RemarkPattern` is a good name now, as it may represent an 
optimization remark that doesn't have any pattern associated with it.
How about calling it `OptimizationRemark` and merging `operator bool` and 
`operator ->` into `bool patternMatch(...) { return Pattern && 
Pattern.match(...); }`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96847

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

[PATCH] D96905: [libclang] Add missing fixed point literal CursorKind to cindex.py

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

steakhal added a comment.

In D96905#2574398 , @steakhal wrote:

> You could call it like `c-index-getCursor-fixed-point.c`.

Hmm, to be honest, this was probably not what you wanted to test.
What about simply inserting your own use-case which uncovered this issue in the 
first place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96905

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

[PATCH] D95845: [ObjC] Add a command line flag that disables recognition of objc_direct for testability

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2141
+  Group, Flags<[CC1Option]>,
+  HelpText<"Ignore attribute objc_direct so that direct methods can be 
tested">;
+

arphaman wrote:
> Do you need to use the new options marshaling infrastructure?
FYI: More information on command line marshalling is here:
* https://lists.llvm.org/pipermail/cfe-dev/2021-February/067714.html
* 
https://clang.llvm.org/docs/InternalsManual.html#adding-new-command-line-option


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

https://reviews.llvm.org/D95845

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

[PATCH] D97041: [clang][cli] Pass '-Wspir-compat' to cc1 from driver

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4267
 
+  if (Triple.isSPIR())
+CmdArgs.push_back("-Wspir-compat");

It seems like this should have been in `addClangWarningOptions` but there is no 
toolchain for SPIR?

Could we add a FIXME at least saying that this deviates from the default flow 
right now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97041

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

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:433-434
+if (To == MAX) {
+  Result.insert(Result.end(), What.begin(), What.end());
+  return makePersistent(std::move(Result));
+}

ASDenysPetrov wrote:
> Here you can just move `What`. And after that you can do `Result.reserve`.
`What` is a persistent range set, so moving it or making it persistent won't 
do, but we indeed can simply return `What`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86465

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

[PATCH] D96719: [clang-tidy] Add new check 'bugprone-thread-canceltype-asynchronous' and alias 'cert-pos47-c'.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Before landing this, can you update the title of this to reflect its in the 
concurrency module, the title is typically used for the commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96719

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

[PATCH] D96948: [OpenCL] Move remaining defines to opencl-c-base.h

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Btw since we are moving this code anyway, do you think we could fix the 
formatting?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96948

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

[PATCH] D97041: [clang][cli] Pass '-Wspir-compat' to cc1 from driver

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 324959.
jansvoboda11 added a comment.

Add FIXME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97041

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/opencl.cl
  clang/test/SemaOpenCL/sampler_t.cl


Index: clang/test/SemaOpenCL/sampler_t.cl
===
--- clang/test/SemaOpenCL/sampler_t.cl
+++ clang/test/SemaOpenCL/sampler_t.cl
@@ -1,9 +1,9 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE 
-Wspir-compat -triple amdgcn--amdhsa
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE 
-triple spir-unknown-unknown
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE 
-Wspir-compat -triple spir-unknown-unknown
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only 
-DCHECK_SAMPLER_VALUE -Wspir-compat -triple amdgcn--amdhsa
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only 
-DCHECK_SAMPLER_VALUE -triple spir-unknown-unknown
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only 
-DCHECK_SAMPLER_VALUE -Wspir-compat -triple spir-unknown-unknown
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE   2
 #define CLK_NORMALIZED_COORDS_TRUE  1
Index: clang/test/Driver/opencl.cl
===
--- clang/test/Driver/opencl.cl
+++ clang/test/Driver/opencl.cl
@@ -19,6 +19,8 @@
 // RUN: %clang -S -### -cl-uniform-work-group-size %s 2>&1 | FileCheck 
--check-prefix=CHECK-UNIFORM-WG %s
 // RUN: not %clang -cl-std=c99 -DOPENCL %s 2>&1 | FileCheck 
--check-prefix=CHECK-C99 %s
 // RUN: not %clang -cl-std=invalid -DOPENCL %s 2>&1 | FileCheck 
--check-prefix=CHECK-INVALID %s
+// RUN: %clang -S -### -target spir-unknown-unknown %s 2>&1 | FileCheck 
--check-prefix=CHECK-W-SPIR-COMPAT %s
+// RUN: %clang -S -### -target amdgcn-amd-amdhsa-opencl %s 2>&1 | FileCheck 
--check-prefix=CHECK-NO-W-SPIR-COMPAT %s
 
 // CHECK-CL: "-cc1" {{.*}} "-cl-std=CL"
 // CHECK-CL10: "-cc1" {{.*}} "-cl-std=CL1.0"
@@ -45,4 +47,7 @@
 // CHECK-C99: error: invalid value 'c99' in '-cl-std=c99'
 // CHECK-INVALID: error: invalid value 'invalid' in '-cl-std=invalid'
 
+// CHECK-W-SPIR-COMPAT: "-Wspir-compat"
+// CHECK-NO-W-SPIR-COMPAT-NOT: "-Wspir-compat"
+
 kernel void func(void);
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4634,10 +4634,6 @@
 Success = false;
   }
 
-  // Turn on -Wspir-compat for SPIR target.
-  if (T.isSPIR())
-Res.getDiagnosticOpts().Warnings.push_back("spir-compat");
-
   // If sanitizer is enabled, disable OPT_ffine_grained_bitfield_accesses.
   if (Res.getCodeGenOpts().FineGrainedBitfieldAccesses &&
   !Res.getLangOpts()->Sanitize.empty()) {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4264,6 +4264,10 @@
   // are provided.
   TC.addClangWarningOptions(CmdArgs);
 
+  // FIXME: Subclass ToolChain for SPIR and move this to 
addClangWarningOptions.
+  if (Triple.isSPIR())
+CmdArgs.push_back("-Wspir-compat");
+
   // Select the appropriate action.
   RewriteKind rewriteKind = RK_None;
 


Index: clang/test/SemaOpenCL/sampler_t.cl
===
--- clang/test/SemaOpenCL/sampler_t.cl
+++ clang/test/SemaOpenCL/sampler_t.cl
@@ -1,9 +1,9 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple amdgcn--amdhsa
-// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -triple spir-unknown-unknown
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple spir-unknown-unknown
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only
 // RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple amdgcn--amdhsa
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -triple spir-unknown-unknown
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -pedantic -fsyntax-only -DCHECK_SAMPLER_VALUE -Wspir-compat -triple spir-unknown-unknown
 
 #define CLK_ADDRESS_CLAMP_TO_EDGE   2
 #define CLK_NORMALIZED_COORDS_TRUE  1
Index: clang/test/Driver/opencl.cl
===
--- clang/test/Driver/opencl.cl
+++ clang/test/Driver/opencl.cl
@@ -19,6

[PATCH] D97041: [clang][cli] Pass '-Wspir-compat' to cc1 from driver

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4267
 
+  if (Triple.isSPIR())
+CmdArgs.push_back("-Wspir-compat");

Anastasia wrote:
> It seems like this should have been in `addClangWarningOptions` but there is 
> no toolchain for SPIR?
> 
> Could we add a FIXME at least saying that this deviates from the default flow 
> right now?
That's right. I added the FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97041

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

[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

[Andrzej Warzynski via Phabricator via cfe-commits]

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

I've left a few comments, but these are nice-to-haves from my perspective. 
@tskeith , what do you think about the suggested changes in `f18`? We could 
upload a separate patch for that.




Comment at: flang/test/Flang-Driver/fdefault.f90:4
+
+! REQUIRES: new-flang-driver
+

tskeith wrote:
> Can't this work with the f18 driver too? That's the best way to ensure they 
> are consistent.
I think that this is a good idea, but there are two things that need to be 
addressed first:

**OPTION NAMES**

 `f18` uses `-module` rather than `-module-dir`. I suggest adding an alias, 
`-module-dir`, to `f18` (e.g. similarly to how `-fparse-only` and 
`-fsyntax-only` are handled)

**IMPLEMENTATION**
If I understand correctly, the current implementation of `-fdefault-real-8` in 
`f18` is incomplete, see [[ 
https://github.com/llvm/llvm-project/blob/b6db47d7e044730dc3c9b35dae6697eee0885dbf/flang/tools/f18/f18.cpp#L568-L569
 | here ]]:
```
} else if (arg == "-r8" || arg == "-fdefault-real-8") {
  defaultKinds.set_defaultRealKind(8);
```
I think that there should be `defaultKinds.set_defaultRealKind(16);` as well.

@tskeith could you confirm?



Comment at: flang/test/Flang-Driver/fdefault.f90:10
+! RUN: not %flang-new -fsyntax-only -fdefault-double-8 %s  2>&1 | FileCheck %s 
--check-prefix=DOUBLE
+! RUN: mkdir -p %t/dir-flang-new && %flang-new -fsyntax-only -module-dir 
%t/dir-flang-new -fdefault-real-8 %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | grep 'real_kind=8_4'

tskeith wrote:
> awarzynski wrote:
> > tskeith wrote:
> > > I don't think you need to create a subdirectory. `%t` is a temp directory 
> > > specific to this test.
> > > 
> > > So I'm suggesting:
> > > ```
> > > ! RUN: %flang -fsyntax-only -fdefault-real-8 %s
> > > ```
> > From [[ https://llvm.org/docs/CommandGuide/lit.html#substitutions | LIT 
> > docs ]]:
> > 
> > ```
> > %t  temporary file name unique to the test
> > %T  parent directory of %t (not unique, deprecated, do not use)
> > ```
> > 
> > So, IIUC, we do need to create a directory. We could skip `` 
> > in the directory name, but it does no harm and can be really helpful when 
> > parsing CI logs.
> > 
> > I might be missing something though. @tskeith ? 
> I thought lit created an empty directory for `%t` but apparently not. You 
> just get whatever was left over from last run. So the safest thing to do is 
> this for each compilation command:
> `! RUN: rm -rf %t && mkdir %t && %flang -module-dir %t ...`
> 
> It's true that adding an extra subdirectory does no harm besides clutter, but 
> what's the benefit?
`%t` is normally used as a temporary file name. By using e.g. 
`%t/dir-flang-new` instead, we communicate it clearly that we are using it as a 
directory name. This is obvious now (and may seem unnecessary), but it might be 
less so in 6 months or when somebody works on this project (or particular test) 
in the future.

Either way, this test is functionally correct and in fact incredibly helpful. 
Both `%t` and `%t/dir-flang-new` will work perfectly fine.



Comment at: flang/test/Flang-Driver/flarge_sizes.f90:1
+! Ensure argument -flarge-sizes works as expected.
+! TODO: Add checks when actual codegen is possible for this family

Could you add a `NOOPTION` variant like you did in fdefault.f90?


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

https://reviews.llvm.org/D96344

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

[PATCH] D96948: [OpenCL] Move remaining defines to opencl-c-base.h

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

In D96948#257 , @Anastasia wrote:

> LGTM! Btw since we are moving this code anyway, do you think we could fix the 
> formatting?

Sure, I'll do that before committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96948

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

[PATCH] D97041: [clang][cli] Pass '-Wspir-compat' to cc1 from driver

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97041

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

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 324964.
ASDenysPetrov edited the summary of this revision.
ASDenysPetrov added a comment.

Rebased revision on top of the main branch. Minor improvements.


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

https://reviews.llvm.org/D96090

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp

Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -394,48 +394,6 @@
   return UnknownVal();
 }
 
-static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) {
-  return ty1->getPointeeType().getCanonicalType().getTypePtr() ==
- ty2->getPointeeType().getCanonicalType().getTypePtr();
-}
-
-/// CastRetrievedVal - Used by subclasses of StoreManager to implement
-///  implicit casts that arise from loads from regions that are reinterpreted
-///  as another region.
-SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R,
-QualType castTy) {
-  if (castTy.isNull() || V.isUnknownOrUndef())
-return V;
-
-  // The dispatchCast() call below would convert the int into a float.
-  // What we want, however, is a bit-by-bit reinterpretation of the int
-  // as a float, which usually yields nothing garbage. For now skip casts
-  // from ints to floats.
-  // TODO: What other combinations of types are affected?
-  if (castTy->isFloatingType()) {
-SymbolRef Sym = V.getAsSymbol();
-if (Sym && !Sym->getType()->isFloatingType())
-  return UnknownVal();
-  }
-
-  // When retrieving symbolic pointer and expecting a non-void pointer,
-  // wrap them into element regions of the expected type if necessary.
-  // SValBuilder::dispatchCast() doesn't do that, but it is necessary to
-  // make sure that the retrieved value makes sense, because there's no other
-  // cast in the AST that would tell us to cast it to the correct pointer type.
-  // We might need to do that for non-void pointers as well.
-  // FIXME: We really need a single good function to perform casts for us
-  // correctly every time we need it.
-  if (castTy->isPointerType() && !castTy->isVoidPointerType())
-if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) {
-  QualType sr = SR->getSymbol()->getType();
-  if (!hasSameUnqualifiedPointeeType(sr, castTy))
-  return loc::MemRegionVal(castRegion(SR, castTy));
-}
-
-  return svalBuilder.dispatchCast(V, castTy);
-}
-
 SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) {
   if (Base.isUnknownOrUndef())
 return Base;
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -65,12 +65,12 @@
 // Transfer function for Casts.
 //===--===//
 
+// FIXME: This function should be eliminated and replaced with `evalCast`
 SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) {
-  assert(Val.getAs() || Val.getAs());
-  return Val.getAs() ? evalCastFromLoc(Val.castAs(), CastTy)
-   : evalCastFromNonLoc(Val.castAs(), CastTy);
+  return evalCast(Val, CastTy);
 }
 
+// FIXME: This function should be eliminated and replaced with `evalCast`
 SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
   bool isLocType = Loc::isLocType(castTy);
   if (val.getAs())
@@ -127,6 +127,7 @@
 return makeIntVal(i);
 }
 
+// FIXME: This function should be eliminated and replaced with `evalCast`
 SVal SimpleSValBuilder::evalCastFromLoc(Loc val, QualType castTy) {
 
   // Casts from pointers -> pointers, just return the lval.
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -536,22 +536,33 @@
 // `evalCastKind` and `evalCastSubKind` are helpers
 //===--===//
 
-SVal SValBuilder::evalCast(SVal V, QualType CastTy, QualType OriginalTy) {
-  CastTy = Context.getCanonicalType(CastTy);
-  OriginalTy = Context.getCanonicalType(OriginalTy);
-  if (CastTy == OriginalTy)
+// In case when `OriginalTy.isNull() == true` we cast `V` less accurately.
+SVal SValBuilder::evalCast(SVal V, QualType CastTy,
+   QualType OriginalTy /*= QualType{}*/) {
+  if (CastTy.isNull())
 return V;
 
-  // FIXME: Mov

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@steakhal wrote:

> This patch preserves all previous reports as expected.

If this patch is technically full-baked and does not bring any harm, can we 
approve this it as well to move forward?


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

https://reviews.llvm.org/D96090

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

[PATCH] D96860: [OpenCL] Add declarations with enum/typedef args

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Sema/OpenCLBuiltins.td:932
+let MinVersion = CL20 in {
+  def : Builtin<"get_fence", [MemFenceFlags, PointerType]>;
+  def : Builtin<"get_fence", [MemFenceFlags, PointerType, 
GenericAS>]>;

Btw, I am guessing you are matching the behavior of `opencl-c.h`, but however 
it doesn't seem entirely conformant.

`cl_mem_fence_flags get_fence(gentype *ptr)`

So I guess we should have implemented these as other functions from `Table 22. 
Built-in Address Space Qualifier Functions` in clang? But since the void 
pointer is only used as a parameter and not return value I doubt this is 
customer visible. Should we add a comment or something?






Comment at: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl:185
+#if !defined(NO_HEADER) && (defined(__OPENCL_CPP_VERSION__) || 
__OPENCL_C_VERSION__ >= 200)
+  sub_group_barrier(CLK_GLOBAL_MEM_FENCE, memory_scope_device);
+#endif

Should `work_group_barrier` and fences also be added to the testing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96860

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

[PATCH] D96195: [HIP] Fix managed variable linkage

[Yaxun Liu via Phabricator via cfe-commits]

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

refactor CGCUDARuntime interface not to expose transformManagedVar.


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

https://reviews.llvm.org/D96195

Files:
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CGCUDARuntime.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCUDA/device-var-linkage.cu
  clang/test/CodeGenCUDA/managed-var.cu
  llvm/lib/IR/ReplaceConstant.cpp

Index: llvm/lib/IR/ReplaceConstant.cpp
===
--- llvm/lib/IR/ReplaceConstant.cpp
+++ llvm/lib/IR/ReplaceConstant.cpp
@@ -60,6 +60,7 @@
   case Instruction::PtrToInt:
   case Instruction::IntToPtr:
   case Instruction::BitCast:
+  case Instruction::AddrSpaceCast:
 return dyn_cast(
 Builder.CreateCast((Instruction::CastOps)OpCode, CE->getOperand(0),
CE->getType(), CE->getName()));
Index: clang/test/CodeGenCUDA/managed-var.cu
===
--- clang/test/CodeGenCUDA/managed-var.cu
+++ clang/test/CodeGenCUDA/managed-var.cu
@@ -2,47 +2,62 @@
 
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -std=c++11 \
 // RUN:   -emit-llvm -o - -x hip %s | FileCheck \
-// RUN:   -check-prefixes=DEV %s
+// RUN:   -check-prefixes=COMMON,DEV %s
 
 // RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device -std=c++11 \
 // RUN:   -emit-llvm -fgpu-rdc -o - -x hip %s | FileCheck \
-// RUN:   -check-prefixes=DEV %s
+// RUN:   -check-prefixes=COMMON,DEV %s
 
 // RUN: %clang_cc1 -triple x86_64-gnu-linux -std=c++11 \
 // RUN:   -emit-llvm -o - -x hip %s | FileCheck \
-// RUN:   -check-prefixes=HOST,NORDC %s
+// RUN:   -check-prefixes=COMMON,HOST,NORDC %s
 
 // RUN: %clang_cc1 -triple x86_64-gnu-linux -std=c++11 \
 // RUN:   -emit-llvm -fgpu-rdc -o - -x hip %s | FileCheck \
-// RUN:   -check-prefixes=HOST,RDC %s
+// RUN:   -check-prefixes=COMMON,HOST,RDC %s
 
 #include "Inputs/cuda.h"
 
-// DEV-DAG: @x = external addrspace(1) externally_initialized global i32
-// NORDC-DAG: @x = internal global i32 1
-// RDC-DAG: @x = dso_local global i32 1
-// NORDC-DAG: @x.managed = internal global i32* null
-// RDC-DAG: @x.managed = dso_local global i32* null
-// HOST-DAG: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"x\00"
-
 struct vec {
   float x,y,z;
 };
 
+// DEV-DAG: @x.managed = dso_local addrspace(1) externally_initialized global i32 1, align 4
+// DEV-DAG: @x = dso_local addrspace(1) externally_initialized global i32 addrspace(1)* null
+// NORDC-DAG: @x.managed = internal global i32 1
+// RDC-DAG: @x.managed = dso_local global i32 1
+// NORDC-DAG: @x = internal externally_initialized global i32* null
+// RDC-DAG: @x = dso_local externally_initialized global i32* null
+// HOST-DAG: @[[DEVNAMEX:[0-9]+]] = {{.*}}c"x\00"
 __managed__ int x = 1;
+
+// DEV-DAG: @v.managed = dso_local addrspace(1) externally_initialized global [100 x %struct.vec] zeroinitializer, align 4
+// DEV-DAG: @v = dso_local addrspace(1) externally_initialized global [100 x %struct.vec] addrspace(1)* null
 __managed__ vec v[100];
+
+// DEV-DAG: @v2.managed = dso_local addrspace(1) externally_initialized global <{ %struct.vec, [99 x %struct.vec] }> <{ %struct.vec { float 1.00e+00, float 1.00e+00, float 1.00e+00 }, [99 x %struct.vec] zeroinitializer }>, align 4
+// DEV-DAG: @v2 = dso_local addrspace(1) externally_initialized global <{ %struct.vec, [99 x %struct.vec] }> addrspace(1)* null
 __managed__ vec v2[100] = {{1, 1, 1}};
 
-// DEV-DAG: @ex = external addrspace(1) global i32
-// HOST-DAG: @ex = external global i32
+// DEV-DAG: @ex.managed = external addrspace(1) global i32, align 4
+// DEV-DAG: @ex = external addrspace(1) externally_initialized global i32 addrspace(1)*
+// HOST-DAG: @ex.managed = external global i32
+// HOST-DAG: @ex = external externally_initialized global i32*
 extern __managed__ int ex;
 
-// DEV-DAG: @_ZL2sx = external addrspace(1) externally_initialized global i32
-// HOST-DAG: @_ZL2sx = internal global i32 1
-// HOST-DAG: @_ZL2sx.managed = internal global i32* null
+// DEV-DAG: @_ZL2sx.managed = dso_local addrspace(1) externally_initialized global i32 1, align 4
+// DEV-DAG: @_ZL2sx = dso_local addrspace(1) externally_initialized global i32 addrspace(1)* null
+// HOST-DAG: @_ZL2sx.managed = internal global i32 1
+// HOST-DAG: @_ZL2sx = internal externally_initialized global i32* null
 static __managed__ int sx = 1;
 
-// HOST-NOT: @ex.managed
+// DEV-DAG: @llvm.compiler.used
+// DEV-SAME-DAG: @x.managed
+// DEV-SAME-DAG: @x
+// DEV-SAME-DAG: @v.managed
+// DEV-SAME-DAG: @v
+// DEV-SAME-DAG: @_ZL2sx.managed
+// DEV-SAME-DAG: @_ZL2sx
 
 // Force ex and sx mitted in device compilation.
 __global__ void foo(int *z) {
@@ -55,42 +70,53 @@
   return ex + sx;
 }
 
-// HOST-LABEL: define {{.*}}@_Z4loadv()
-// HOST:  %ld.managed = load i32*, i32** @x.managed, align 4
+// COMMON-LABEL:

[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

[Tim Keith via Phabricator via cfe-commits]

tskeith added inline comments.



Comment at: flang/test/Flang-Driver/fdefault.f90:4
+
+! REQUIRES: new-flang-driver
+

awarzynski wrote:
> tskeith wrote:
> > Can't this work with the f18 driver too? That's the best way to ensure they 
> > are consistent.
> I think that this is a good idea, but there are two things that need to be 
> addressed first:
> 
> **OPTION NAMES**
> 
>  `f18` uses `-module` rather than `-module-dir`. I suggest adding an alias, 
> `-module-dir`, to `f18` (e.g. similarly to how `-fparse-only` and 
> `-fsyntax-only` are handled)
> 
> **IMPLEMENTATION**
> If I understand correctly, the current implementation of `-fdefault-real-8` 
> in `f18` is incomplete, see [[ 
> https://github.com/llvm/llvm-project/blob/b6db47d7e044730dc3c9b35dae6697eee0885dbf/flang/tools/f18/f18.cpp#L568-L569
>  | here ]]:
> ```
> } else if (arg == "-r8" || arg == "-fdefault-real-8") {
>   defaultKinds.set_defaultRealKind(8);
> ```
> I think that there should be `defaultKinds.set_defaultRealKind(16);` as well.
> 
> @tskeith could you confirm?
> I think that this is a good idea, but there are two things that need to be 
> addressed first:
> 
> **OPTION NAMES**
> 
>  `f18` uses `-module` rather than `-module-dir`. I suggest adding an alias, 
> `-module-dir`, to `f18` (e.g. similarly to how `-fparse-only` and 
> `-fsyntax-only` are handled)

Yes, in general when an option is given a different name in the new driver that 
option should be added to f18. Not only for testing consistent behavior but 
also because f18 gets more usage so any problems are more likely to turn up.

> **IMPLEMENTATION**
> If I understand correctly, the current implementation of `-fdefault-real-8` 
> in `f18` is incomplete, see [[ 
> https://github.com/llvm/llvm-project/blob/b6db47d7e044730dc3c9b35dae6697eee0885dbf/flang/tools/f18/f18.cpp#L568-L569
>  | here ]]:
> ```
> } else if (arg == "-r8" || arg == "-fdefault-real-8") {
>   defaultKinds.set_defaultRealKind(8);
> ```
> I think that there should be `defaultKinds.set_defaultRealKind(16);` as well.

Do you mean `set_doublePrecisionKind(16)`? Yes, if that's how 
`-fdefault-real-8` is supposed to behave, it should work that way in f18 too.






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

https://reviews.llvm.org/D96344

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

[PATCH] D95799: [analyzer] Symbolicate float values with integral casting

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

This is just a note for me to correctly rebase it in the future:
https://reviews.llvm.org/D96090?vs=321602&id=322422#toc


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

https://reviews.llvm.org/D95799

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

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 324981.
njames93 added a comment.

Restrict availability of default capture fixit based on init captures already 
specified.
`[=, A]() {}` and `[&, &A]() {}` are both invalid as explicit captures can't be 
specified with the same type as default capture. 
So we shouldn't offer '=' fixit if any init captures are by value and '&' if 
any by ref.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96975

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p12.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2-generic-lambda-1y.cpp
  clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
  clang/test/SemaCXX/cxx1y-generic-lambdas.cpp
  clang/test/SemaCXX/cxx1y-init-captures.cpp
  clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
  clang/test/SemaCXX/lambda-expressions.cpp
  clang/test/SemaCXX/lambda-invalid-capture.cpp
  clang/test/SemaObjCXX/capturing-flexible-array-in-block.mm

Index: clang/test/SemaObjCXX/capturing-flexible-array-in-block.mm
===
--- clang/test/SemaObjCXX/capturing-flexible-array-in-block.mm
+++ clang/test/SemaObjCXX/capturing-flexible-array-in-block.mm
@@ -5,5 +5,5 @@
   struct { int x; int y[]; } a; // expected-note 3 {{'a' declared here}}
   ^{return a.x;}(); // expected-error {{cannot refer to declaration of structure variable with flexible array member inside block}}
   [=] {return a.x;}(); // expected-error {{variable 'a' with flexible array member cannot be captured in a lambda expression}}
-  [] {return a.x;}(); // expected-error {{variable 'a' cannot be implicitly captured in a lambda with no capture-default}} expected-note {{here}}
+  [] {return a.x;}(); // expected-error {{variable 'a' cannot be implicitly captured in a lambda with no capture-default}} expected-note {{here}} expected-note 4 {{capture}}
 }
Index: clang/test/SemaCXX/lambda-invalid-capture.cpp
===
--- clang/test/SemaCXX/lambda-invalid-capture.cpp
+++ clang/test/SemaCXX/lambda-invalid-capture.cpp
@@ -18,7 +18,7 @@
 }
 
 int pr43080(int i) { // expected-note {{declared here}}
-  return [] { // expected-note {{begins here}}
+  return [] { // expected-note {{begins here}} expected-note 4 {{capture}}
 return sizeof i <
   i; // expected-error {{variable 'i' cannot be implicitly captured in a lambda with no capture-default specified}}
   }();
Index: clang/test/SemaCXX/lambda-expressions.cpp
===
--- clang/test/SemaCXX/lambda-expressions.cpp
+++ clang/test/SemaCXX/lambda-expressions.cpp
@@ -12,17 +12,17 @@
 virtual C& Overload(float);
 
 void ImplicitThisCapture() {
-  [](){(void)Member;}; // expected-error {{'this' cannot be implicitly captured in this context}}
-  const int var = [](){(void)Member; return 0;}(); // expected-error {{'this' cannot be implicitly captured in this context}}
+  [](){(void)Member;}; // expected-error {{'this' cannot be implicitly captured in this context}} expected-note {{explicitly capture 'this'}}
+  const int var = [](){(void)Member; return 0;}(); // expected-error {{'this' cannot be implicitly captured in this context}} expected-note {{explicitly capture 'this'}}
   [&](){(void)Member;};
 
   [this](){(void)Member;};
   [this]{[this]{};};
   []{[this]{};};// expected-error {{'this' cannot be implicitly captured in this context}}
   []{Overload(3);};
-  []{Overload();}; // expected-error {{'this' cannot be implicitly captured in this context}}
+  []{Overload();}; // expected-error {{'this' cannot be implicitly captured in this context}} expected-note {{explicitly capture 'this'}}
   []{(void)typeid(Overload());};
-  []{(void)typeid(Overload(.5f));};// expected-error {{'this' cannot be implicitly captured in this context}}
+  []{(void)typeid(Overload(.5f));};// expected-error {{'this' cannot be implicitly captured in this context}} expected-note {{explicitly capture 'this'}}
 }
   };
 
@@ -47,14 +47,14 @@
 namespace ImplicitCapture {
   void test() {
 int a = 0; // expected-note 5 {{declared}}
-[]() { return a; }; // expected-error {{variable 'a' cannot be implicitly captured in a lambda with no capture-default specified}} expected-note {{begins here}}
+[]() { return a; }; // expected-error {{variable 'a' cannot be implicitly captured in a lambda with no capture-default specified}} expected-note {{begins here}} expected-note 4 {{capture}}
 [&]() { return a; };
 [=]() { return a; };
 [=]() { int* b = &a; }; // expected-error {{cannot initialize a variable of type 'int *' with an rvalue of type 'const int *'}}
 [=]() { return [&]() { return

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7459-7460
 "capture-default specified">;
+  def note_lambda_variable_capture_fixit : Note<
+"capture variable %0 by %select{value|reference}1">;
+  def note_lambda_default_capture_fixit : Note<

Does the variable name need attaching to the note, given the note is attached 
to a fix-it containing the name of the variable already?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96975

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

[PATCH] D97052: [OpenCL] Prevent adding extension pragma by default

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: azabaznov, svenvh, mantognini.
Herald added subscribers: ebevhan, jfb, yaxunl.
Anastasia requested review of this revision.

Currently, extension pragma is always added when new OpenCL option (extension 
or feature) is added to the frontend. This change adds a new field for the 
options that allows specifying the need for the pragmas.

For backward compatibility, the pragmas are added for all extensions prior to 
OpenCL 3.0.

For OpenCL 3.0 features and future extensions (those that are documented in the 
unified spec only), the pragmas is not to be added unless they are explicitly 
requested and their behavior is sufficiently well explained.


https://reviews.llvm.org/D97052

Files:
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/include/clang/Basic/OpenCLOptions.h
  clang/lib/Basic/OpenCLOptions.cpp
  clang/lib/Basic/Targets.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/SemaOpenCL/extension-version.cl

Index: clang/test/SemaOpenCL/extension-version.cl
===
--- clang/test/SemaOpenCL/extension-version.cl
+++ clang/test/SemaOpenCL/extension-version.cl
@@ -3,15 +3,13 @@
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=clc++ %s -verify -triple spir-unknown-unknown
+// RUN: %clang_cc1 -x cl -cl-std=CL3.0 %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=clc++ %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
-
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200) && !defined(TEST_CORE_FEATURES)
-// expected-no-diagnostics
-#endif
+// RUN: %clang_cc1 -x cl -cl-std=CL3.0 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 
 // Extensions in all versions
 #ifndef cl_clang_storage_class_specifiers
@@ -100,7 +98,7 @@
 #error "Missing cl_khr_3d_image_writes define"
 #endif
 #pragma OPENCL EXTENSION cl_khr_3d_image_writes : enable
-#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200) && defined TEST_CORE_FEATURES
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ == 200) && defined TEST_CORE_FEATURES
 // expected-warning@-2{{OpenCL extension 'cl_khr_3d_image_writes' is core feature or supported optional core feature - ignoring}}
 #endif
 
@@ -215,3 +213,55 @@
 // expected-warning@+2{{unsupported OpenCL extension 'cl_intel_device_side_avc_motion_estimation' - ignoring}}
 #endif
 #pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable
+
+// Check that pragmas for the OpenCL 3.0 features are rejected.
+
+#pragma OPENCL EXTENSION __opencl_c_int64 : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_int64' - ignoring}}
+#pragma OPENCL EXTENSION __opencl_c_3d_image_writes : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_3d_image_writes' - ignoring}}
+#pragma OPENCL EXTENSION __opencl_c_atomic_order_acq_rel : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_atomic_order_acq_rel' - ignoring}}
+#pragma OPENCL EXTENSION __opencl_c_atomic_order_seq_cst : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_atomic_order_seq_cst' - ignoring}}
+#pragma OPENCL EXTENSION __opencl_c_device_enqueue : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_device_enqueue' - ignoring}}
+#pragma OPENCL EXTENSION __opencl_c_fp64 : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_fp64' - ignoring}}
+#pragma OPENCL EXTENSION __opencl_c_generic_address_space : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_generic_address_space' - ignoring}}
+#pragma OPENCL EXTENSION __opencl_c_images : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_images' - ignoring}}
+#pragma OPENCL EXTENSION __opencl_c_pipes : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_pipes' - ignoring}}
+#pragma OPENCL EXTENSION __opencl_c_program_scope_global_variables : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_program_scope_global_variables' - ignoring}}
+#pragma OPENCL EXTENSION __opencl_c_read_write_images : disable
+//expected-warning@-1{{unknown OpenCL extension '__opencl_c_

[PATCH] D96771: [OpenCL] Add distinct file extension for C++ for OpenCL

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D96771#2571855 , @Anastasia wrote:

> This is only the initial patch and for the moment the primary goal is to 
> remove the need for the flag at least from the clang perspective.

Shorter compiler invocation is always nice! Does OpenCL (will) require a flag 
to specify the C++ standard used in the kernel? Or is that going to be 
controlled with `-cl-std={cl2.1|cl2.2|cl2.3`? This would be very weird 
`-cl-std=clc++ -cl-std=cl2.1`, so I see why you would want to remove  
`-cl-std=clc++`.

> I just dislike the fact that moving files in the repo complicates the commit 
> history lookup so I was not sure if the renaming was good or bad thing. Do 
> you have any suggestions?

Have you tried it? Git is very clever in this respect and will track the file 
changes very accurately anyway. IMHO this wouldn't be disruptive in terms of 
browsing the history at all. If you are making this change to remove the need 
for `-cl-std=clc++` and to simplify things, you could as well organize tests 
per kernel language (i.e. OpenCL C vs OpenCL C++). Also, `-cl-std=clc++` in 
tests will look very confusing (i.e., why keep OpenCL C++ tests in OpenCL C 
tests and force the standard with `-cl-std=clc++`)? Lastly, such change would 
be a very through verification of your changes :)

> I was thinking to introduce as a guideline that the new tests should 
> definitely use the new extension though.

+1

> I find the way driver is setup for the languages quite different so I was 
> wondering if we have any guidelines?

I'm not aware of any documentation for the driver, sorry.

By scanning your patch I get the impression that you don't need `TY_CLCXX` just 
yet. The language for the kernel is set in 
`clang/lib/Frontend/CompilerInvocation.cpp`:

  .Case("clcpp", Language::OpenCLCXX)

In Types.cpp this should be sufficient for now:

  .Case("clcpp", TY_CLCXX)

Basically, you could limit your changes to the frontend driver and the changes 
in the compiler driver keep to a tiny minimum. I think! :)


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

https://reviews.llvm.org/D96771

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

[PATCH] D97053: [clang][SVE] Don't warn on vector to sizeless builtin implicit conversion

[Joe Ellis via Phabricator via cfe-commits]

joechrisellis created this revision.
joechrisellis added reviewers: c-rhodes, bsmith.
Herald added subscribers: psnobl, kristof.beyls, tschuett.
Herald added a reviewer: efriedma.
joechrisellis requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This commit prevents warnings from -Wconversion when a clang vector type
is implicitly converted to a sizeless builtin type -- for example, when
implicitly converting a fixed-predicate to a scalable predicate.

The code below:

  1#include 
  2
  3#define N __ARM_FEATURE_SVE_BITS
  4#define FIXED_ATTR __attribute__((arm_sve_vector_bits (N)))
  5typedef svbool_t fixed_svbool_t FIXED_ATTR;
  6
  7inline fixed_svbool_t foo(fixed_svbool_t p) {
  8  return svnot_z(svptrue_b64(), p);
  9}

would previously raise this warning:

  warning: implicit conversion turns vector to scalar: \
  'fixed_svbool_t' (vector of 8 'unsigned char' values) to 'svbool_t' \
  (aka '__SVBool_t') [-Wconversion]

Note that many cases of these implicit conversions were already
permitted because many functions inside arm_sve.h are spawned via
preprocessor macros, and the call to isInSystemMacro would cover us in
this case. This commit fixes the remaining cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97053

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c


Index: clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c
===
--- /dev/null
+++ clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve \
+// RUN: -msve-vector-bits=512 -Wconversion -fallow-half-arguments-and-returns \
+// RUN: -o /dev/null %s 2>&1 | FileCheck %s -allow-empty
+
+// CHECK-NOT: warning
+
+#include 
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svbool_t fixed_svbool_t FIXED_ATTR;
+
+fixed_svbool_t check_svnot(fixed_svbool_t p) {
+  return svnot_z(svptrue_b64(), p);
+}
+
+fixed_svbool_t check_svorr(fixed_svbool_t p1, fixed_svbool_t p2) {
+  return svorr_z(svptrue_b64(), p1, p2);
+}
+
+fixed_svbool_t check_svorn(fixed_svbool_t p1, fixed_svbool_t p2) {
+  return svorn_z(svptrue_b64(), p1, p2);
+}
+
+fixed_svbool_t check_svand(fixed_svbool_t p1, fixed_svbool_t p2) {
+  return svand_z(svptrue_b64(), p1, p2);
+}
+
+fixed_svbool_t check_svnand(fixed_svbool_t p1, fixed_svbool_t p2) {
+  return svnand_z(svptrue_b64(), p1, p2);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12052,6 +12052,9 @@
 
   // Strip vector types.
   if (isa(Source)) {
+// Converting from a vector to a sizeless builtin type is fine.
+if (Target->isSizelessBuiltinType())
+  return;
 if (!isa(Target)) {
   if (S.SourceMgr.isInSystemMacro(CC))
 return;


Index: clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c
===
--- /dev/null
+++ clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve \
+// RUN: -msve-vector-bits=512 -Wconversion -fallow-half-arguments-and-returns \
+// RUN: -o /dev/null %s 2>&1 | FileCheck %s -allow-empty
+
+// CHECK-NOT: warning
+
+#include 
+
+#define N __ARM_FEATURE_SVE_BITS
+#define FIXED_ATTR __attribute__((arm_sve_vector_bits(N)))
+
+typedef svbool_t fixed_svbool_t FIXED_ATTR;
+
+fixed_svbool_t check_svnot(fixed_svbool_t p) {
+  return svnot_z(svptrue_b64(), p);
+}
+
+fixed_svbool_t check_svorr(fixed_svbool_t p1, fixed_svbool_t p2) {
+  return svorr_z(svptrue_b64(), p1, p2);
+}
+
+fixed_svbool_t check_svorn(fixed_svbool_t p1, fixed_svbool_t p2) {
+  return svorn_z(svptrue_b64(), p1, p2);
+}
+
+fixed_svbool_t check_svand(fixed_svbool_t p1, fixed_svbool_t p2) {
+  return svand_z(svptrue_b64(), p1, p2);
+}
+
+fixed_svbool_t check_svnand(fixed_svbool_t p1, fixed_svbool_t p2) {
+  return svnand_z(svptrue_b64(), p1, p2);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12052,6 +12052,9 @@
 
   // Strip vector types.
   if (isa(Source)) {
+// Converting from a vector to a sizeless builtin type is fine.
+if (Target->isSizelessBuiltinType())
+  return;
 if (!isa(Target)) {
   if (S.SourceMgr.isInSystemMacro(CC))
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97053: [clang][SVE] Don't warn on vector to sizeless builtin implicit conversion

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: 
clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c:1-32
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve \
+// RUN: -msve-vector-bits=512 -Wconversion -fallow-half-arguments-and-returns \
+// RUN: -o /dev/null %s 2>&1 | FileCheck %s -allow-empty
+
+// CHECK-NOT: warning
+
+#include 

I wonder if we can just add `-Wconversion` to the existing Sema tests?
* clang/test/Sema/attr-arm-sve-vector-bits.c
* clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97053

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

[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 updated this revision to Diff 324997.
arnamoy10 added a comment.

Added a missing option test for `-flarge-sizes`


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

https://reviews.llvm.org/D96344

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/fdefault.f90
  flang/test/Flang-Driver/flarge_sizes.f90
  flang/test/Flang-Driver/pipeline.f90

Index: flang/test/Flang-Driver/pipeline.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/pipeline.f90
@@ -0,0 +1,17 @@
+! This file tests that flang-new forwards 
+! all Flang frontend options to flang-new -fc1
+! as expected.
+!
+! RUN: %flang-new -fsyntax-only -### %s -o %t 2>&1 \
+! RUN: -fdefault-double-8 \
+! RUN: -fdefault-integer-8 \
+! RUN: -fdefault-real-8 \
+! RUN: -flarge-sizes \
+! RUN:   | FileCheck %s
+!
+!
+! CHECK: "-fdefault-double-8"
+! CHECK: "-fdefault-integer-8"
+! CHECK: "-fdefault-real-8"
+! CHECK: "-flarge-sizes"
+
Index: flang/test/Flang-Driver/flarge_sizes.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/flarge_sizes.f90
@@ -0,0 +1,37 @@
+! Ensure argument -flarge-sizes works as expected.
+! TODO: Add checks when actual codegen is possible.
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fsyntax-only -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=NOLARGE
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fsyntax-only -flarge-sizes -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=LARGE
+
+!-
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fc1 -fsyntax-only -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=NOLARGE
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fc1 -fsyntax-only -flarge-sizes -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=LARGE
+
+!-
+! EXPECTED OUTPUT WITHOUT -flarge-sizes
+!-
+! NOLARGE: real(4)::z(1_8:10_8)
+! NOLARGE-NEXT: integer(4),parameter::size_kind=4_4
+
+!-
+! EXPECTED OUTPUT FOR -flarge-sizes
+!-
+! LARGE: real(4)::z(1_8:10_8)
+! LARGE-NEXT: integer(4),parameter::size_kind=8_4
+
+module m
+  implicit none
+  real :: z(10)
+  integer, parameter :: size_kind = kind(ubound(z, 1)) !-flarge-sizes
+end
Index: flang/test/Flang-Driver/fdefault.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fdefault.f90
@@ -0,0 +1,62 @@
+! Ensure argument -fdefault* work as expected.
+! TODO: Add checks when actual codegen is possible for this family
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fsyntax-only -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=NOOPTION
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fsyntax-only -fdefault-real-8 -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=REAL8
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fsyntax-only -fdefault-real-8 -fdefault-double-8 -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=DOUBLE8
+! RUN: not %flang -fsyntax-only -fdefault-double-8 %s  2>&1 | FileCheck %s --check-prefix=ERROR
+
+!-
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fc1 -fsyntax-only -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=NOOPTION
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fc1 -fsyntax-only -fdefault-real-8 -module-dir %t/dir-flang-new %s  2>&1
+! RUN: cat %t/dir-flang-new/m.mod | FileCheck %s --check-prefix=REAL8
+! RUN: rm -rf %t/dir-flang-new  && mkdir -p %t/dir-flang-new && %flang -fc1 -fsyntax-only -fdefault-real-8 -fdefault-double-

[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 added inline comments.



Comment at: flang/test/Flang-Driver/flarge_sizes.f90:1
+! Ensure argument -flarge-sizes works as expected.
+! TODO: Add checks when actual codegen is possible for this family

awarzynski wrote:
> Could you add a `NOOPTION` variant like you did in fdefault.f90?
Done!


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

https://reviews.llvm.org/D96344

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

[PATCH] D97058: [OpenCL] Refactor diagnostic for OpenCL extension/feature

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov created this revision.
azabaznov added reviewers: Anastasia, svenvh.
Herald added subscribers: jfb, yaxunl.
azabaznov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There is no need to check for enabled pragma for core or optional core features,
thus this check is removed


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97058

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenCLOptions.h
  clang/lib/Basic/OpenCLOptions.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Parser/opencl-atomics-cl20.cl
  clang/test/SemaOpenCL/access-qualifier.cl
  clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
  clang/test/SemaOpenCL/extension-begin.cl
  clang/test/SemaOpenCL/extensions.cl

Index: clang/test/SemaOpenCL/extensions.cl
===
--- clang/test/SemaOpenCL/extensions.cl
+++ clang/test/SemaOpenCL/extensions.cl
@@ -43,8 +43,8 @@
 #endif
 
 #if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ < 120)
-void f1(double da) { // expected-error {{type 'double' requires cl_khr_fp64 extension}}
-  double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
+void f1(double da) { // expected-error {{type 'double' requires cl_khr_fp64 support}}
+  double d; // expected-error {{type 'double' requires cl_khr_fp64 support}}
   (void) 1.0; // expected-warning {{double precision constant requires cl_khr_fp64}}
 }
 #endif
@@ -72,14 +72,14 @@
 void f2(void) {
   double d;
 #ifdef NOFP64
-// expected-error@-2{{use of type 'double' requires cl_khr_fp64 extension to be enabled}}
+// expected-error@-2{{use of type 'double' requires cl_khr_fp64 support}}
 #endif
 
   typedef double double4 __attribute__((ext_vector_type(4)));
   double4 d4 = {0.0f, 2.0f, 3.0f, 1.0f};
 #ifdef NOFP64
-// expected-error@-3 {{use of type 'double' requires cl_khr_fp64 extension to be enabled}}
-// expected-error@-3 {{use of type 'double4' (vector of 4 'double' values) requires cl_khr_fp64 extension to be enabled}}
+// expected-error@-3 {{use of type 'double' requires cl_khr_fp64 support}}
+// expected-error@-3 {{use of type 'double4' (vector of 4 'double' values) requires cl_khr_fp64 support}}
 #endif
 
   (void) 1.0;
@@ -96,6 +96,6 @@
 
 #if (defined(__OPENCL_C_VERSION__) && __OPENCL_C_VERSION__ < 120)
 void f3(void) {
-  double d; // expected-error {{type 'double' requires cl_khr_fp64 extension}}
+  double d; // expected-error {{type 'double' requires cl_khr_fp64 support}}
 }
 #endif
Index: clang/test/SemaOpenCL/extension-begin.cl
===
--- clang/test/SemaOpenCL/extension-begin.cl
+++ clang/test/SemaOpenCL/extension-begin.cl
@@ -41,11 +41,11 @@
 
 #pragma OPENCL EXTENSION my_ext : disable 
 void test_f2(void) {
-  struct A test_A2; // expected-error {{use of type 'struct A' requires my_ext extension to be enabled}}
-  const struct A test_A_local; // expected-error {{use of type 'struct A' requires my_ext extension to be enabled}}
-  TypedefOfA test_typedef_A; // expected-error {{use of type 'TypedefOfA' (aka 'struct A') requires my_ext extension to be enabled}}
-  PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka 'const __private struct A *') requires my_ext extension to be enabled}}
-  f(); // expected-error {{use of declaration 'f' requires my_ext extension to be enabled}}
+  struct A test_A2; // expected-error {{use of type 'struct A' requires my_ext support}}
+  const struct A test_A_local; // expected-error {{use of type 'struct A' requires my_ext support}}
+  TypedefOfA test_typedef_A; // expected-error {{use of type 'TypedefOfA' (aka 'struct A') requires my_ext support}}
+  PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka 'const __private struct A *') requires my_ext support}}
+  f(); // expected-error {{use of declaration 'f' requires my_ext support}}
   g(0); // expected-error {{no matching function for call to 'g'}}
 // expected-note@extension-begin.h:18 {{candidate unavailable as it requires OpenCL extension 'my_ext' to be enabled}}
 // expected-note@extension-begin.h:23 {{candidate function not viable: requires 0 arguments, but 1 was provided}}
Index: clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
===
--- clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
+++ clang/test/SemaOpenCL/cl20-device-side-enqueue.cl
@@ -235,11 +235,11 @@
 kernel void foo1(global unsigned int *buf)
 {
   ndrange_t n;
-  buf[0] = get_kernel_max_sub_group_size_for_ndrange(n, ^(){}); // expected-error {{use

[PATCH] D97052: [OpenCL] Prevent adding extension pragma by default

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added a comment.

Thanks! I review is shortly. As for now, I also was doing some refactoring: 
https://reviews.llvm.org/D97058. Check-clang passes, as I see there are no 
conflicts for now.


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

https://reviews.llvm.org/D97052

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

[PATCH] D96860: [OpenCL] Add declarations with enum/typedef args

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: clang/lib/Sema/OpenCLBuiltins.td:932
+let MinVersion = CL20 in {
+  def : Builtin<"get_fence", [MemFenceFlags, PointerType]>;
+  def : Builtin<"get_fence", [MemFenceFlags, PointerType, 
GenericAS>]>;

Anastasia wrote:
> Btw, I am guessing you are matching the behavior of `opencl-c.h`, but however 
> it doesn't seem entirely conformant.
> 
> `cl_mem_fence_flags get_fence(gentype *ptr)`
> 
> So I guess we should have implemented these as other functions from `Table 
> 22. Built-in Address Space Qualifier Functions` in clang? But since the void 
> pointer is only used as a parameter and not return value I doubt this is 
> customer visible. Should we add a comment or something?
> 
> 
> 
Yes, I'm matching  `opencl-c.h` here, as the "gentype argument to indicate any 
of the built-in data types supported by OpenCL C or a user defined type.  Since 
this cannot be captured" can't be expressed literally.  I can add the following 
comment if that makes sense?
```// The OpenCL 3.0 specification defines these with a "gentype" argument 
indicating any builtin
// type or user-defined type, which cannot be represented currently.  Hence we 
slightly diverge
// from this by providing only the following two overloads with a void pointer.
```



Comment at: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl:185
+#if !defined(NO_HEADER) && (defined(__OPENCL_CPP_VERSION__) || 
__OPENCL_C_VERSION__ >= 200)
+  sub_group_barrier(CLK_GLOBAL_MEM_FENCE, memory_scope_device);
+#endif

Anastasia wrote:
> Should `work_group_barrier` and fences also be added to the testing?
MemFenceFlags are already tested using `barrier()` on line 23, so not strictly 
needed.  I added this test to verify the combination of MemFenceFlags and 
extensions.  Do you think we're missing any tablegen functionality coverage 
that would require adding more builtins to the test (also taking into account 
the comment on lines 10-13)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96860

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

[PATCH] D96803: EntryExitInstrumenter: Enable at all optimization levels (PR49143)

[Adhemerval Zanella via Phabricator via cfe-commits]

zatrazz updated this revision to Diff 325004.
zatrazz retitled this revision from "EntryExitInstrumenter: Move to a module 
pass and enable at all optimization levels (PR49143)" to 
"EntryExitInstrumenter: Enable at all optimization levels (PR49143)".
zatrazz edited the summary of this revision.
zatrazz added a comment.

Updated patch based on previous comments.


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

https://reviews.llvm.org/D96803

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Transforms/Utils/EntryExitInstrumenter.h
  llvm/test/Transforms/EntryExitInstrumenter/mcount.ll


Index: llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
===
--- llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
+++ llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
@@ -1,4 +1,7 @@
-; RUN: opt 
-passes="function(ee-instrument),cgscc(inline),function(post-inline-ee-instrument)"
 -S < %s | FileCheck %s
+; RUN: opt --O0 --ee-instrument --inline --post-inline-ee-instrument -S < %s | 
FileCheck %s
+
+; Check if the pass is enabbled on both -O0 and higher optimization levels
+; RUN: opt --O2 --ee-instrument --post-inline-ee-instrument -S < %s | 
FileCheck --check-prefix=OPT %s
 
 ; Running the passes twice should not result in more instrumentation.
 ; RUN: opt 
-passes="function(ee-instrument),function(ee-instrument),cgscc(inline),function(post-inline-ee-instrument),function(post-inline-ee-instrument)"
 -S < %s | FileCheck %s
@@ -18,6 +21,15 @@
 ; CHECK-NEXT: %1 = call i8* @llvm.returnaddress(i32 0)
 ; CHECK-NEXT: call void @__cyg_profile_func_exit(i8* bitcast (void ()* 
@leaf_function to i8*), i8* %1)
 ; CHECK-NEXT: ret void
+
+; OPT-LABEL: define void @leaf_function()
+; OPT: entry:
+; OPT-NEXT: call void @mcount()
+; OPT-NEXT: %0 = call i8* @llvm.returnaddress(i32 0)
+; OPT-NEXT: call void @__cyg_profile_func_enter(i8* bitcast (void ()* 
@leaf_function to i8*), i8* %0)
+; OPT-NEXT: %1 = call i8* @llvm.returnaddress(i32 0)
+; OPT-NEXT: call void @__cyg_profile_func_exit(i8* bitcast (void ()* 
@leaf_function to i8*), i8* %1)
+; OPT-NEXT: ret void
 }
 
 
@@ -42,6 +54,16 @@
 
 ; CHECK-NEXT: call void @__cyg_profile_func_exit(i8* bitcast (void ()* 
@root_function to i8*), i8* %3)
 ; CHECK-NEXT: ret void
+
+; OPT-LABEL: define void @root_function()
+; OPT: entry:
+; OPT-NEXT: call void @mcount()
+
+; OPT-NEXT: %0 = call i8* @llvm.returnaddress(i32 0)
+; OPT-NEXT: call void @__cyg_profile_func_enter(i8* bitcast (void ()* 
@root_function to i8*), i8* %0)
+; OPT-NEXT: %1 = call i8* @llvm.returnaddress(i32 0)
+; OPT-NEXT: call void @__cyg_profile_func_exit(i8* bitcast (void ()* 
@root_function to i8*), i8* %1)
+; OPT-NEXT: ret void
 }
 
 
Index: llvm/include/llvm/Transforms/Utils/EntryExitInstrumenter.h
===
--- llvm/include/llvm/Transforms/Utils/EntryExitInstrumenter.h
+++ llvm/include/llvm/Transforms/Utils/EntryExitInstrumenter.h
@@ -19,8 +19,6 @@
 
 namespace llvm {
 
-class Function;
-
 struct EntryExitInstrumenterPass
 : public PassInfoMixin {
   EntryExitInstrumenterPass(bool PostInlining) : PostInlining(PostInlining) {}
@@ -28,6 +26,8 @@
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
 
   bool PostInlining;
+
+  static bool isRequired() { return true; }
 };
 
 } // namespace llvm
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1312,7 +1312,9 @@
/*DropTypeTests=*/true));
   });
 
-if (Level != PassBuilder::OptimizationLevel::O0) {
+if (CodeGenOpts.InstrumentFunctions ||
+CodeGenOpts.InstrumentFunctionsAfterInlining ||
+CodeGenOpts.InstrumentFunctionEntryBare) {
   PB.registerPipelineStartEPCallback(
   [](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) {
 MPM.addPass(createModuleToFunctionPassAdaptor(


Index: llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
===
--- llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
+++ llvm/test/Transforms/EntryExitInstrumenter/mcount.ll
@@ -1,4 +1,7 @@
-; RUN: opt -passes="function(ee-instrument),cgscc(inline),function(post-inline-ee-instrument)" -S < %s | FileCheck %s
+; RUN: opt --O0 --ee-instrument --inline --post-inline-ee-instrument -S < %s | FileCheck %s
+
+; Check if the pass is enabbled on both -O0 and higher optimization levels
+; RUN: opt --O2 --ee-instrument --post-inline-ee-instrument -S < %s | FileCheck --check-prefix=OPT %s
 
 ; Running the passes twice should not result in more instrumentation.
 ; RUN: opt -passes="function(ee-instrument),function(ee-instrument),cgscc(inline),function(post-inline-ee-instrument),function(post-inline-ee-instrument)" -S < %s | FileCheck 

[clang] 71a8e4e - [MemCopyOpt] Enable MemorySSA by default

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-02-19T18:06:25+01:00
New Revision: 71a8e4e7d6b947c8b954ec0763ff7969b3879d7b

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

LOG: [MemCopyOpt] Enable MemorySSA by default

This enables use of MemorySSA instead of MemDep in MemCpyOpt. To
allow this without significant compile-time impact, the MemCpyOpt
pass is moved directly before DSE (in the cases where this was not
already the case), which allows us to reuse the existing MemorySSA
analysis.

Unlike the MemDep-based implementation, the MemorySSA-based MemCpyOpt
can also perform simple optimizations across basic blocks.

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

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll
llvm/lib/Passes/PassBuilder.cpp
llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
llvm/test/Analysis/BasicAA/phi-values-usage.ll
llvm/test/CodeGen/AMDGPU/opt-pipeline.ll
llvm/test/Other/new-pm-defaults.ll
llvm/test/Other/new-pm-lto-defaults.ll
llvm/test/Other/new-pm-thinlto-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
llvm/test/Other/opt-LTO-pipeline.ll
llvm/test/Other/opt-O2-pipeline.ll
llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
llvm/test/Other/opt-O3-pipeline.ll
llvm/test/Other/opt-Os-pipeline.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index 4c031db6734b..867203417754 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -116,9 +116,6 @@
 ; CHECK-O: Running pass: SROA on main
 ; CHECK-O: Running pass: MergedLoadStoreMotionPass on main
 ; CHECK-O: Running pass: GVN on main
-; CHECK-O: Running analysis: MemoryDependenceAnalysis on main
-; CHECK-O: Running analysis: PhiValuesAnalysis on main
-; CHECK-O: Running pass: MemCpyOptPass on main
 ; CHECK-O: Running pass: SCCPPass on main
 ; CHECK-O: Running pass: BDCEPass on main
 ; CHECK-O: Running analysis: DemandedBitsAnalysis on main
@@ -127,6 +124,7 @@
 ; CHECK-O: Running pass: CorrelatedValuePropagationPass on main
 ; CHECK-O: Running pass: ADCEPass on main
 ; CHECK-O: Running analysis: PostDominatorTreeAnalysis on main
+; CHECK-O: Running pass: MemCpyOptPass on main
 ; CHECK-O: Running pass: DSEPass on main
 ; CHECK-O: Starting {{.*}}Function pass manager run.
 ; CHECK-O: Running pass: LoopSimplifyPass on main

diff  --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 236d54b97bba..1e75690d554d 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -790,9 +790,6 @@ 
PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
   else
 FPM.addPass(GVN());
 
-  // Specially optimize memory movement as it doesn't look like dataflow in 
SSA.
-  FPM.addPass(MemCpyOptPass());
-
   // Sparse conditional constant propagation.
   // FIXME: It isn't clear why we do this *after* loop passes rather than
   // before...
@@ -818,6 +815,9 @@ 
PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
   // TODO: Investigate if this is too expensive.
   FPM.addPass(ADCEPass());
 
+  // Specially optimize memory movement as it doesn't look like dataflow in 
SSA.
+  FPM.addPass(MemCpyOptPass());
+
   FPM.addPass(DSEPass());
   FPM.addPass(createFunctionToLoopPassAdaptor(
   LICMPass(PTO.LicmMssaOptCap, PTO.LicmMssaNoAccForPromotionCap),

diff  --git a/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp 
b/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
index 068328391dff..269b82eba7ad 100644
--- a/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
+++ b/llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
@@ -472,7 +472,6 @@ void PassManagerBuilder::addFunctionSimplificationPasses(
 MPM.add(NewGVN ? createNewGVNPass()
: createGVNPass(DisableGVNLoadPRE)); // Remove redundancies
   }
-  MPM.add(createMemCpyOptPass()); // Remove memcpy / form memset
   MPM.add(createSCCPPass());  // Constant prop with SCCP
 
   if (EnableConstraintElimination)
@@ -493,6 +492,7 @@ void PassManagerBuilder::addFunctionSimplificationPasses(
   }
   MPM.add(createAggressiveDCEPass()); // Delete dead instructions
 
+  MPM.add(createMemCpyOptPass());   // Remove memcpy / form memset
   // TODO: Investigate if this is too expensive at O1.
   if (OptLevel > 1) {
 MPM.add(createDeadStoreEliminationPass());  // Delete dead stores

diff  --git a/llvm

[PATCH] D94376: [MemCpyOpt] Enable MemorySSA by default

[Nikita Popov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
nikic marked an inline comment as done.
Closed by commit rG71a8e4e7d6b9: [MemCopyOpt] Enable MemorySSA by default 
(authored by nikic).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D94376?vs=316520&id=325007#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94376

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/test/Analysis/BasicAA/phi-values-usage.ll
  llvm/test/CodeGen/AMDGPU/opt-pipeline.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Other/opt-LTO-pipeline.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll

Index: llvm/test/Other/opt-Os-pipeline.ll
===
--- llvm/test/Other/opt-Os-pipeline.ll
+++ llvm/test/Other/opt-Os-pipeline.ll
@@ -131,14 +131,10 @@
 ; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Optimization Remark Emitter
 ; CHECK-NEXT: Global Value Numbering
-; CHECK-NEXT: Phi Values Analysis
-; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
-; CHECK-NEXT: Function Alias Analysis Results
-; CHECK-NEXT: Memory Dependence Analysis
-; CHECK-NEXT: MemCpy Optimization
 ; CHECK-NEXT: Sparse Conditional Constant Propagation
 ; CHECK-NEXT: Demanded bits analysis
 ; CHECK-NEXT: Bit-Tracking Dead Code Elimination
+; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
 ; CHECK-NEXT: Function Alias Analysis Results
 ; CHECK-NEXT: Lazy Branch Probability Analysis
 ; CHECK-NEXT: Lazy Block Frequency Analysis
@@ -152,6 +148,7 @@
 ; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
 ; CHECK-NEXT: Function Alias Analysis Results
 ; CHECK-NEXT: Memory SSA
+; CHECK-NEXT: MemCpy Optimization
 ; CHECK-NEXT: Dead Store Elimination
 ; CHECK-NEXT: Natural Loop Information
 ; CHECK-NEXT: Canonicalize natural loops
Index: llvm/test/Other/opt-O3-pipeline.ll
===
--- llvm/test/Other/opt-O3-pipeline.ll
+++ llvm/test/Other/opt-O3-pipeline.ll
@@ -150,14 +150,10 @@
 ; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Optimization Remark Emitter
 ; CHECK-NEXT: Global Value Numbering
-; CHECK-NEXT: Phi Values Analysis
-; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
-; CHECK-NEXT: Function Alias Analysis Results
-; CHECK-NEXT: Memory Dependence Analysis
-; CHECK-NEXT: MemCpy Optimization
 ; CHECK-NEXT: Sparse Conditional Constant Propagation
 ; CHECK-NEXT: Demanded bits analysis
 ; CHECK-NEXT: Bit-Tracking Dead Code Elimination
+; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
 ; CHECK-NEXT: Function Alias Analysis Results
 ; CHECK-NEXT: Lazy Branch Probability Analysis
 ; CHECK-NEXT: Lazy Block Frequency Analysis
@@ -171,6 +167,7 @@
 ; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
 ; CHECK-NEXT: Function Alias Analysis Results
 ; CHECK-NEXT: Memory SSA
+; CHECK-NEXT: MemCpy Optimization
 ; CHECK-NEXT: Dead Store Elimination
 ; CHECK-NEXT: Natural Loop Information
 ; CHECK-NEXT: Canonicalize natural loops
Index: llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
===
--- llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
+++ llvm/test/Other/opt-O3-pipeline-enable-matrix.ll
@@ -150,14 +150,10 @@
 ; CHECK-NEXT: Lazy Block Frequency Analysis
 ; CHECK-NEXT: Optimization Remark Emitter
 ; CHECK-NEXT: Global Value Numbering
-; CHECK-NEXT: Phi Values Analysis
-; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
-; CHECK-NEXT: Function Alias Analysis Results
-; CHECK-NEXT: Memory Dependence Analysis
-; CHECK-NEXT: MemCpy Optimization
 ; CHECK-NEXT: Sparse Conditional Constant Propagation
 ; CHECK-NEXT: Demanded bits analysis
 ; CHECK-NEXT: Bit-Tracking Dead Code Elimination
+; CHECK-NEXT: Basic Alias Analysis (stateless AA impl)
 ; CHECK-NEXT:  

[PATCH] D97060: [OpenCL] Add ndrange builtin functions to TableGen

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added a reviewer: Anastasia.
svenvh added a project: clang.
Herald added a subscriber: yaxunl.
svenvh requested review of this revision.
Herald added a subscriber: cfe-commits.

Add ndrange builtin functions to TableGen.

Also ensure all kernel enqueue functions have CL 2.0 as minimum version.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97060

Files:
  clang/lib/Sema/OpenCLBuiltins.td
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl


Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -51,6 +51,8 @@
 typedef uint cl_mem_fence_flags;
 #define CLK_GLOBAL_MEM_FENCE 0x02
 
+typedef struct {int a;} ndrange_t;
+
 // Enable extensions that are enabled in opencl-c-base.h.
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
 #define cl_khr_subgroup_extended_types 1
@@ -88,6 +90,9 @@
 
   atomic_flag_clear(flg);
   bool result = atomic_flag_test_and_set(flg);
+
+  size_t ws[2] = {2, 8};
+  ndrange_t r = ndrange_2D(ws);
 }
 #endif
 
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -311,6 +311,7 @@
 def ReserveId : Type<"reserve_id_t", 
QualType<"Context.OCLReserveIDTy">>;
 def MemFenceFlags : TypedefType<"cl_mem_fence_flags">;
 def ClkProfilingInfo  : TypedefType<"clk_profiling_info">;
+def NDRange   : TypedefType<"ndrange_t">;
 
 // OpenCL v2.0 s6.13.11: Atomic integer and floating-point types.
 def AtomicInt : Type<"atomic_int", 
QualType<"Context.getAtomicType(Context.IntTy)">>;
@@ -1350,21 +1351,38 @@
 // Defined in Builtins.def
 
 // --- Table 33 ---
-def : Builtin<"enqueue_marker",
-[Int, Queue, UInt, PointerType, GenericAS>, 
PointerType]>;
-
-// --- Table 34 ---
-def : Builtin<"retain_event", [Void, ClkEvent]>;
-def : Builtin<"release_event", [Void, ClkEvent]>;
-def : Builtin<"create_user_event", [ClkEvent]>;
-def : Builtin<"is_valid_event", [Bool, ClkEvent]>;
-def : Builtin<"set_user_event_status", [Void, ClkEvent, Int]>;
-def : Builtin<"capture_event_profiling_info",
-[Void, ClkEvent, ClkProfilingInfo, PointerType]>;
-
-// --- Table 35 ---
-def : Builtin<"get_default_queue", [Queue]>;
-// TODO: ndrange functions
+let MinVersion = CL20 in {
+  def : Builtin<"enqueue_marker",
+  [Int, Queue, UInt, PointerType, GenericAS>, 
PointerType]>;
+
+  // --- Table 34 ---
+  def : Builtin<"retain_event", [Void, ClkEvent]>;
+  def : Builtin<"release_event", [Void, ClkEvent]>;
+  def : Builtin<"create_user_event", [ClkEvent]>;
+  def : Builtin<"is_valid_event", [Bool, ClkEvent]>;
+  def : Builtin<"set_user_event_status", [Void, ClkEvent, Int]>;
+  def : Builtin<"capture_event_profiling_info",
+  [Void, ClkEvent, ClkProfilingInfo, PointerType]>;
+
+  // --- Table 35 ---
+  def : Builtin<"get_default_queue", [Queue]>;
+
+  def : Builtin<"ndrange_1D", [NDRange, Size]>;
+  def : Builtin<"ndrange_1D", [NDRange, Size, Size]>;
+  def : Builtin<"ndrange_1D", [NDRange, Size, Size, Size]>;
+  def : Builtin<"ndrange_2D", [NDRange, PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_2D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_2D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_3D", [NDRange, PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_3D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_3D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+}
 
 
 //


Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -51,6 +51,8 @@
 typedef uint cl_mem_fence_flags;
 #define CLK_GLOBAL_MEM_FENCE 0x02
 
+typedef struct {int a;} ndrange_t;
+
 // Enable extensions that are enabled in opencl-c-base.h.
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
 #define cl_khr_subgroup_extended_types 1
@@ -88,6 +90,9 @@
 
   atomic_flag_clear(flg);
   bool result = atomic_flag_test_and_set(flg);
+
+  size_t ws[2] = {2, 8};
+  ndrange_t r = ndrange_2D(ws);
 }
 #endif
 
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/li

[PATCH] D97052: [OpenCL] Prevent adding extension pragma by default

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: clang/include/clang/Basic/OpenCLExtensions.def:24
+//its behavior explicitly is deprecated. Therefore the default
+//value if false.
 //   avail - minimum OpenCL version supporting it.

typo



Comment at: clang/include/clang/Basic/OpenCLExtensions.def:65
+OPENCL_COREFEATURE(cl_khr_global_int32_extended_atomics, true, 100, OCL_C_11P)
+OPENCL_COREFEATURE(cl_khr_local_int32_base_atomics, true, 100,  OCL_C_11P)
+OPENCL_COREFEATURE(cl_khr_local_int32_extended_atomics, true, 100, OCL_C_11P)

You're introducing a double space.



Comment at: clang/lib/Parse/ParsePragma.cpp:785
+  // Therefore, it should never be added by default.
+  Opt.acceptsPragma(Name);
 }

I fail to understand why this is needed, so perhaps this needs a bit more 
explanation.  Extensions that should continue to support pragmas already have 
their `WithPragma` field set to `true` via `OpenCLExtensions.def`.  Why do we 
need to dynamically modify the field?


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

https://reviews.llvm.org/D97052

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

[PATCH] D97053: [clang][SVE] Don't warn on vector to sizeless builtin implicit conversion

[Joe Ellis via Phabricator via cfe-commits]

joechrisellis added inline comments.



Comment at: 
clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c:1-32
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve \
+// RUN: -msve-vector-bits=512 -Wconversion -fallow-half-arguments-and-returns \
+// RUN: -o /dev/null %s 2>&1 | FileCheck %s -allow-empty
+
+// CHECK-NOT: warning
+
+#include 

c-rhodes wrote:
> I wonder if we can just add `-Wconversion` to the existing Sema tests?
> * clang/test/Sema/attr-arm-sve-vector-bits.c
> * clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
> 
`clang/test/Sema/attr-arm-sve-vector-bits.c` contains a lot of `// 
expected-error ...` and such, so I'm reluctant to make changes to that because 
it seems a little weird to mix up expected errors and non-expected warnings.

I suppose we could cat this test into `attr-arm-sve-vector-bits.cpp`, though, 
if you would prefer? 🙂 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97053

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

[PATCH] D97009: [CUDA] fix builtin constraints for PTX 7.2

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

This fixes a build error we're seeing, so I'd like to land this in a bit if 
that's OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97009

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

[PATCH] D97053: [clang][SVE] Don't warn on vector to sizeless builtin implicit conversion

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes added inline comments.



Comment at: 
clang/test/Sema/aarch64-fixed-vector-to-scalable-implicit-conversion.c:1-32
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve \
+// RUN: -msve-vector-bits=512 -Wconversion -fallow-half-arguments-and-returns \
+// RUN: -o /dev/null %s 2>&1 | FileCheck %s -allow-empty
+
+// CHECK-NOT: warning
+
+#include 

joechrisellis wrote:
> c-rhodes wrote:
> > I wonder if we can just add `-Wconversion` to the existing Sema tests?
> > * clang/test/Sema/attr-arm-sve-vector-bits.c
> > * clang/test/SemaCXX/attr-arm-sve-vector-bits.cpp
> > 
> `clang/test/Sema/attr-arm-sve-vector-bits.c` contains a lot of `// 
> expected-error ...` and such, so I'm reluctant to make changes to that 
> because it seems a little weird to mix up expected errors and non-expected 
> warnings.
> 
> I suppose we could cat this test into `attr-arm-sve-vector-bits.cpp`, though, 
> if you would prefer? 🙂 
> `clang/test/Sema/attr-arm-sve-vector-bits.c` contains a lot of `// 
> expected-error ...` and such, so I'm reluctant to make changes to that 
> because it seems a little weird to mix up expected errors and non-expected 
> warnings.
> 

I sort of see what you mean but unless there's a good reason (which I can't 
really think of) then I think it's preferential to introducing another test for 
this attribute. There's already a bunch of tests with this exact logic being 
tested here the only difference is `-Wconversion` is on.

To be honest I don't have strong feelings about it so it can stay as a separate 
test if you wish, but if it does I think this can be reduced to a single test 
for predicates and one extra test for a data vector like int8 or something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97053

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

[clang] 1a368ae - [CUDA] fix builtin constraints for PTX 7.2

[Jordan Rupprecht via cfe-commits]

Author: Artem Belevich
Date: 2021-02-19T09:57:21-08:00
New Revision: 1a368ae3b78dd7a364e8f17658fddaf86b1e98db

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

LOG: [CUDA] fix builtin constraints for PTX 7.2

This fixes build issues w/ CUDA-11 introduced by https://reviews.llvm.org/D95974

Reviewed By: yaxunl

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsNVPTX.def
clang/test/CodeGen/builtins-nvptx-sm_70.cu

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsNVPTX.def 
b/clang/include/clang/Basic/BuiltinsNVPTX.def
index 44a5e4ae01c1..b225ddcfa3fa 100644
--- a/clang/include/clang/Basic/BuiltinsNVPTX.def
+++ b/clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -38,7 +38,9 @@
 #pragma push_macro("PTX65")
 #pragma push_macro("PTX70")
 #pragma push_macro("PTX71")
-#define PTX71 "ptx71"
+#pragma push_macro("PTX72")
+#define PTX72 "ptx72"
+#define PTX71 "ptx71|" PTX72
 #define PTX70 "ptx70|" PTX71
 #define PTX65 "ptx65|" PTX70
 #define PTX64 "ptx64|" PTX65
@@ -740,3 +742,4 @@ TARGET_BUILTIN(__imma_m8n8k32_st_c_i32, "vi*iC*UiIi", "", 
AND(SM_75,PTX63))
 #pragma pop_macro("PTX65")
 #pragma pop_macro("PTX70")
 #pragma pop_macro("PTX71")
+#pragma pop_macro("PTX72")

diff  --git a/clang/test/CodeGen/builtins-nvptx-sm_70.cu 
b/clang/test/CodeGen/builtins-nvptx-sm_70.cu
index bd6b2c2b1a49..9de9a70190e2 100644
--- a/clang/test/CodeGen/builtins-nvptx-sm_70.cu
+++ b/clang/test/CodeGen/builtins-nvptx-sm_70.cu
@@ -6,6 +6,11 @@
 // RUN:-fcuda-is-device -target-feature +ptx61 -DPTX61 \
 // RUN:-S -emit-llvm -o - -x cuda %s \
 // RUN:   | FileCheck -check-prefixes=CHECK_M16,CHECK_M32_M8 %s
+// Make sure builtins still work with the latest combination of GPU & PTX.
+// RUN: %clang_cc1 -triple nvptx64-unknown-unknown -target-cpu sm_86 \
+// RUN:-fcuda-is-device -target-feature +ptx72 -DPTX61 \
+// RUN:-S -emit-llvm -o - -x cuda %s \
+// RUN:   | FileCheck -check-prefixes=CHECK_M16,CHECK_M32_M8 %s
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown -target-cpu sm_60 \
 // RUN:   -DPTX61 -fcuda-is-device -S -o /dev/null -x cuda -verify=pre-sm_70 %s
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown \



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

[PATCH] D97009: [CUDA] fix builtin constraints for PTX 7.2

[Jordan Rupprecht via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a368ae3b78d: [CUDA] fix builtin constraints for PTX 7.2 
(authored by tra, committed by rupprecht).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97009

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/test/CodeGen/builtins-nvptx-sm_70.cu


Index: clang/test/CodeGen/builtins-nvptx-sm_70.cu
===
--- clang/test/CodeGen/builtins-nvptx-sm_70.cu
+++ clang/test/CodeGen/builtins-nvptx-sm_70.cu
@@ -6,6 +6,11 @@
 // RUN:-fcuda-is-device -target-feature +ptx61 -DPTX61 \
 // RUN:-S -emit-llvm -o - -x cuda %s \
 // RUN:   | FileCheck -check-prefixes=CHECK_M16,CHECK_M32_M8 %s
+// Make sure builtins still work with the latest combination of GPU & PTX.
+// RUN: %clang_cc1 -triple nvptx64-unknown-unknown -target-cpu sm_86 \
+// RUN:-fcuda-is-device -target-feature +ptx72 -DPTX61 \
+// RUN:-S -emit-llvm -o - -x cuda %s \
+// RUN:   | FileCheck -check-prefixes=CHECK_M16,CHECK_M32_M8 %s
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown -target-cpu sm_60 \
 // RUN:   -DPTX61 -fcuda-is-device -S -o /dev/null -x cuda -verify=pre-sm_70 %s
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown \
Index: clang/include/clang/Basic/BuiltinsNVPTX.def
===
--- clang/include/clang/Basic/BuiltinsNVPTX.def
+++ clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -38,7 +38,9 @@
 #pragma push_macro("PTX65")
 #pragma push_macro("PTX70")
 #pragma push_macro("PTX71")
-#define PTX71 "ptx71"
+#pragma push_macro("PTX72")
+#define PTX72 "ptx72"
+#define PTX71 "ptx71|" PTX72
 #define PTX70 "ptx70|" PTX71
 #define PTX65 "ptx65|" PTX70
 #define PTX64 "ptx64|" PTX65
@@ -740,3 +742,4 @@
 #pragma pop_macro("PTX65")
 #pragma pop_macro("PTX70")
 #pragma pop_macro("PTX71")
+#pragma pop_macro("PTX72")


Index: clang/test/CodeGen/builtins-nvptx-sm_70.cu
===
--- clang/test/CodeGen/builtins-nvptx-sm_70.cu
+++ clang/test/CodeGen/builtins-nvptx-sm_70.cu
@@ -6,6 +6,11 @@
 // RUN:-fcuda-is-device -target-feature +ptx61 -DPTX61 \
 // RUN:-S -emit-llvm -o - -x cuda %s \
 // RUN:   | FileCheck -check-prefixes=CHECK_M16,CHECK_M32_M8 %s
+// Make sure builtins still work with the latest combination of GPU & PTX.
+// RUN: %clang_cc1 -triple nvptx64-unknown-unknown -target-cpu sm_86 \
+// RUN:-fcuda-is-device -target-feature +ptx72 -DPTX61 \
+// RUN:-S -emit-llvm -o - -x cuda %s \
+// RUN:   | FileCheck -check-prefixes=CHECK_M16,CHECK_M32_M8 %s
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown -target-cpu sm_60 \
 // RUN:   -DPTX61 -fcuda-is-device -S -o /dev/null -x cuda -verify=pre-sm_70 %s
 // RUN: %clang_cc1 -triple nvptx-unknown-unknown \
Index: clang/include/clang/Basic/BuiltinsNVPTX.def
===
--- clang/include/clang/Basic/BuiltinsNVPTX.def
+++ clang/include/clang/Basic/BuiltinsNVPTX.def
@@ -38,7 +38,9 @@
 #pragma push_macro("PTX65")
 #pragma push_macro("PTX70")
 #pragma push_macro("PTX71")
-#define PTX71 "ptx71"
+#pragma push_macro("PTX72")
+#define PTX72 "ptx72"
+#define PTX71 "ptx71|" PTX72
 #define PTX70 "ptx70|" PTX71
 #define PTX65 "ptx65|" PTX70
 #define PTX64 "ptx64|" PTX65
@@ -740,3 +742,4 @@
 #pragma pop_macro("PTX65")
 #pragma pop_macro("PTX70")
 #pragma pop_macro("PTX71")
+#pragma pop_macro("PTX72")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96825: [AArch64] Adding Neon Polynomial vadd Intrinsics

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic updated this revision to Diff 325027.
rsanthir.quic added a comment.

Windows builds were failing due to missing builtins in Intrinsics map


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

https://reviews.llvm.org/D96825

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-poly-add.c

Index: clang/test/CodeGen/aarch64-poly-add.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-poly-add.c
@@ -0,0 +1,85 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon \
+// RUN: -disable-O0-optnone -emit-llvm -o - %s | opt -S -mem2reg \
+// RUN:  | FileCheck %s
+
+#include 
+
+// CHECK-LABEL: @test_vadd_p8(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = xor <8 x i8> [[A:%.*]], [[B:%.*]]
+// CHECK-NEXT:ret <8 x i8> [[TMP0]]
+//
+poly8x8_t test_vadd_p8(poly8x8_t a, poly8x8_t b) {
+  return vadd_p8 (a, b);
+}
+
+// CHECK-LABEL: @test_vadd_p16(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <4 x i16> [[A:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <4 x i16> [[B:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <8 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <8 x i8> [[TMP2]] to <4 x i16>
+// CHECK-NEXT:ret <4 x i16> [[TMP3]]
+//
+poly16x4_t test_vadd_p16(poly16x4_t a, poly16x4_t b) {
+  return vadd_p16 (a, b);
+}
+
+// CHECK-LABEL: @test_vadd_p64(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <1 x i64> [[A:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <1 x i64> [[B:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <8 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <8 x i8> [[TMP2]] to <1 x i64>
+// CHECK-NEXT:ret <1 x i64> [[TMP3]]
+//
+poly64x1_t test_vadd_p64(poly64x1_t a, poly64x1_t b) {
+  return vadd_p64(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p8(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = xor <16 x i8> [[A:%.*]], [[B:%.*]]
+// CHECK-NEXT:ret <16 x i8> [[TMP0]]
+//
+poly8x16_t test_vaddq_p8(poly8x16_t a, poly8x16_t b){
+  return vaddq_p8(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p16(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <8 x i16> [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <8 x i16> [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to <8 x i16>
+// CHECK-NEXT:ret <8 x i16> [[TMP3]]
+//
+poly16x8_t test_vaddq_p16(poly16x8_t a, poly16x8_t b){
+  return vaddq_p16(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p64(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <2 x i64> [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <2 x i64> [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to <2 x i64>
+// CHECK-NEXT:ret <2 x i64> [[TMP3]]
+//
+poly64x2_t test_vaddq_p64(poly64x2_t a, poly64x2_t b){
+  return vaddq_p64(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p128(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast i128 [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast i128 [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to i128
+// CHECK-NEXT:ret i128 [[TMP3]]
+//
+poly128_t test_vaddq_p128 (poly128_t a, poly128_t b){
+  return vaddq_p128(a, b);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -5368,7 +5368,10 @@
   NEONMAP2(vabdq_v, arm_neon_vabdu, arm_neon_vabds, Add1ArgType | UnsignedAlts),
   NEONMAP1(vabs_v, arm_neon_vabs, 0),
   NEONMAP1(vabsq_v, arm_neon_vabs, 0),
+  NEONMAP0(vadd_v),
   NEONMAP0(vaddhn_v),
+  NEONMAP0(vaddq_p128),
+  NEONMAP0(vaddq_v),
   NEONMAP1(vaesdq_v, arm_neon_aesd, 0),
   NEONMAP1(vaeseq_v, arm_neon_aese, 0),
   NEONMAP1(vaesimcq_v, arm_neon_aesimc, 0),
@@ -5662,7 +5665,10 @@
   NEONMAP0(splatq_laneq_v),
   NEONMAP1(vabs_v, aarch64_neon_abs, 0),
   NEONMAP1(vabsq_v, aarch64_neon_abs, 0),
+  NEONMAP0(vadd_v),
   NEONMAP0(vaddhn_v),
+  NEONMAP0(vaddq_p128),
+  NEONMAP0(vaddq_v),
   NEONMAP1(vaesdq_v, aarch64_crypto_aesd, 0),
   NEONMAP1(vaeseq_v, aarch64_crypto_aese, 0),
   NEONMAP1(vaesimcq_v, aarch64_crypto_aesimc, 0),
@@ -6290,6 +6296,14 @@
 if (VTy->getElementType()->isFloatingPointTy())
   return EmitNeonCall(CGM.getIntrinsic(Intrinsic::fabs, Ty), Ops, "vabs");
 return EmitNeonCall(CGM.getIntrinsic(LLVMIntrinsic, Ty), Ops, "vabs");
+  case NEON::BI__builtin_neon_vadd_v:
+  case NEON::BI__bui

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

[David Li via Phabricator via cfe-commits]

davidxl added a comment.

So in this case, the virtual call in user code is resolved to a strong 
definition in the shared library even though user code also has a derived class 
defined? In other words, the library provides another overrider definition as 
the final overrider?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96919

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

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 325029.
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

Updating based on review feedback.


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

https://reviews.llvm.org/D95396

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Parser/static_assert.c
  clang/test/Preprocessor/static_assert.c
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/static-assert.cpp

Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -triple=x86_64-linux-gnu
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -pedantic -triple=x86_64-linux-gnu
 
 int f(); // expected-note {{declared here}}
 
Index: clang/test/Sema/static-assert.c
===
--- clang/test/Sema/static-assert.c
+++ clang/test/Sema/static-assert.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -std=c11 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fms-compatibility -DMS -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fms-compatibility -DMS -fsyntax-only -verify=expected,ms %s
 // RUN: %clang_cc1 -std=c99 -pedantic -fsyntax-only -verify=expected,ext %s
 // RUN: %clang_cc1 -xc++ -std=c++11 -pedantic -fsyntax-only -verify=expected,ext,cxx %s
 
@@ -13,7 +13,7 @@
// ext-warning {{'_Static_assert' is a C11 extension}}
 
 #ifdef MS
-static_assert(1, "1 is nonzero");
+static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without inclusion of  is a Microsoft extension}}
 #endif
 
 void foo(void) {
@@ -21,7 +21,7 @@
   _Static_assert(0, "0 is nonzero"); // expected-error {{static_assert failed "0 is nonzero"}} \
  // ext-warning {{'_Static_assert' is a C11 extension}}
 #ifdef MS
-  static_assert(1, "1 is nonzero");
+  static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without}}
 #endif
 }
 
@@ -34,7 +34,7 @@
   _Static_assert(0, "0 is nonzero"); // expected-error {{static_assert failed "0 is nonzero"}} \
  // ext-warning {{'_Static_assert' is a C11 extension}}
 #ifdef MS
-  static_assert(1, "1 is nonzero");
+  static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without}}
 #endif
 };
 
@@ -58,3 +58,15 @@
// ext-warning 3 {{'_Static_assert' is a C11 extension}}
 typedef UNION(float, 0.5f) U4; // expected-error {{expected a type}} \
// ext-warning 3 {{'_Static_assert' is a C11 extension}}
+
+// After defining the assert macro in MS-compatibility mode, we should
+// no longer warn about including  under the assumption the
+// user already did that.
+#ifdef MS
+#define assert(expr)
+static_assert(1, "1 is nonzero"); // ok
+
+// But we should still warn if the user did something bonkers.
+#undef static_assert
+static_assert(1, "1 is nonzero"); // ms-warning {{use of 'static_assert' without}}
+#endif
Index: clang/test/Preprocessor/static_assert.c
===
--- /dev/null
+++ clang/test/Preprocessor/static_assert.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -E -dM %s | FileCheck --strict-whitespace --check-prefix=NOMS %s
+// RUN: %clang_cc1 -fms-compatibility -E -dM %s | FileCheck --strict-whitespace --check-prefix=MS %s
+
+// If the assert macro is defined in MS compatibility mode in C, we
+// automatically inject a macro definition for static_assert. Test that the
+// macro is properly added to the preprocessed output. This allows us to
+// diagonse use of the static_assert keyword when  has not been
+// included while still being able to compile preprocessed code.
+#define assert
+
+MS: #define static_assert _Static_assert
+NOMS-NOT: #define static_assert _Static_assert
Index: clang/test/Parser/static_assert.c
===
--- /dev/null
+++ clang/test/Parser/static_assert.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -verify=c2x %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -fms-compatibility -verify=c2x-ms %s
+// RUN: %clang_cc1 -fsyntax-only -std=c2x -Wpre-c2x-compat -verify=c2x-compat %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -verify=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c99 -pedantic -verify=c99-pedantic %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify=cxx17 -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -pedantic -verify=cxx17-pedantic -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -verify=cxx98 -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++98 -pedantic -verify=cxx98-pe

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:2884
+Tok.setKind(tok::kw__Static_assert);
+Tok.setIdentifierInfo(getIdentifierInfo("_Static_assert"));
+MI->AddTokenToBody(Tok);

rsmith wrote:
> Do we need to give the expansion token a source location? What do diagnostics 
> pointing at this token look like?
Given:
```
#define assert

// This is a comment
int i = 0;
static_assert(0, "test");
```
I get:
```
F:\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only "F:\Aaron B
allman\Desktop\test.c"
F:\Aaron Ballman\Desktop\test.c:5:1: error: static_assert failed "test"
static_assert(0, "test");
^ ~
F:\Aaron Ballman\Desktop\test.c:5:1: note: expanded from macro 'static_assert'
static_assert(0, "test");
^
1 error generated.
```
which is a bit funky due to the note. However, I'm not certain what source 
location to give it. If I give it the location of the `assert` identifier, then 
I get output like:
```
F:\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only "F:\Aaron B
allman\Desktop\test.c"
F:\Aaron Ballman\Desktop\test.c:5:1: error: static_assert failed "test"
static_assert(0, "test");
^ ~
F:\Aaron Ballman\Desktop\test.c:1:9: note: expanded from macro 'static_assert'
#define assert
^
1 error generated.
```
which seems even more mysterious.


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

https://reviews.llvm.org/D95396

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

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D96919#2575037 , @davidxl wrote:

> So in this case, the virtual call in user code is resolved to a strong 
> definition in the shared library even though user code also has a derived 
> class defined? In other words, the library provides another overrider 
> definition as the final overrider?

Yes. There is already support via LTO and the linkers to be conservative when a 
vtable is defined in a shared library. But without the type metadata on the 
available_externally vtables, LTO doesn't even know that the derived class 
vtable is related to the base class vtable from the shared library. So we 
erroneously think that there is only one possible implementation of the virtual 
method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96919

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

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

[David Li via Phabricator via cfe-commits]

davidxl added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1770
 
-  if (!VTable->isDeclarationForLinker())
+  if (!VTable->isDeclarationForLinker() ||
+  CGM.getCodeGenOpts().WholeProgramVTables) {

Can you add some comment here -- also help user understand the code. Note that 
isDeclarationForLinker is not intuitive by name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96919

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

[PATCH] D97052: [OpenCL] Prevent adding extension pragma by default

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added inline comments.



Comment at: clang/include/clang/Basic/OpenCLExtensions.def:71
+OPENCL_EXTENSION(cl_khr_int64_extended_atomics, true, 100)
+OPENCL_COREFEATURE(cl_khr_3d_image_writes, true, 100, OCL_C_20)
 

I think core and optional core features do not require pragma too. So for 
example 3d image writes or fp64 do not require pragma in OpenCL C 2.0. Isn't 
that so?



Comment at: clang/test/SemaOpenCL/extension-version.cl:217
+
+// Check that pragmas for the OpenCL 3.0 features are rejected.
+

Again about core or optional core: don't you feel that current diagnostic is 
good already (https://godbolt.org/z/Kcjdvr)?


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

https://reviews.llvm.org/D97052

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

[PATCH] D85176: [Coverage] Emit gap region after conditions when macro is present.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added a comment.

In D85176#2572661 , @saugustine wrote:

> This commit introduced an unused variable warning when built without asserts. 
> (CoverageMappingGen.cpp:984) I have fixed it with  rG4544a63b7705 
> .

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

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

[PATCH] D96825: [AArch64] Adding Neon Polynomial vadd Intrinsics

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic updated this revision to Diff 325034.
rsanthir.quic added a comment.

rebased on main


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

https://reviews.llvm.org/D96825

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-poly-add.c

Index: clang/test/CodeGen/aarch64-poly-add.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-poly-add.c
@@ -0,0 +1,85 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -target-feature +neon \
+// RUN: -disable-O0-optnone -emit-llvm -o - %s | opt -S -mem2reg \
+// RUN:  | FileCheck %s
+
+#include 
+
+// CHECK-LABEL: @test_vadd_p8(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = xor <8 x i8> [[A:%.*]], [[B:%.*]]
+// CHECK-NEXT:ret <8 x i8> [[TMP0]]
+//
+poly8x8_t test_vadd_p8(poly8x8_t a, poly8x8_t b) {
+  return vadd_p8 (a, b);
+}
+
+// CHECK-LABEL: @test_vadd_p16(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <4 x i16> [[A:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <4 x i16> [[B:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <8 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <8 x i8> [[TMP2]] to <4 x i16>
+// CHECK-NEXT:ret <4 x i16> [[TMP3]]
+//
+poly16x4_t test_vadd_p16(poly16x4_t a, poly16x4_t b) {
+  return vadd_p16 (a, b);
+}
+
+// CHECK-LABEL: @test_vadd_p64(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <1 x i64> [[A:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <1 x i64> [[B:%.*]] to <8 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <8 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <8 x i8> [[TMP2]] to <1 x i64>
+// CHECK-NEXT:ret <1 x i64> [[TMP3]]
+//
+poly64x1_t test_vadd_p64(poly64x1_t a, poly64x1_t b) {
+  return vadd_p64(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p8(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = xor <16 x i8> [[A:%.*]], [[B:%.*]]
+// CHECK-NEXT:ret <16 x i8> [[TMP0]]
+//
+poly8x16_t test_vaddq_p8(poly8x16_t a, poly8x16_t b){
+  return vaddq_p8(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p16(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <8 x i16> [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <8 x i16> [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to <8 x i16>
+// CHECK-NEXT:ret <8 x i16> [[TMP3]]
+//
+poly16x8_t test_vaddq_p16(poly16x8_t a, poly16x8_t b){
+  return vaddq_p16(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p64(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast <2 x i64> [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast <2 x i64> [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to <2 x i64>
+// CHECK-NEXT:ret <2 x i64> [[TMP3]]
+//
+poly64x2_t test_vaddq_p64(poly64x2_t a, poly64x2_t b){
+  return vaddq_p64(a, b);
+}
+
+// CHECK-LABEL: @test_vaddq_p128(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = bitcast i128 [[A:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP1:%.*]] = bitcast i128 [[B:%.*]] to <16 x i8>
+// CHECK-NEXT:[[TMP2:%.*]] = xor <16 x i8> [[TMP0]], [[TMP1]]
+// CHECK-NEXT:[[TMP3:%.*]] = bitcast <16 x i8> [[TMP2]] to i128
+// CHECK-NEXT:ret i128 [[TMP3]]
+//
+poly128_t test_vaddq_p128 (poly128_t a, poly128_t b){
+  return vaddq_p128(a, b);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -5371,7 +5371,10 @@
   NEONMAP2(vabdq_v, arm_neon_vabdu, arm_neon_vabds, Add1ArgType | UnsignedAlts),
   NEONMAP1(vabs_v, arm_neon_vabs, 0),
   NEONMAP1(vabsq_v, arm_neon_vabs, 0),
+  NEONMAP0(vadd_v),
   NEONMAP0(vaddhn_v),
+  NEONMAP0(vaddq_p128),
+  NEONMAP0(vaddq_v),
   NEONMAP1(vaesdq_v, arm_neon_aesd, 0),
   NEONMAP1(vaeseq_v, arm_neon_aese, 0),
   NEONMAP1(vaesimcq_v, arm_neon_aesimc, 0),
@@ -5665,7 +5668,10 @@
   NEONMAP0(splatq_laneq_v),
   NEONMAP1(vabs_v, aarch64_neon_abs, 0),
   NEONMAP1(vabsq_v, aarch64_neon_abs, 0),
+  NEONMAP0(vadd_v),
   NEONMAP0(vaddhn_v),
+  NEONMAP0(vaddq_p128),
+  NEONMAP0(vaddq_v),
   NEONMAP1(vaesdq_v, aarch64_crypto_aesd, 0),
   NEONMAP1(vaeseq_v, aarch64_crypto_aese, 0),
   NEONMAP1(vaesimcq_v, aarch64_crypto_aesimc, 0),
@@ -6302,6 +6308,14 @@
 if (VTy->getElementType()->isFloatingPointTy())
   return EmitNeonCall(CGM.getIntrinsic(Intrinsic::fabs, Ty), Ops, "vabs");
 return EmitNeonCall(CGM.getIntrinsic(LLVMIntrinsic, Ty), Ops, "vabs");
+  case NEON::BI__builtin_neon_vadd_v:
+  case NEON::BI__builtin_neon_vaddq_v: {
+llvm::Type *VTy = llvm::Fixe

[PATCH] D50106: [libc++] Fix tuple assignment from types derived from a tuple-like

[Louis Dionne via Phabricator via cfe-commits]

ldionne updated this revision to Diff 325039.
ldionne added a comment.

Fix GCC issues. I swear I had done the same fix locally previously, but I must
have messed something up with Git or `arc diff`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50106

Files:
  libcxx/include/tuple
  
libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.assign/const_array.pass.cpp
  
libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.assign/rvalue_array.pass.cpp
  
libcxx/test/libcxx/utilities/tuple/tuple.tuple/tuple.assign/tuple_array_template_depth.pass.cpp
  libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/const_pair.pass.cpp
  libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/convert_copy.pass.cpp
  libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/convert_move.pass.cpp
  libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/copy.pass.cpp
  
libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/derived_from_tuple_like.pass.cpp
  libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/laziness.pass.cpp
  libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/move.pass.cpp
  libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/move_pair.pass.cpp
  
libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/tuple_array_template_depth.pass.cpp
  libcxx/test/support/propagate_value_category.hpp

Index: libcxx/test/support/propagate_value_category.hpp
===
--- /dev/null
+++ libcxx/test/support/propagate_value_category.hpp
@@ -0,0 +1,153 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef TEST_SUPPORT_PROPAGATE_VALUE_CATEGORY
+#define TEST_SUPPORT_PROPAGATE_VALUE_CATEGORY
+
+#include "test_macros.h"
+#include 
+
+#if TEST_STD_VER < 11
+#error this header may only be used in C++11
+#endif
+
+using UnderlyingVCType = unsigned;
+enum ValueCategory : UnderlyingVCType {
+  VC_None = 0,
+  VC_LVal = 1 << 0,
+  VC_RVal = 1 << 1,
+  VC_Const = 1 << 2,
+  VC_Volatile = 1 << 3,
+  VC_ConstVolatile = VC_Const | VC_Volatile
+};
+
+inline constexpr ValueCategory operator&(ValueCategory LHS, ValueCategory RHS) {
+  return ValueCategory(LHS & (UnderlyingVCType)RHS);
+}
+
+inline constexpr ValueCategory operator|(ValueCategory LHS, ValueCategory RHS) {
+  return ValueCategory(LHS | (UnderlyingVCType)RHS);
+}
+
+inline constexpr ValueCategory operator^(ValueCategory LHS, ValueCategory RHS) {
+  return ValueCategory(LHS ^ (UnderlyingVCType)RHS);
+}
+
+inline constexpr bool isValidValueCategory(ValueCategory VC) {
+  return (VC & (VC_LVal | VC_RVal)) != (VC_LVal | VC_RVal);
+}
+
+inline constexpr bool hasValueCategory(ValueCategory Arg, ValueCategory Key) {
+  return Arg == Key || ((Arg & Key) == Key);
+}
+
+template 
+using UnCVRef =
+typename std::remove_cv::type>::type;
+
+template 
+constexpr ValueCategory getReferenceQuals() {
+  return std::is_lvalue_reference::value
+ ? VC_LVal
+ : (std::is_rvalue_reference::value ? VC_RVal : VC_None);
+}
+static_assert(getReferenceQuals() == VC_None, "");
+static_assert(getReferenceQuals() == VC_LVal, "");
+static_assert(getReferenceQuals() == VC_RVal, "");
+
+template 
+constexpr ValueCategory getCVQuals() {
+  using Vp = typename std::remove_reference::type;
+  return std::is_const::value && std::is_volatile::value
+ ? VC_ConstVolatile
+ : (std::is_const::value
+? VC_Const
+: (std::is_volatile::value ? VC_Volatile : VC_None));
+}
+static_assert(getCVQuals() == VC_None, "");
+static_assert(getCVQuals() == VC_Const, "");
+static_assert(getCVQuals() == VC_Volatile, "");
+static_assert(getCVQuals() == VC_ConstVolatile, "");
+static_assert(getCVQuals() == VC_None, "");
+static_assert(getCVQuals() == VC_Const, "");
+
+template 
+inline constexpr ValueCategory getValueCategory() {
+  return getReferenceQuals() | getCVQuals();
+}
+static_assert(getValueCategory() == VC_None, "");
+static_assert(getValueCategory() == (VC_LVal | VC_Const), "");
+static_assert(getValueCategory() ==
+  (VC_RVal | VC_ConstVolatile),
+  "");
+
+template 
+struct ApplyValueCategory {
+private:
+  static_assert(isValidValueCategory(VC), "");
+
+  template 
+  using CondT = typename std::conditional::type;
+
+public:
+  template >
+  using ApplyCVQuals = CondT<
+  hasValueCategory(VC, VC_ConstVolatile), typename std::add_cv::type,
+  CondT::type,
+CondT::type, Tp>>>;
+
+  template ::type>
+  using ApplyReferenceQuals =
+  CondT::type,
+CondT::type, Vp>>;
+
+  template 

[PATCH] D95017: [clang-format] Add case aware include sorting.

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D95017#2572578 , @PragmaNull wrote:

> In D95017#2572238 , @curdeius wrote:
>
>> Do you have an idea for better names?
>> I see that e.g. MS documentation uses ascending order and case-sensitive 
>> order.
>
> I'm okay with the names, it just seems to me that they are reversed, where 
> CaseSensitive should sort using ASCIIbetic order, and CaseInsensitive should 
> ignore case entirely in the same way that most case-insensitive string 
> compares would.

After initially reading it I was not sure about it, but now I have looked into 
the wikipedia:

> In computers, case sensitivity defines whether uppercase and lowercase 
> letters are treated as distinct (case-sensitive) or equivalent 
> (case-insensitive).

So I would support a name switch. I would wait a few days for @kentsommer to 
say something, if he doesn't respond just make a diff and add me as reviewer. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95017

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

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 325041.
tejohnson added a comment.

Add comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96919

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/type-metadata.cpp


Index: clang/test/CodeGenCXX/type-metadata.cpp
===
--- clang/test/CodeGenCXX/type-metadata.cpp
+++ clang/test/CodeGenCXX/type-metadata.cpp
@@ -7,6 +7,7 @@
 // Tests for the whole-program-vtables feature:
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility 
hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck 
--check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM %s
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux 
-fwhole-program-vtables -emit-llvm -o - %s | FileCheck 
--check-prefix=VTABLE-OPT --check-prefix=ITANIUM-DEFAULTVIS 
--check-prefix=TT-ITANIUM %s
+// RUN: %clang_cc1 -O2 -flto -flto-unit -triple x86_64-unknown-linux 
-fwhole-program-vtables -emit-llvm -o - %s | FileCheck 
--check-prefix=ITANIUM-OPT %s
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc 
-fwhole-program-vtables -emit-llvm -o - %s | FileCheck 
--check-prefix=VTABLE-OPT --check-prefix=MS --check-prefix=TT-MS %s
 
 // Tests for cfi + whole-program-vtables:
@@ -79,6 +80,13 @@
 // ITANIUM-DIAG-SAME: !type [[ALL16]]
 // ITANIUM-SAME: !type [[FAF16:![0-9]+]]
 
+// ITANIUM: @_ZTVN5test31EE = external unnamed_addr constant
+// ITANIUM-DEFAULTVIS: @_ZTVN5test31EE = external unnamed_addr constant
+// ITANIUM-OPT: @_ZTVN5test31EE = available_externally unnamed_addr constant 
{{[^!]*}},
+// ITANIUM-OPT-SAME: !type [[E16:![0-9]+]],
+// ITANIUM-OPT-SAME: !type [[EF16:![0-9]+]]
+// ITANIUM-OPT: @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast 
({ [3 x i8*] }* @_ZTVN5test31EE to i8*)]
+
 // MS: comdat($"??_7A@@6B@"), !type [[A8:![0-9]+]]
 // MS: comdat($"??_7B@@6B0@@"), !type [[B8:![0-9]+]]
 // MS: comdat($"??_7B@@6BA@@@"), !type [[A8]]
@@ -253,6 +261,20 @@
 
 }
 
+namespace test3 {
+// All virtual functions are outline, so we can assume that it will
+// be generated in translation unit where foo is defined.
+struct E {
+  virtual void foo();
+};
+
+void g() {
+  E e;
+  e.foo();
+}
+
+}  // Test9
+
 // ITANIUM: [[A16]] = !{i64 16, !"_ZTS1A"}
 // ITANIUM-DIAG: [[ALL16]] = !{i64 16, !"all-vtables"}
 // ITANIUM: [[AF16]] = !{i64 16, !"_ZTSM1AFvvE.virtual"}
@@ -286,6 +308,9 @@
 // ITANIUM: [[FAF16]] = !{i64 16, [[FAF_ID:![0-9]+]]}
 // ITANIUM: [[FAF_ID]] = distinct !{}
 
+// ITANIUM-OPT: [[E16]] = !{i64 16, !"_ZTSN5test31EE"}
+// ITANIUM-OPT: [[EF16]] = !{i64 16, !"_ZTSMN5test31EEFvvE.virtual"}
+
 // MS: [[A8]] = !{i64 8, !"?AUA@@"}
 // MS: [[B8]] = !{i64 8, !"?AUB@@"}
 // MS: [[D8]] = !{i64 8, [[D_ID:![0-9]+]]}
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1767,8 +1767,20 @@
   DC->getParent()->isTranslationUnit())
 EmitFundamentalRTTIDescriptors(RD);
 
-  if (!VTable->isDeclarationForLinker())
+  // Always emit type metadata on non-available_externally definitions, and on
+  // available_externally definitions if we are performing whole program
+  // devirtualization. For WPD we need the type metadata on all vtable
+  // definitions to ensure we associate derived classes with base classes
+  // defined in headers but with a strong definition only in a shared library.
+  if (!VTable->isDeclarationForLinker() ||
+  CGM.getCodeGenOpts().WholeProgramVTables) {
 CGM.EmitVTableTypeMetadata(RD, VTable, VTLayout);
+// For available_externally definitions, add the vtable to
+// @llvm.compiler.used so that it isn't deleted before whole program
+// analysis.
+if (VTable->isDeclarationForLinker())
+  CGM.addCompilerUsedGlobal(VTable);
+  }
 
   if (VTContext.isRelativeLayout() && !VTable->isDSOLocal())
 CGVT.GenerateRelativeVTableAlias(VTable, VTable->getName());


Index: clang/test/CodeGenCXX/type-metadata.cpp
===
--- clang/test/CodeGenCXX/type-metadata.cpp
+++ clang/test/CodeGenCXX/type-metadata.cpp
@@ -7,6 +7,7 @@
 // Tests for the whole-program-vtables feature:
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=TT-ITANIUM %s
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM-DEFAULTVIS --check-prefix=TT-ITANIUM %s
+// RUN: %clang_cc1 -O2 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM-OPT %

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

[David Li via Phabricator via cfe-commits]

davidxl added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1781
+// analysis.
+if (VTable->isDeclarationForLinker())
+  CGM.addCompilerUsedGlobal(VTable);

Is it better to guard it with with if 
(CGM.getCodeGenOpts().WholeProgramVTables)  which seems clearer in intention?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96919

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

[PATCH] D78652: [clang-tidy] Suppress reports to similarly used parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity updated this revision to Diff 325036.
whisperity retitled this revision from "[clang-tidy] Add "ignore related 
parameters" heuristics to 
'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type'" to 
"[clang-tidy] Suppress reports to similarly used parameters in 
'bugprone-easily-swappable-parameters'".
whisperity edited the summary of this revision.
whisperity added a comment.

- Rebased over updated, rewritten, newer base patch version.
- Added the heuristic for //"same member access"// to the patch (which was so 
far only on my development branch).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78652

Files:
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
@@ -0,0 +1,185 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1} \
+// RUN:  ]}' --
+
+namespace std {
+template 
+T max(const T &A, const T &B);
+} // namespace std
+
+bool coin();
+void f(int);
+void g(int);
+void h(int, int);
+void i(int, bool);
+void i(int, char);
+
+struct Tmp {
+  int f(int);
+  int g(int, int);
+};
+
+struct Int {
+  int I;
+};
+
+void compare(int Left, int Right) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 2 adjacent parameters of 'compare' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:18: note: the first parameter in the range is 'Left'
+// CHECK-MESSAGES: :[[@LINE-3]]:28: note: the last parameter in the range is 'Right'
+
+int decideSequence(int A, int B) {
+  if (A)
+return 1;
+  if (B)
+return 2;
+  return 3;
+}
+// CHECK-MESSAGES: :[[@LINE-7]]:20: warning: 2 adjacent parameters of 'decideSequence' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-8]]:24: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-9]]:31: note: the last parameter in the range is 'B'
+
+int myMax(int A, int B) { // NO-WARN: Appears in same expression.
+  return A < B ? A : B;
+}
+
+int myMax2(int A, int B) { // NO-WARN: Appears in same expression.
+  if (A < B)
+return A;
+  return B;
+}
+
+int myMax3(int A, int B) { // NO-WARN: Appears in same expression.
+  return std::max(A, B);
+}
+
+int binaryToUnary(int A, int) {
+  return A;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:19: warning: 2 adjacent parameters of 'binaryToUnary' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-4]]:23: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-5]]:29: note: the last parameter in the range is ''
+
+int randomReturn1(int A, int B) { // NO-WARN: Appears in same expression.
+  return coin() ? A : B;
+}
+
+int randomReturn2(int A, int B) { // NO-WARN: Both parameters returned.
+  if (coin())
+return A;
+  return B;
+}
+
+int randomReturn3(int A, int B) { // NO-WARN: Both parameters returned.
+  bool Flip = coin();
+  if (Flip)
+return A;
+  Flip = coin();
+  if (Flip)
+return B;
+  Flip = coin();
+  if (!Flip)
+return 0;
+  return -1;
+}
+
+void passthrough1(int A, int B) { // WARN: Different functions, different params.
+  f(A);
+  g(B);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:19: warning: 2 adjacent parameters of 'passthrough1' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-5]]:23: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-6]]:30: note: the last parameter in the range is 'B'
+
+void passthrough2(int A, int B) { // NO-WARN: Passed to same index of same function.
+  f(A);
+  f(B);
+}
+
+void passthrough3(int A, int B) { // NO-WARN: Passed to same index of same funtion.
+  h(1, A);
+  h(1, B);
+}
+
+void passthrough4(int A, int B) { // WARN: Different index used.
+  h(1, A);
+  h(B, 2);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:19: warning: 2 adjacent parameters of 'passt

[PATCH] D96906: [AMDGPU] gfx90a support

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/include/clang/Driver/Options.td:3097-3101
+def mtgsplit : Flag<["-"], "mtgsplit">, Group,
+  HelpText<"Enable threadgroup split execution mode (AMDGPU only)">;
+def mno_tgsplit : Flag<["-"], "mno-tgsplit">, Group,
+  HelpText<"Disable threadgroup split execution mode (AMDGPU only)">;
+

kzhuravl wrote:
> rampitec wrote:
> > kzhuravl wrote:
> > > kzhuravl wrote:
> > > > kzhuravl wrote:
> > > > > rampitec wrote:
> > > > > > kzhuravl wrote:
> > > > > > > kzhuravl wrote:
> > > > > > > > tra wrote:
> > > > > > > > > We have `BoolFOption` to generate `-fsomething` and 
> > > > > > > > > `-fno-something`
> > > > > > > > The reason we use m_amdgpu_Features_Group is it is getting 
> > > > > > > > transformed to target features (-mtgsplit to +tgsplit, 
> > > > > > > > mno-tgsplit to -tgsplit. For example, tgsplit target feature in 
> > > > > > > > AMDGPU backend:
> > > > > > > > 
> > > > > > > > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AMDGPU/AMDGPU.td#L157
> > > > > > > > 
> > > > > > > > Does BoolFOption get translated to target features as well?
> > > > > > > Quickly glancing over Options.td, BoolFOption is in f_group, and 
> > > > > > > does not get automatically converted to target-features
> > > > > > We could probably create similar BoolMOption. This is not only 
> > > > > > tgsplit, there are plenty of such binary options around.
> > > > > agreed, seems like a good choice given there is BoolFOption, 
> > > > > BoolGOption
> > > > but it will still need to be in m_amdgpu_Features_Group because 
> > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/AMDGPU.cpp#L403
> > > >  unless we want to switch away from that. so maybe make a group an 
> > > > optional, last template parameter to BoolMOption?
> > > Other targets also have its own corresponding groups: 
> > > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Driver/Options.td#L151
> > Looks like. It is not just the same thing as BoolFOption or BoolGOption.
> @tra , do you have any suggestion based on the comments above? thanks.
I have created new BoolMOption here: D97069. Not sure if it saves much code but 
let's see if we like it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96906

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

[PATCH] D96919: [clang] Emit type metadata on available_externally vtables for WPD

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:1781
+// analysis.
+if (VTable->isDeclarationForLinker())
+  CGM.addCompilerUsedGlobal(VTable);

davidxl wrote:
> Is it better to guard it with with if 
> (CGM.getCodeGenOpts().WholeProgramVTables)  which seems clearer in intention?
No I don't think we want to do this, it would mean every single vtable would be 
put on the llvm.compiler.used which is unnecessary. The available_externally 
are the ones that aren't sticking around otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96919

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

[PATCH] D97052: [OpenCL] Prevent adding extension pragma by default

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/include/clang/Basic/OpenCLExtensions.def:71
+OPENCL_EXTENSION(cl_khr_int64_extended_atomics, true, 100)
+OPENCL_COREFEATURE(cl_khr_3d_image_writes, true, 100, OCL_C_20)
 

azabaznov wrote:
> I think core and optional core features do not require pragma too. So for 
> example 3d image writes or fp64 do not require pragma in OpenCL C 2.0. Isn't 
> that so?
Well to be honest nothing seems to need pragma but we have to accept it for the 
backward compatibility. :)

In case of the core features if pragma would have been useful we would have to 
still accept it for the backward compatibility even if the feature became core.



Comment at: clang/lib/Parse/ParsePragma.cpp:785
+  // Therefore, it should never be added by default.
+  Opt.acceptsPragma(Name);
 }

svenvh wrote:
> I fail to understand why this is needed, so perhaps this needs a bit more 
> explanation.  Extensions that should continue to support pragmas already have 
> their `WithPragma` field set to `true` via `OpenCLExtensions.def`.  Why do we 
> need to dynamically modify the field?
It is a bit twisty here to be honest. Because we have also introduced the 
pragma `begin` and `end` that would add pragma `enable`/`disable` by default. 
So any extension added dynamically using `begin`/`end` would have to accept the 
pragma `enable`/`disable`. 

https://clang.llvm.org/docs/UsersManual.html#opencl-extensions

But in the subsequent patches, I hope to remove this because I just don't see 
where it is useful but it is very confusing.



Comment at: clang/test/SemaOpenCL/extension-version.cl:217
+
+// Check that pragmas for the OpenCL 3.0 features are rejected.
+

azabaznov wrote:
> Again about core or optional core: don't you feel that current diagnostic is 
> good already (https://godbolt.org/z/Kcjdvr)?
First of all with current approach, clang will only provide the diagnostic if 
an extra warning flag is passed. By default it accepts the pragma silently.

Secondly, to provide consistency clang will either need to know all of the 
features in the standard even if they are added in the libraries or otherwise, 
it will provide inconsistent diagnostics - for features added into clang we 
provide this diagnostic but for the others, we provide the diagnostics that you 
refer to. I appreciate that we already have the behavior very inconsistent but 
it is not great.

Another aspect is that the diagnostic we have currently indicates that the 
pragma is somehow legal (known) and might do something but in fact we never 
intended it to exist and never intended that frontend would do something with 
it. So I prefer to have consistency here with the regular diagnostics about the 
pragmas and indicate that the pragma is unknown. This just makes things simpler 
for everyone and avoids redundancy in the language too.

FYI this change is generally driving towards deprecation of the extension 
pragmas overall.


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

https://reviews.llvm.org/D97052

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

[PATCH] D96381: [AArch64] Adding SHA3 Intrinsics support

[Ryan Santhirarajan via Phabricator via cfe-commits]

rsanthir.quic added a comment.

Thank you for reviewing this @DavidSpickett ! If you get a chance could you 
commit this for me? I do not have commit access yet.


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

https://reviews.llvm.org/D96381

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