[clang] [Clang] Don't diagnose VLA for `-std=gnu++*` by default (PR #89943)

[Kolya Panchenko via cfe-commits]

https://github.com/nikolaypanchenko created 
https://github.com/llvm/llvm-project/pull/89943

The changeset aims to keep Clang's behavior prior to `7339c0f` and to match 
current behavior in GCC until GCC community decides what should be default: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848

>From c9bdaf6644894a5a42d68d0ca4d82766a7cfa928 Mon Sep 17 00:00:00 2001
From: Kolya Panchenko 
Date: Wed, 24 Apr 2024 08:47:03 -0700
Subject: [PATCH] [Clang] Don't diagnose VLA for `-std=gnu++*` by default

The changeset aims to keep Clang's behavior prior to `7339c0f` and to match
current behavior in GCC until GCC community decides what should be
default: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848
---
 clang/include/clang/Basic/DiagnosticGroups.td| 3 ++-
 clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++--
 clang/test/Sema/vla-ext.c| 1 +
 clang/test/SemaCXX/c99-variable-length-array.cpp | 1 +
 clang/test/SemaCXX/coroutine-vla.cpp | 1 +
 5 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 60f87da2a7387c..e85a09e7d00beb 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -870,7 +870,8 @@ def VectorConversion : DiagGroup<"vector-conversion">;  
// clang specific
 def VexingParse : DiagGroup<"vexing-parse">;
 def VLAUseStaticAssert : DiagGroup<"vla-extension-static-assert">;
 def VLACxxExtension : DiagGroup<"vla-cxx-extension", [VLAUseStaticAssert]>;
-def VLAExtension : DiagGroup<"vla-extension", [VLACxxExtension]>;
+def GNUVLACxxExtension : DiagGroup<"gnu-vla-cxx-extension", [VLACxxExtension]>;
+def VLAExtension : DiagGroup<"vla-extension", [GNUVLACxxExtension]>;
 def VLA : DiagGroup<"vla", [VLAExtension]>;
 def VolatileRegisterVar : DiagGroup<"volatile-register-var">;
 def Visibility : DiagGroup<"visibility">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 63e951daec7477..e0f9a074bdfb0c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -143,12 +143,12 @@ def ext_vla_cxx : ExtWarn<
   "variable length arrays in C++ are a Clang extension">,
   InGroup;
 def ext_vla_cxx_in_gnu_mode : Extension,
-  InGroup;
+  InGroup;
 def ext_vla_cxx_static_assert : ExtWarn<
   "variable length arrays in C++ are a Clang extension; did you mean to use "
   "'static_assert'?">, InGroup;
 def ext_vla_cxx_in_gnu_mode_static_assert : Extension<
-  ext_vla_cxx_static_assert.Summary>, InGroup;
+  ext_vla_cxx_static_assert.Summary>, InGroup;
 def warn_vla_used : Warning<"variable length array used">,
   InGroup, DefaultIgnore;
 def err_vla_in_sfinae : Error<
diff --git a/clang/test/Sema/vla-ext.c b/clang/test/Sema/vla-ext.c
index 4ad96d97901c9e..df553d5c030b44 100644
--- a/clang/test/Sema/vla-ext.c
+++ b/clang/test/Sema/vla-ext.c
@@ -3,6 +3,7 @@
  * RUN: %clang_cc1 -verify -pedantic -std=c89 %s
  * RUN: %clang_cc1 -verify -Wvla-extension -std=c89 %s
  * RUN: %clang_cc1 -verify=off -Wvla-cxx-extension -std=c89 %s
+ * RUN: %clang_cc1 -verify=off -Wgnu-vla-cxx-extension -std=c89 %s
  * RUN: %clang_cc1 -verify=off -pedantic -std=c99 %s
  * RUN: %clang_cc1 -verify=off -Wall -std=c99 %s
  * RUN: %clang_cc1 -verify=off -std=c99 -Wvla-extension %s
diff --git a/clang/test/SemaCXX/c99-variable-length-array.cpp 
b/clang/test/SemaCXX/c99-variable-length-array.cpp
index 82ddb0fd2e2337..5df5fa98291ddb 100644
--- a/clang/test/SemaCXX/c99-variable-length-array.cpp
+++ b/clang/test/SemaCXX/c99-variable-length-array.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wvla-extension %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wgnu-vla-cxx-extension %s
 struct NonPOD {
   NonPOD();
 };
diff --git a/clang/test/SemaCXX/coroutine-vla.cpp 
b/clang/test/SemaCXX/coroutine-vla.cpp
index 176e35f346e2b4..145358b08f3501 100644
--- a/clang/test/SemaCXX/coroutine-vla.cpp
+++ b/clang/test/SemaCXX/coroutine-vla.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -Wno-vla-cxx-extension -verify
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -Wno-gnu-vla-cxx-extension 
-verify
 #include "Inputs/std-coroutine.h"
 
 struct promise;

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

[clang] [Clang] Don't diagnose VLA for `-std=gnu++*` by default (PR #89943)

[Kolya Panchenko via cfe-commits]

nikolaypanchenko wrote:

> This was discussed in this RFC: 
> https://discourse.llvm.org/t/rfc-diagnosing-use-of-vlas-in-c/73109 and our 
> decision to move forward with that is independent of what GCC does. 

I saw that discussion, but I do miss where it has been agreed to diverge from 
GCC on this. Maybe more generic question will clear my misunderstanding: what 
is a stance of Clang on`-std=gnu++*`? Can it be different to what GCC does?

> Without more justification for walking this back, I do not agree with moving 
> forward with the revert.

We have library code that has to use VLA in source code in certain scenario and 
rely on compiler to optimize it away. The library is built with `-Wall` and 
after it started to use newer Clang, the error appear.
The main concern is simple: GCC doesn't do that (yet). Understandably, if GCC 
agrees to diagnose it, that change will need to be reverted.

https://github.com/llvm/llvm-project/pull/89943
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Don't diagnose VLA for `-std=gnu++*` by default (PR #89943)

[Kolya Panchenko via cfe-commits]

https://github.com/nikolaypanchenko closed 
https://github.com/llvm/llvm-project/pull/89943
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Don't diagnose VLA for `-std=gnu++*` by default (PR #89943)

[Kolya Panchenko via cfe-commits]

nikolaypanchenko wrote:

@AaronBallman @erichkeane thanks for the feedback. It's safe to close the PR 
then

https://github.com/llvm/llvm-project/pull/89943
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [LV] Support generating masks for switch terminators. (PR #99808)

[Kolya Panchenko via cfe-commits]

@@ -6,9 +6,43 @@ define void @switch_default_to_latch_common_dest(ptr %start, 
ptr %end) {
 ; IC1-LABEL: define void @switch_default_to_latch_common_dest(
 ; IC1-SAME: ptr [[START:%.*]], ptr [[END:%.*]]) #[[ATTR0:[0-9]+]] {
 ; IC1-NEXT:  [[ENTRY:.*]]:
+; IC1-NEXT:[[START2:%.*]] = ptrtoint ptr [[START]] to i64
+; IC1-NEXT:[[END1:%.*]] = ptrtoint ptr [[END]] to i64
+; IC1-NEXT:[[TMP0:%.*]] = add i64 [[END1]], -8
+; IC1-NEXT:[[TMP1:%.*]] = sub i64 [[TMP0]], [[START2]]
+; IC1-NEXT:[[TMP2:%.*]] = lshr i64 [[TMP1]], 3
+; IC1-NEXT:[[TMP3:%.*]] = add nuw nsw i64 [[TMP2]], 1
+; IC1-NEXT:[[MIN_ITERS_CHECK:%.*]] = icmp ult i64 [[TMP3]], 4
+; IC1-NEXT:br i1 [[MIN_ITERS_CHECK]], label %[[SCALAR_PH:.*]], label 
%[[VECTOR_PH:.*]]
+; IC1:   [[VECTOR_PH]]:
+; IC1-NEXT:[[N_MOD_VF:%.*]] = urem i64 [[TMP3]], 4
+; IC1-NEXT:[[N_VEC:%.*]] = sub i64 [[TMP3]], [[N_MOD_VF]]
+; IC1-NEXT:[[TMP4:%.*]] = mul i64 [[N_VEC]], 8
+; IC1-NEXT:[[IND_END:%.*]] = getelementptr i8, ptr [[START]], i64 [[TMP4]]
+; IC1-NEXT:br label %[[VECTOR_BODY:.*]]
+; IC1:   [[VECTOR_BODY]]:
+; IC1-NEXT:[[INDEX:%.*]] = phi i64 [ 0, %[[VECTOR_PH]] ], [ 
[[INDEX_NEXT:%.*]], %[[VECTOR_BODY]] ]
+; IC1-NEXT:[[OFFSET_IDX:%.*]] = mul i64 [[INDEX]], 8
+; IC1-NEXT:[[TMP5:%.*]] = add i64 [[OFFSET_IDX]], 0
+; IC1-NEXT:[[NEXT_GEP:%.*]] = getelementptr i8, ptr [[START]], i64 [[TMP5]]
+; IC1-NEXT:[[TMP6:%.*]] = getelementptr i64, ptr [[NEXT_GEP]], i32 0
+; IC1-NEXT:[[WIDE_LOAD:%.*]] = load <4 x i64>, ptr [[TMP6]], align 1
+; IC1-NEXT:[[TMP7:%.*]] = icmp eq <4 x i64> [[WIDE_LOAD]], 
+; IC1-NEXT:[[TMP8:%.*]] = icmp eq <4 x i64> [[WIDE_LOAD]], 
+; IC1-NEXT:[[TMP9:%.*]] = or <4 x i1> [[TMP7]], [[TMP8]]
+; IC1-NEXT:[[TMP10:%.*]] = or <4 x i1> [[TMP9]], [[TMP9]]

nikolaypanchenko wrote:

redundant

https://github.com/llvm/llvm-project/pull/99808
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [LV] Support generating masks for switch terminators. (PR #99808)

[Kolya Panchenko via cfe-commits]

https://github.com/nikolaypanchenko edited 
https://github.com/llvm/llvm-project/pull/99808
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [LV] Support generating masks for switch terminators. (PR #99808)

[Kolya Panchenko via cfe-commits]

https://github.com/nikolaypanchenko commented:

Add tests for vplan printing ?

https://github.com/llvm/llvm-project/pull/99808
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [LV] Support generating masks for switch terminators. (PR #99808)

[Kolya Panchenko via cfe-commits]

@@ -7763,6 +7763,41 @@ VPValue *VPRecipeBuilder::createEdgeMask(BasicBlock 
*Src, BasicBlock *Dst) {
 
   VPValue *SrcMask = getBlockInMask(Src);
 
+  if (auto *SI = dyn_cast(Src->getTerminator())) {
+// Create mask where the terminator in Src is a switch. We need to handle 2
+// separate cases:
+// 1. Dst is not the default desintation. Dst is reached if any of the 
cases

nikolaypanchenko wrote:

`desintation` -> `destination`

https://github.com/llvm/llvm-project/pull/99808
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits