* [Patch][V3][Patch 1/2]Change the 3rd parameter of function .DEFERRED_INIT from IS_VLA to decl name
@ 2022-01-10 23:58 Qing Zhao
2022-01-11 13:53 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Qing Zhao @ 2022-01-10 23:58 UTC (permalink / raw)
To: Richard Biener, Martin Sebor; +Cc: gcc Patches
[-- Attachment #1: Type: text/plain, Size: 5700 bytes --]
Hi, Richard,
I splited the previous patch for “Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables” into two separate patches.
This is the first one
This first patch is to fix (or work around ) PR103720, therefore it’s an important change, and need to be go into GCC12.
At the same time, this patch is the preparation for the second patch that will actually enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
The reason I separate the previous patch into two is: most of the previous concern was on the second part of the patch (the change in tree-ssa-uninit.c), I don’t
want those concern prevent this first patch from being approved into GCC12.
In this part, I addressed your comments in gimplify.c :
=====
tree decl_name
+ = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
+ IDENTIFIER_POINTER (DECL_NAME (decl)));
you need to deal with DECL_NAME being NULL.
=====
Please also see the detailed description below for the problem and solution of this patch.
This first patch has been bootstrapped and regressing tested on both X86 and aarch64.
Okay for GCC12?
Thanks.
Qing.
=================================
Change the 3rd parameter of function .DEFERRED_INIT from
IS_VLA to decl name.
Currently, the 3rd parameter of function .DEFERRED_INIT is IS_VLA, which is
not needed at all;
In this patch, we change the 3rd parameter from IS_VLA to the name of the var
decl for the following purposes:
1. Fix (or work around) PR103720:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
As confirmed in PR103720, with the current definition of .DEFERRED_INIT,
Dom transformed:
c$a$0_6 = .DEFERRED_INIT (8, 2, 0);
_1 = .DEFERRED_INIT (8, 2, 0);
into:
c$a$0_6 = .DEFERRED_INIT (8, 2, 0);
_1 = c$a$0_6;
which is incorrectly done due to Dom treating the two calls to const function
.DEFERRED_INIT as the same call since all actual parameters are the same.
The same issue has been exposed in PR102608 due to a different optimization VN,
the fix for PR102608 is to specially handle call to .DEFERRED_INIT in VN to
exclude it from CSE.
To fix PR103720, we could do the same as the fix to PR102608 to specially
handle call to .DEFERRED_INIT in Dom to exclude it from being optimized.
However, in addition to Dom and VN, there should be other optimizations that
have the same issue as PR103720 or PR102608 (As I built Linux kernel with
-ftrivial-auto-var-init=zero -Werror, I noticed a bunch of bugos warnings).
Other than identifying all the optimizations and specially handling call to
.DEFERRED_INIT in all these optimizations, changing the 3rd parameter of the
function .DEFERRED_INIT from IS_VLA to the name string of the var decl might
be a better workaround (or a fix). After this change, since the 3rd actual
parameter is the name string of the variable, different calls for different
variables will have different name strings as the 3rd actual, As a result, the
optimization that previously treated the different calls to .DEFERRED_INIT as
the same will be prevented.
2. Prepare for enabling -Wuninitialized + -ftrivail-auto-var-init for address
taken variables.
As discussion in the following thread:
https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
With the current implemenation of -ftrivial-auto-var-init and uninitialized
warning analysis, the uninitialized warning for an address taken auto variable
might be missed since the variable is completely eliminated by optimization and
replaced with a temporary variable in all the uses.
In order to improve such situation, changing the 3rd parameter of the function
.DEFERRED_INIT to the name string of the variable will provide necessary
information to uninitialized warning analysis to make the missing warning
possible.
gcc/ChangeLog:
2022-01-10 qing zhao <qing.zhao@oracle.com>
* gimplify.c (gimple_add_init_for_auto_var): Delete the 3rd argument.
Change the 3rd argument of function .DEFERRED_INIT to the name of the
decl.
(gimplify_decl_expr): Delete the 3rd argument when call
gimple_add_init_for_auto_var.
* internal-fn.c (expand_DEFERRED_INIT): Update comments to reflect
the 3rd argument change of function .DEFERRED_INIT.
* tree-cfg.c (verify_gimple_call): Update comments and verification
to reflect the 3rd argument change of function .DEFERRED_INIT.
* tree-sra.c (generate_subtree_deferred_init): Delete the 3rd argument.
(sra_modify_deferred_init): Change the 3rd argument of function
.DEFERRED_INIT to the name of the decl.
gcc/testsuite/ChangeLog:
2022-01-10 qing zhao <qing.zhao@oracle.com>
* c-c++-common/auto-init-1.c: Adjust testcase to reflect the 3rd
argument change of function .DEFERRED_INIT.
* c-c++-common/auto-init-10.c: Likewise.
* c-c++-common/auto-init-11.c: Likewise.
* c-c++-common/auto-init-12.c: Likewise.
* c-c++-common/auto-init-13.c: Likewise.
* c-c++-common/auto-init-14.c: Likewise.
* c-c++-common/auto-init-15.c: Likewise.
* c-c++-common/auto-init-16.c: Likewise.
* c-c++-common/auto-init-2.c: Likewise.
* c-c++-common/auto-init-3.c: Likewise.
* c-c++-common/auto-init-4.c: Likewise.
* c-c++-common/auto-init-5.c: Likewise.
* c-c++-common/auto-init-6.c: Likewise.
* c-c++-common/auto-init-7.c: Likewise.
* c-c++-common/auto-init-8.c: Likewise.
* c-c++-common/auto-init-9.c: Likewise.
* c-c++-common/auto-init-esra.c: Likewise.
* c-c++-common/auto-init-padding-1.c: Likewise.
* gcc.target/aarch64/auto-init-2.c: Likewise.
******The complete patch is attached:
[-- Attachment #2: 0001-Change-the-3rd-parameter-of-function-.DEFERRED_INIT-.patch --]
[-- Type: application/octet-stream, Size: 29797 bytes --]
From b31e4e25923a43ef4720457a3ac7a0ef16ae7458 Mon Sep 17 00:00:00 2001
From: Qing Zhao <qing.zhao@oracle.com>
Date: Mon, 10 Jan 2022 20:20:44 +0000
Subject: [PATCH 1/2] Change the 3rd parameter of function .DEFERRED_INIT from
IS_VLA to decl name.
Currently, the 3rd parameter of function .DEFERRED_INIT is IS_VLA, which is
not needed at all;
In this patch, we change the 3rd parameter from IS_VLA to the name of the var
decl for the following purposes:
1. Fix (or work around) PR103720:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
As confirmed in PR103720, with the current definition of .DEFERRED_INIT,
Dom transformed:
c$a$0_6 = .DEFERRED_INIT (8, 2, 0);
_1 = .DEFERRED_INIT (8, 2, 0);
into:
c$a$0_6 = .DEFERRED_INIT (8, 2, 0);
_1 = c$a$0_6;
which is incorrectly done due to Dom treating the two calls to const function
.DEFERRED_INIT as the same call since all actual parameters are the same.
The same issue has been exposed in PR102608 due to a different optimization VN,
the fix for PR102608 is to specially handle call to .DEFERRED_INIT in VN to
exclude it from CSE.
To fix PR103720, we could do the same as the fix to PR102608 to specially
handle call to .DEFERRED_INIT in Dom to exclude it from being optimized.
However, in addition to Dom and VN, there should be other optimizations that
have the same issue as PR103720 or PR102608 (As I built Linux kernel with
-ftrivial-auto-var-init=zero -Werror, I noticed a bunch of bugos warnings).
Other than identifying all the optimizations and specially handling call to
.DEFERRED_INIT in all these optimizations, changing the 3rd parameter of the
function .DEFERRED_INIT from IS_VLA to the name string of the var decl might
be a better workaround (or a fix). After this change, since the 3rd actual
parameter is the name string of the variable, different calls for different
variables will have different name strings as the 3rd actual, As a result, the
optimization that previously treated the different calls to .DEFERRED_INIT as
the same will be prevented.
2. Prepare for enabling -Wuninitialized + -ftrivail-auto-var-init for address
taken variables.
As discussion in the following thread:
https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
With the current implemenation of -ftrivial-auto-var-init and uninitialized
warning analysis, the uninitialized warning for an address taken auto variable
might be missed since the variable is completely eliminated by optimization and
replaced with a temporary variable in all the uses.
In order to improve such situation, changing the 3rd parameter of the function
.DEFERRED_INIT to the name string of the variable will provide necessary
information to uninitialized warning analysis to make the missing warning
possible.
gcc/ChangeLog:
2022-01-10 qing zhao <qing.zhao@oracle.com>
* gimplify.c (gimple_add_init_for_auto_var): Delete the 3rd argument.
Change the 3rd argument of function .DEFERRED_INIT to the name of the
decl.
(gimplify_decl_expr): Delete the 3rd argument when call
gimple_add_init_for_auto_var.
* internal-fn.c (expand_DEFERRED_INIT): Update comments to reflect
the 3rd argument change of function .DEFERRED_INIT.
* tree-cfg.c (verify_gimple_call): Update comments and verification
to reflect the 3rd argument change of function .DEFERRED_INIT.
* tree-sra.c (generate_subtree_deferred_init): Delete the 3rd argument.
(sra_modify_deferred_init): Change the 3rd argument of function
.DEFERRED_INIT to the name of the decl.
gcc/testsuite/ChangeLog:
2022-01-10 qing zhao <qing.zhao@oracle.com>
* c-c++-common/auto-init-1.c: Adjust testcase to reflect the 3rd
argument change of function .DEFERRED_INIT.
* c-c++-common/auto-init-10.c: Likewise.
* c-c++-common/auto-init-11.c: Likewise.
* c-c++-common/auto-init-12.c: Likewise.
* c-c++-common/auto-init-13.c: Likewise.
* c-c++-common/auto-init-14.c: Likewise.
* c-c++-common/auto-init-15.c: Likewise.
* c-c++-common/auto-init-16.c: Likewise.
* c-c++-common/auto-init-2.c: Likewise.
* c-c++-common/auto-init-3.c: Likewise.
* c-c++-common/auto-init-4.c: Likewise.
* c-c++-common/auto-init-5.c: Likewise.
* c-c++-common/auto-init-6.c: Likewise.
* c-c++-common/auto-init-7.c: Likewise.
* c-c++-common/auto-init-8.c: Likewise.
* c-c++-common/auto-init-9.c: Likewise.
* c-c++-common/auto-init-esra.c: Likewise.
* c-c++-common/auto-init-padding-1.c: Likewise.
* gcc.target/aarch64/auto-init-2.c: Likewise.
---
gcc/gimplify.c | 23 ++++++-----
gcc/internal-fn.c | 10 ++---
gcc/testsuite/c-c++-common/auto-init-1.c | 20 +++++-----
gcc/testsuite/c-c++-common/auto-init-10.c | 2 +-
gcc/testsuite/c-c++-common/auto-init-11.c | 2 +-
gcc/testsuite/c-c++-common/auto-init-12.c | 2 +-
gcc/testsuite/c-c++-common/auto-init-13.c | 4 +-
gcc/testsuite/c-c++-common/auto-init-14.c | 4 +-
gcc/testsuite/c-c++-common/auto-init-15.c | 2 +-
gcc/testsuite/c-c++-common/auto-init-16.c | 2 +-
gcc/testsuite/c-c++-common/auto-init-2.c | 20 +++++-----
gcc/testsuite/c-c++-common/auto-init-3.c | 6 +--
gcc/testsuite/c-c++-common/auto-init-4.c | 6 +--
gcc/testsuite/c-c++-common/auto-init-5.c | 6 +--
gcc/testsuite/c-c++-common/auto-init-6.c | 6 +--
gcc/testsuite/c-c++-common/auto-init-7.c | 8 ++--
gcc/testsuite/c-c++-common/auto-init-8.c | 8 ++--
gcc/testsuite/c-c++-common/auto-init-9.c | 4 +-
gcc/testsuite/c-c++-common/auto-init-esra.c | 6 +--
.../c-c++-common/auto-init-padding-1.c | 2 +-
.../gcc.target/aarch64/auto-init-2.c | 2 +-
gcc/tree-cfg.c | 39 +++++--------------
gcc/tree-sra.c | 12 +++---
23 files changed, 88 insertions(+), 108 deletions(-)
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index d1b27d7f46f4..0480b7ab5686 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1748,16 +1748,13 @@ force_labels_r (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
Build a call to internal const function DEFERRED_INIT:
1st argument: SIZE of the DECL;
2nd argument: INIT_TYPE;
- 3rd argument: IS_VLA, 0 NO, 1 YES;
+ 3rd argument: NAME of the DECL;
+
+ as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, NAME of the DECL). */
- as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA)
- if IS_VLA is false, the LHS is the DECL itself,
- if IS_VLA is true, the LHS is a MEM_REF whose address is the pointer
- to this DECL. */
static void
gimple_add_init_for_auto_var (tree decl,
enum auto_init_type init_type,
- bool is_vla,
gimple_seq *seq_p)
{
gcc_assert (auto_var_p (decl));
@@ -1767,15 +1764,22 @@ gimple_add_init_for_auto_var (tree decl,
tree init_type_node
= build_int_cst (integer_type_node, (int) init_type);
- tree is_vla_node
- = build_int_cst (integer_type_node, (int) is_vla);
+
+ char *decl_name_anonymous = xasprintf ("D.%u", DECL_UID (decl));
+ const char *decl_name_str = DECL_NAME (decl)
+ ? IDENTIFIER_POINTER (DECL_NAME (decl))
+ : decl_name_anonymous;
+ tree decl_name
+ = build_string_literal (strlen (decl_name_str) + 1,
+ decl_name_str);
tree call = build_call_expr_internal_loc (loc, IFN_DEFERRED_INIT,
TREE_TYPE (decl), 3,
decl_size, init_type_node,
- is_vla_node);
+ decl_name);
gimplify_assign (decl, call, seq_p);
+ free (decl_name_anonymous);
}
/* Generate padding initialization for automatic vairable DECL.
@@ -1947,7 +1951,6 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
{
gimple_add_init_for_auto_var (decl,
flag_auto_var_init,
- is_vla,
seq_p);
/* The expanding of a call to the above .DEFERRED_INIT will apply
block initialization to the whole space covered by this variable.
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index b24102a5990b..db16179c9f83 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -3011,11 +3011,7 @@ expand_UNIQUE (internal_fn, gcall *stmt)
}
/* Expand the IFN_DEFERRED_INIT function:
- LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA);
-
- if IS_VLA is false, the LHS is the DECL itself,
- if IS_VLA is true, the LHS is a MEM_REF whose address is the pointer
- to this DECL.
+ LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, NAME of the DECL);
Initialize the LHS with zero/pattern according to its second argument
INIT_TYPE:
@@ -3071,8 +3067,8 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
if (!reg_lhs)
{
- /* If this is a VLA or the variable is not in register,
- expand to a memset to initialize it. */
+ /* If the variable is not in register, expand to a memset
+ to initialize it. */
mark_addressable (lhs);
tree var_addr = build_fold_addr_expr (lhs);
diff --git a/gcc/testsuite/c-c++-common/auto-init-1.c b/gcc/testsuite/c-c++-common/auto-init-1.c
index 84ba0a90d45a..df04358728ba 100644
--- a/gcc/testsuite/c-c++-common/auto-init-1.c
+++ b/gcc/testsuite/c-c++-common/auto-init-1.c
@@ -29,13 +29,13 @@ void foo()
return;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(1, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(2, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" { target ilp32 } } } */
-/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" { target lp64 } } } */
-/* { dg-final { scan-tree-dump "temp6 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" { target ilp32 } } } */
-/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" { target lp64 } } } */
-/* { dg-final { scan-tree-dump "temp8 = .DEFERRED_INIT \\(1, 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(1, 2, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(2, 2, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(4, 2, \&\"temp3\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(4, 2, \&\"temp4\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(4, 2, \&\"temp5\"" "gimple" { target ilp32 } } } */
+/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(8, 2, \&\"temp5\"" "gimple" { target lp64 } } } */
+/* { dg-final { scan-tree-dump "temp6 = .DEFERRED_INIT \\(8, 2, \&\"temp6\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(4, 2, \&\"temp7\"" "gimple" { target ilp32 } } } */
+/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(8, 2, \&\"temp7\"" "gimple" { target lp64 } } } */
+/* { dg-final { scan-tree-dump "temp8 = .DEFERRED_INIT \\(1, 2, \&\"temp8\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-10.c b/gcc/testsuite/c-c++-common/auto-init-10.c
index f35205f2a300..dda7ea1e0324 100644
--- a/gcc/testsuite/c-c++-common/auto-init-10.c
+++ b/gcc/testsuite/c-c++-common/auto-init-10.c
@@ -18,5 +18,5 @@ void foo()
return;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(2, 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(2, 1, \&\"temp1\"" "gimple" } } */
/* { dg-final { scan-tree-dump-not "temp2 = .DEFERRED_INIT \\(" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-11.c b/gcc/testsuite/c-c++-common/auto-init-11.c
index a2d66908d83e..6eb468785ce1 100644
--- a/gcc/testsuite/c-c++-common/auto-init-11.c
+++ b/gcc/testsuite/c-c++-common/auto-init-11.c
@@ -11,4 +11,4 @@ void foo(int n)
return;
}
-/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 2, 1\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 2, \&\"arr\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-12.c b/gcc/testsuite/c-c++-common/auto-init-12.c
index f05d743fda02..964291c5bd94 100644
--- a/gcc/testsuite/c-c++-common/auto-init-12.c
+++ b/gcc/testsuite/c-c++-common/auto-init-12.c
@@ -11,4 +11,4 @@ void foo(int n)
return;
}
-/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 1, 1\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 1, \&\"arr\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-13.c b/gcc/testsuite/c-c++-common/auto-init-13.c
index b0c0365b288a..aa5883af770f 100644
--- a/gcc/testsuite/c-c++-common/auto-init-13.c
+++ b/gcc/testsuite/c-c++-common/auto-init-13.c
@@ -19,5 +19,5 @@ int foo()
return d.b + var.bar.b;
}
-/* { dg-final { scan-tree-dump "d = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "var = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "d = .DEFERRED_INIT \\(4, 1, \&\"d\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "var = .DEFERRED_INIT \\(4, 1, \&\"var\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-14.c b/gcc/testsuite/c-c++-common/auto-init-14.c
index 986bb19faaf0..dd1ff3e339d2 100644
--- a/gcc/testsuite/c-c++-common/auto-init-14.c
+++ b/gcc/testsuite/c-c++-common/auto-init-14.c
@@ -19,5 +19,5 @@ int foo()
return d.b + var.bar.b;
}
-/* { dg-final { scan-tree-dump "d = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "var = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "d = .DEFERRED_INIT \\(4, 2, \&\"d\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "var = .DEFERRED_INIT \\(4, 2, \&\"var\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-15.c b/gcc/testsuite/c-c++-common/auto-init-15.c
index aa9d7fab68a6..5857287ecbef 100644
--- a/gcc/testsuite/c-c++-common/auto-init-15.c
+++ b/gcc/testsuite/c-c++-common/auto-init-15.c
@@ -10,4 +10,4 @@ void foo(int a)
g(x);
}
-/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 2, 1\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 2, \&\"x\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-16.c b/gcc/testsuite/c-c++-common/auto-init-16.c
index 86493eef9e47..1e309959fc5c 100644
--- a/gcc/testsuite/c-c++-common/auto-init-16.c
+++ b/gcc/testsuite/c-c++-common/auto-init-16.c
@@ -10,4 +10,4 @@ void foo(int a)
g(x);
}
-/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 1, 1\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(D.\\d*, 1, \&\"x\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-2.c b/gcc/testsuite/c-c++-common/auto-init-2.c
index 69768d6451da..6ac63bb1ddac 100644
--- a/gcc/testsuite/c-c++-common/auto-init-2.c
+++ b/gcc/testsuite/c-c++-common/auto-init-2.c
@@ -29,13 +29,13 @@ void foo()
return;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(1, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(2, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" { target ilp32 } } } */
-/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" { target lp64 } } } */
-/* { dg-final { scan-tree-dump "temp6 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" { target ilp32 } } } */
-/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" { target lp64 } } } */
-/* { dg-final { scan-tree-dump "temp8 = .DEFERRED_INIT \\(1, 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(1, 1, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(2, 1, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(4, 1, \&\"temp3\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(4, 1, \&\"temp4\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(4, 1, \&\"temp5\"" "gimple" { target ilp32 } } } */
+/* { dg-final { scan-tree-dump "temp5 = .DEFERRED_INIT \\(8, 1, \&\"temp5\"" "gimple" { target lp64 } } } */
+/* { dg-final { scan-tree-dump "temp6 = .DEFERRED_INIT \\(8, 1, \&\"temp6\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(4, 1, \&\"temp7\"" "gimple" { target ilp32 } } } */
+/* { dg-final { scan-tree-dump "temp7 = .DEFERRED_INIT \\(8, 1, \&\"temp7\"" "gimple" { target lp64 } } } */
+/* { dg-final { scan-tree-dump "temp8 = .DEFERRED_INIT \\(1, 1, \&\"temp8\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-3.c b/gcc/testsuite/c-c++-common/auto-init-3.c
index 062d60c1631e..9d9c86d8dd08 100644
--- a/gcc/testsuite/c-c++-common/auto-init-3.c
+++ b/gcc/testsuite/c-c++-common/auto-init-3.c
@@ -14,6 +14,6 @@ long double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(4, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((8|12|16), 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(4, 2, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(8, 2, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((8|12|16), 2, \&\"temp3\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-4.c b/gcc/testsuite/c-c++-common/auto-init-4.c
index 9d8f23ed4e33..848df2a0e264 100644
--- a/gcc/testsuite/c-c++-common/auto-init-4.c
+++ b/gcc/testsuite/c-c++-common/auto-init-4.c
@@ -14,6 +14,6 @@ long double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(4, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((8|12|16), 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(4, 1, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(8, 1, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((8|12|16), 1, \&\"temp3\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-5.c b/gcc/testsuite/c-c++-common/auto-init-5.c
index 9c98a6e7ab2a..9c4de6121826 100644
--- a/gcc/testsuite/c-c++-common/auto-init-5.c
+++ b/gcc/testsuite/c-c++-common/auto-init-5.c
@@ -15,7 +15,7 @@ _Complex long double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(16, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((16|24|32), 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(8, 2, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(16, 2, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((16|24|32), 2, \&\"temp3\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-6.c b/gcc/testsuite/c-c++-common/auto-init-6.c
index 3fe24562ebb3..6a406447f3d9 100644
--- a/gcc/testsuite/c-c++-common/auto-init-6.c
+++ b/gcc/testsuite/c-c++-common/auto-init-6.c
@@ -15,7 +15,7 @@ _Complex long double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(16, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((16|24|32), 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(8, 1, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(16, 1, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\((16|24|32), 1, \&\"temp3\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-7.c b/gcc/testsuite/c-c++-common/auto-init-7.c
index 19986969a8f7..b44dd5e68ed1 100644
--- a/gcc/testsuite/c-c++-common/auto-init-7.c
+++ b/gcc/testsuite/c-c++-common/auto-init-7.c
@@ -29,7 +29,7 @@ double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(12, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(24, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(28, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(12, 2, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(24, 2, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(28, 2, \&\"temp3\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(8, 2, \&\"temp4\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-8.c b/gcc/testsuite/c-c++-common/auto-init-8.c
index 9778e911e3ad..739ac0289315 100644
--- a/gcc/testsuite/c-c++-common/auto-init-8.c
+++ b/gcc/testsuite/c-c++-common/auto-init-8.c
@@ -29,7 +29,7 @@ double foo()
return result;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(12, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(24, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(28, 1, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(8, 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(12, 1, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp2 = .DEFERRED_INIT \\(24, 1, \&\"temp2\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp3 = .DEFERRED_INIT \\(28, 1, \&\"temp3\"" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp4 = .DEFERRED_INIT \\(8, 1, \&\"temp4\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-9.c b/gcc/testsuite/c-c++-common/auto-init-9.c
index 29acb7f86691..113107ffd5c0 100644
--- a/gcc/testsuite/c-c++-common/auto-init-9.c
+++ b/gcc/testsuite/c-c++-common/auto-init-9.c
@@ -16,5 +16,5 @@ void foo()
return;
}
-/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(2, 2, 0\\)" "gimple" } } */
-/* { dg-final { scan-tree-dump-not "temp2 = .DEFERRED_INIT \\(8, 2, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump "temp1 = .DEFERRED_INIT \\(2, 2, \&\"temp1\"" "gimple" } } */
+/* { dg-final { scan-tree-dump-not "temp2 = .DEFERRED_INIT \\(8, 2, \&\"temp2\"" "gimple" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-esra.c b/gcc/testsuite/c-c++-common/auto-init-esra.c
index 77ec02355df4..ce6779f20fcd 100644
--- a/gcc/testsuite/c-c++-common/auto-init-esra.c
+++ b/gcc/testsuite/c-c++-common/auto-init-esra.c
@@ -1,6 +1,6 @@
/* Verify the strength reduction adjustment for -ftrivial-auto-var-init. */
/* { dg-do compile } */
-/* { dg-options "-O2 -ftrivial-auto-var-init=zero -fdump-tree-gimple -fdump-tree-esra" } */
+/* { dg-options "-O2 -ftrivial-auto-var-init=zero -fno-PIC -fdump-tree-gimple -fdump-tree-esra" } */
typedef double VECTOR[3];
@@ -31,5 +31,5 @@ void VCross(VECTOR a, const VECTOR b, const VECTOR c)
Assign_Vector(a, tmp);
}
-/* { dg-final { scan-tree-dump-times "tmp = .DEFERRED_INIT \\(24, 2, 0\\)" 1 "gimple" } } */
-/* { dg-final { scan-tree-dump-times ".DEFERRED_INIT \\(8, 2, 0\\)" 3 "esra" } } */
+/* { dg-final { scan-tree-dump-times "tmp = .DEFERRED_INIT \\(24, 2, \&\"tmp\"" 1 "gimple" } } */
+/* { dg-final { scan-tree-dump-times ".DEFERRED_INIT \\(8, 2, \&\"tmp\"" 3 "esra" } } */
diff --git a/gcc/testsuite/c-c++-common/auto-init-padding-1.c b/gcc/testsuite/c-c++-common/auto-init-padding-1.c
index 83db8dde8324..d2e322717f0e 100644
--- a/gcc/testsuite/c-c++-common/auto-init-padding-1.c
+++ b/gcc/testsuite/c-c++-common/auto-init-padding-1.c
@@ -19,5 +19,5 @@ void foo(int a)
g(s);
}
-/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(24, 1, 0\\)" "gimple" } } */
+/* { dg-final { scan-tree-dump ".DEFERRED_INIT \\(24, 1, \&\"s\"" "gimple" } } */
/* { dg-final { scan-tree-dump "__builtin_clear_padding" "gimple" } } */
diff --git a/gcc/testsuite/gcc.target/aarch64/auto-init-2.c b/gcc/testsuite/gcc.target/aarch64/auto-init-2.c
index 2c54e6d60382..375befd325b0 100644
--- a/gcc/testsuite/gcc.target/aarch64/auto-init-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/auto-init-2.c
@@ -32,4 +32,4 @@ void foo()
/* { dg-final { scan-rtl-dump-times "0xfe\\\]" 1 "expand" } } */
/* { dg-final { scan-rtl-dump-times "0xfffffffffffffefe" 1 "expand" } } */
/* { dg-final { scan-rtl-dump-times "0xfffffffffefefefe" 2 "expand" } } */
-/* { dg-final { scan-rtl-dump-times "0xfefefefefefefefe" 2 "expand" } } */
+/* { dg-final { scan-rtl-dump-times "0xfefefefefefefefe" 3 "expand" } } */
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index bb3779367fee..b7fe313b70c3 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -3455,19 +3455,14 @@ verify_gimple_call (gcall *stmt)
}
/* For a call to .DEFERRED_INIT,
- LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA)
- we should guarantee that the 1st and the 3rd arguments are consistent:
- 1st argument: SIZE of the DECL;
- 3rd argument: IS_VLA, 0 NO, 1 YES;
+ LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, NAME of the DECL)
+ we should guarantee that when the 1st argument is a constant, it should
+ be the same as the size of the LHS. */
- if IS_VLA is false, the 1st argument should be a constant and the same as
- the size of the LHS. */
if (gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
{
tree size_of_arg0 = gimple_call_arg (stmt, 0);
tree size_of_lhs = TYPE_SIZE_UNIT (TREE_TYPE (lhs));
- tree is_vla_node = gimple_call_arg (stmt, 2);
- bool is_vla = (bool) TREE_INT_CST_LOW (is_vla_node);
if (TREE_CODE (lhs) == SSA_NAME)
lhs = SSA_NAME_VAR (lhs);
@@ -3477,27 +3472,13 @@ verify_gimple_call (gcall *stmt)
&size_from_arg0);
bool is_constant_size_lhs = poly_int_tree_p (size_of_lhs,
&size_from_lhs);
- if (!is_vla)
- {
- if (!is_constant_size_arg0)
- {
- error ("%<DEFFERED_INIT%> calls for non-VLA should have "
- "constant size for the first argument");
- return true;
- }
- else if (!is_constant_size_lhs)
- {
- error ("%<DEFFERED_INIT%> calls for non-VLA should have "
- "constant size for the LHS");
- return true;
- }
- else if (maybe_ne (size_from_arg0, size_from_lhs))
- {
- error ("%<DEFFERED_INIT%> calls for non-VLA should have same "
- "constant size for the first argument and LHS");
- return true;
- }
- }
+ if (is_constant_size_arg0 && is_constant_size_lhs)
+ if (maybe_ne (size_from_arg0, size_from_lhs))
+ {
+ error ("%<DEFFERED_INIT%> calls should have same "
+ "constant size for the first argument and LHS");
+ return true;
+ }
}
/* ??? The C frontend passes unpromoted arguments in case it
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 889c028f1d7c..e0ea2c7b3084 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -4123,7 +4123,7 @@ get_repl_default_def_ssa_name (struct access *racc, tree reg_type)
static void
generate_subtree_deferred_init (struct access *access,
tree init_type,
- tree is_vla,
+ tree decl_name,
gimple_stmt_iterator *gsi,
location_t loc)
{
@@ -4135,7 +4135,7 @@ generate_subtree_deferred_init (struct access *access,
gimple *call
= gimple_build_call_internal (IFN_DEFERRED_INIT, 3,
TYPE_SIZE_UNIT (TREE_TYPE (repl)),
- init_type, is_vla);
+ init_type, decl_name);
gimple_call_set_lhs (call, repl);
gsi_insert_before (gsi, call, GSI_SAME_STMT);
update_stmt (call);
@@ -4144,7 +4144,7 @@ generate_subtree_deferred_init (struct access *access,
}
if (access->first_child)
generate_subtree_deferred_init (access->first_child, init_type,
- is_vla, gsi, loc);
+ decl_name, gsi, loc);
access = access ->next_sibling;
}
@@ -4152,7 +4152,7 @@ generate_subtree_deferred_init (struct access *access,
}
/* For a call to .DEFERRED_INIT:
- var = .DEFERRED_INIT (size_of_var, init_type, is_vla);
+ var = .DEFERRED_INIT (size_of_var, init_type, name_of_var);
examine the LHS variable VAR and replace it with a scalar replacement if
there is one, also replace the RHS call to a call to .DEFERRED_INIT of
the corresponding scalar relacement variable. Examine the subtree and
@@ -4164,7 +4164,7 @@ sra_modify_deferred_init (gimple *stmt, gimple_stmt_iterator *gsi)
{
tree lhs = gimple_call_lhs (stmt);
tree init_type = gimple_call_arg (stmt, 1);
- tree is_vla = gimple_call_arg (stmt, 2);
+ tree decl_name = gimple_call_arg (stmt, 2);
struct access *lhs_access = get_access_for_expr (lhs);
if (!lhs_access)
@@ -4185,7 +4185,7 @@ sra_modify_deferred_init (gimple *stmt, gimple_stmt_iterator *gsi)
if (lhs_access->first_child)
generate_subtree_deferred_init (lhs_access->first_child,
- init_type, is_vla, gsi, loc);
+ init_type, decl_name, gsi, loc);
if (lhs_access->grp_covered)
{
unlink_stmt_vdef (stmt);
--
2.27.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch][V3][Patch 1/2]Change the 3rd parameter of function .DEFERRED_INIT from IS_VLA to decl name
2022-01-10 23:58 [Patch][V3][Patch 1/2]Change the 3rd parameter of function .DEFERRED_INIT from IS_VLA to decl name Qing Zhao
@ 2022-01-11 13:53 ` Richard Biener
2022-01-11 15:38 ` Qing Zhao
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2022-01-11 13:53 UTC (permalink / raw)
To: Qing Zhao; +Cc: Martin Sebor, gcc Patches
On Tue, Jan 11, 2022 at 12:58 AM Qing Zhao <qing.zhao@oracle.com> wrote:
>
> Hi, Richard,
>
> I splited the previous patch for “Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables” into two separate patches.
> This is the first one
>
> This first patch is to fix (or work around ) PR103720, therefore it’s an important change, and need to be go into GCC12.
> At the same time, this patch is the preparation for the second patch that will actually enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
>
> The reason I separate the previous patch into two is: most of the previous concern was on the second part of the patch (the change in tree-ssa-uninit.c), I don’t
> want those concern prevent this first patch from being approved into GCC12.
>
>
> In this part, I addressed your comments in gimplify.c :
>
> =====
> tree decl_name
> + = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
> + IDENTIFIER_POINTER (DECL_NAME (decl)));
>
> you need to deal with DECL_NAME being NULL.
> =====
>
> Please also see the detailed description below for the problem and solution of this patch.
>
> This first patch has been bootstrapped and regressing tested on both X86 and aarch64.
>
> Okay for GCC12?
+
+ char *decl_name_anonymous = xasprintf ("D.%u", DECL_UID (decl));
+ const char *decl_name_str = DECL_NAME (decl)
+ ? IDENTIFIER_POINTER (DECL_NAME (decl))
+ : decl_name_anonymous;
+ tree decl_name
+ = build_string_literal (strlen (decl_name_str) + 1,
+ decl_name_str);
please avoid the xasprintf in the case DECL_NAME is not NULL, I'd be happy
with sth like
tree decl_name;
if (DECL_NAME (decl))
decl_name = build_string_literal (...);
else
{
char *decl_name_anon = xasprintf (...);
decl_name = build_string_literal (...);
free (decl_name_anon);
}
otherwise the patch is OK to commit (just do the above change and
re-test / push).
Thanks,
Richard.
> Thanks.
>
> Qing.
>
>
> =================================
>
> Change the 3rd parameter of function .DEFERRED_INIT from
> IS_VLA to decl name.
>
> Currently, the 3rd parameter of function .DEFERRED_INIT is IS_VLA, which is
> not needed at all;
>
> In this patch, we change the 3rd parameter from IS_VLA to the name of the var
> decl for the following purposes:
>
> 1. Fix (or work around) PR103720:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
>
> As confirmed in PR103720, with the current definition of .DEFERRED_INIT,
>
> Dom transformed:
> c$a$0_6 = .DEFERRED_INIT (8, 2, 0);
> _1 = .DEFERRED_INIT (8, 2, 0);
>
> into:
> c$a$0_6 = .DEFERRED_INIT (8, 2, 0);
> _1 = c$a$0_6;
>
> which is incorrectly done due to Dom treating the two calls to const function
> .DEFERRED_INIT as the same call since all actual parameters are the same.
>
> The same issue has been exposed in PR102608 due to a different optimization VN,
> the fix for PR102608 is to specially handle call to .DEFERRED_INIT in VN to
> exclude it from CSE.
>
> To fix PR103720, we could do the same as the fix to PR102608 to specially
> handle call to .DEFERRED_INIT in Dom to exclude it from being optimized.
>
> However, in addition to Dom and VN, there should be other optimizations that
> have the same issue as PR103720 or PR102608 (As I built Linux kernel with
> -ftrivial-auto-var-init=zero -Werror, I noticed a bunch of bugos warnings).
>
> Other than identifying all the optimizations and specially handling call to
> .DEFERRED_INIT in all these optimizations, changing the 3rd parameter of the
> function .DEFERRED_INIT from IS_VLA to the name string of the var decl might
> be a better workaround (or a fix). After this change, since the 3rd actual
> parameter is the name string of the variable, different calls for different
> variables will have different name strings as the 3rd actual, As a result, the
> optimization that previously treated the different calls to .DEFERRED_INIT as
> the same will be prevented.
>
> 2. Prepare for enabling -Wuninitialized + -ftrivail-auto-var-init for address
> taken variables.
>
> As discussion in the following thread:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
>
> With the current implemenation of -ftrivial-auto-var-init and uninitialized
> warning analysis, the uninitialized warning for an address taken auto variable
> might be missed since the variable is completely eliminated by optimization and
> replaced with a temporary variable in all the uses.
>
> In order to improve such situation, changing the 3rd parameter of the function
> .DEFERRED_INIT to the name string of the variable will provide necessary
> information to uninitialized warning analysis to make the missing warning
> possible.
>
> gcc/ChangeLog:
>
> 2022-01-10 qing zhao <qing.zhao@oracle.com>
>
> * gimplify.c (gimple_add_init_for_auto_var): Delete the 3rd argument.
> Change the 3rd argument of function .DEFERRED_INIT to the name of the
> decl.
> (gimplify_decl_expr): Delete the 3rd argument when call
> gimple_add_init_for_auto_var.
> * internal-fn.c (expand_DEFERRED_INIT): Update comments to reflect
> the 3rd argument change of function .DEFERRED_INIT.
> * tree-cfg.c (verify_gimple_call): Update comments and verification
> to reflect the 3rd argument change of function .DEFERRED_INIT.
> * tree-sra.c (generate_subtree_deferred_init): Delete the 3rd argument.
> (sra_modify_deferred_init): Change the 3rd argument of function
> .DEFERRED_INIT to the name of the decl.
>
> gcc/testsuite/ChangeLog:
>
> 2022-01-10 qing zhao <qing.zhao@oracle.com>
>
> * c-c++-common/auto-init-1.c: Adjust testcase to reflect the 3rd
> argument change of function .DEFERRED_INIT.
> * c-c++-common/auto-init-10.c: Likewise.
> * c-c++-common/auto-init-11.c: Likewise.
> * c-c++-common/auto-init-12.c: Likewise.
> * c-c++-common/auto-init-13.c: Likewise.
> * c-c++-common/auto-init-14.c: Likewise.
> * c-c++-common/auto-init-15.c: Likewise.
> * c-c++-common/auto-init-16.c: Likewise.
> * c-c++-common/auto-init-2.c: Likewise.
> * c-c++-common/auto-init-3.c: Likewise.
> * c-c++-common/auto-init-4.c: Likewise.
> * c-c++-common/auto-init-5.c: Likewise.
> * c-c++-common/auto-init-6.c: Likewise.
> * c-c++-common/auto-init-7.c: Likewise.
> * c-c++-common/auto-init-8.c: Likewise.
> * c-c++-common/auto-init-9.c: Likewise.
> * c-c++-common/auto-init-esra.c: Likewise.
> * c-c++-common/auto-init-padding-1.c: Likewise.
> * gcc.target/aarch64/auto-init-2.c: Likewise.
>
> ******The complete patch is attached:
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch][V3][Patch 1/2]Change the 3rd parameter of function .DEFERRED_INIT from IS_VLA to decl name
2022-01-11 13:53 ` Richard Biener
@ 2022-01-11 15:38 ` Qing Zhao
2022-01-11 23:22 ` Qing Zhao
0 siblings, 1 reply; 4+ messages in thread
From: Qing Zhao @ 2022-01-11 15:38 UTC (permalink / raw)
To: Richard Biener; +Cc: Martin Sebor, gcc Patches
> On Jan 11, 2022, at 7:53 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>
> On Tue, Jan 11, 2022 at 12:58 AM Qing Zhao <qing.zhao@oracle.com> wrote:
>>
>> Hi, Richard,
>>
>> I splited the previous patch for “Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables” into two separate patches.
>> This is the first one
>>
>> This first patch is to fix (or work around ) PR103720, therefore it’s an important change, and need to be go into GCC12.
>> At the same time, this patch is the preparation for the second patch that will actually enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
>>
>> The reason I separate the previous patch into two is: most of the previous concern was on the second part of the patch (the change in tree-ssa-uninit.c), I don’t
>> want those concern prevent this first patch from being approved into GCC12.
>>
>>
>> In this part, I addressed your comments in gimplify.c :
>>
>> =====
>> tree decl_name
>> + = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
>> + IDENTIFIER_POINTER (DECL_NAME (decl)));
>>
>> you need to deal with DECL_NAME being NULL.
>> =====
>>
>> Please also see the detailed description below for the problem and solution of this patch.
>>
>> This first patch has been bootstrapped and regressing tested on both X86 and aarch64.
>>
>> Okay for GCC12?
>
> +
> + char *decl_name_anonymous = xasprintf ("D.%u", DECL_UID (decl));
> + const char *decl_name_str = DECL_NAME (decl)
> + ? IDENTIFIER_POINTER (DECL_NAME (decl))
> + : decl_name_anonymous;
> + tree decl_name
> + = build_string_literal (strlen (decl_name_str) + 1,
> + decl_name_str);
>
> please avoid the xasprintf in the case DECL_NAME is not NULL, I'd be happy
> with sth like
>
> tree decl_name;
> if (DECL_NAME (decl))
> decl_name = build_string_literal (...);
> else
> {
> char *decl_name_anon = xasprintf (...);
> decl_name = build_string_literal (...);
> free (decl_name_anon);
> }
>
> otherwise the patch is OK to commit (just do the above change and
> re-test / push).
Thanks for the comment, will fix this and commit the patch.
Qing
>
> Thanks,
> Richard.
>
>> Thanks.
>>
>> Qing.
>>
>>
>> =================================
>>
>> Change the 3rd parameter of function .DEFERRED_INIT from
>> IS_VLA to decl name.
>>
>> Currently, the 3rd parameter of function .DEFERRED_INIT is IS_VLA, which is
>> not needed at all;
>>
>> In this patch, we change the 3rd parameter from IS_VLA to the name of the var
>> decl for the following purposes:
>>
>> 1. Fix (or work around) PR103720:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
>>
>> As confirmed in PR103720, with the current definition of .DEFERRED_INIT,
>>
>> Dom transformed:
>> c$a$0_6 = .DEFERRED_INIT (8, 2, 0);
>> _1 = .DEFERRED_INIT (8, 2, 0);
>>
>> into:
>> c$a$0_6 = .DEFERRED_INIT (8, 2, 0);
>> _1 = c$a$0_6;
>>
>> which is incorrectly done due to Dom treating the two calls to const function
>> .DEFERRED_INIT as the same call since all actual parameters are the same.
>>
>> The same issue has been exposed in PR102608 due to a different optimization VN,
>> the fix for PR102608 is to specially handle call to .DEFERRED_INIT in VN to
>> exclude it from CSE.
>>
>> To fix PR103720, we could do the same as the fix to PR102608 to specially
>> handle call to .DEFERRED_INIT in Dom to exclude it from being optimized.
>>
>> However, in addition to Dom and VN, there should be other optimizations that
>> have the same issue as PR103720 or PR102608 (As I built Linux kernel with
>> -ftrivial-auto-var-init=zero -Werror, I noticed a bunch of bugos warnings).
>>
>> Other than identifying all the optimizations and specially handling call to
>> .DEFERRED_INIT in all these optimizations, changing the 3rd parameter of the
>> function .DEFERRED_INIT from IS_VLA to the name string of the var decl might
>> be a better workaround (or a fix). After this change, since the 3rd actual
>> parameter is the name string of the variable, different calls for different
>> variables will have different name strings as the 3rd actual, As a result, the
>> optimization that previously treated the different calls to .DEFERRED_INIT as
>> the same will be prevented.
>>
>> 2. Prepare for enabling -Wuninitialized + -ftrivail-auto-var-init for address
>> taken variables.
>>
>> As discussion in the following thread:
>>
>> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
>>
>> With the current implemenation of -ftrivial-auto-var-init and uninitialized
>> warning analysis, the uninitialized warning for an address taken auto variable
>> might be missed since the variable is completely eliminated by optimization and
>> replaced with a temporary variable in all the uses.
>>
>> In order to improve such situation, changing the 3rd parameter of the function
>> .DEFERRED_INIT to the name string of the variable will provide necessary
>> information to uninitialized warning analysis to make the missing warning
>> possible.
>>
>> gcc/ChangeLog:
>>
>> 2022-01-10 qing zhao <qing.zhao@oracle.com>
>>
>> * gimplify.c (gimple_add_init_for_auto_var): Delete the 3rd argument.
>> Change the 3rd argument of function .DEFERRED_INIT to the name of the
>> decl.
>> (gimplify_decl_expr): Delete the 3rd argument when call
>> gimple_add_init_for_auto_var.
>> * internal-fn.c (expand_DEFERRED_INIT): Update comments to reflect
>> the 3rd argument change of function .DEFERRED_INIT.
>> * tree-cfg.c (verify_gimple_call): Update comments and verification
>> to reflect the 3rd argument change of function .DEFERRED_INIT.
>> * tree-sra.c (generate_subtree_deferred_init): Delete the 3rd argument.
>> (sra_modify_deferred_init): Change the 3rd argument of function
>> .DEFERRED_INIT to the name of the decl.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2022-01-10 qing zhao <qing.zhao@oracle.com>
>>
>> * c-c++-common/auto-init-1.c: Adjust testcase to reflect the 3rd
>> argument change of function .DEFERRED_INIT.
>> * c-c++-common/auto-init-10.c: Likewise.
>> * c-c++-common/auto-init-11.c: Likewise.
>> * c-c++-common/auto-init-12.c: Likewise.
>> * c-c++-common/auto-init-13.c: Likewise.
>> * c-c++-common/auto-init-14.c: Likewise.
>> * c-c++-common/auto-init-15.c: Likewise.
>> * c-c++-common/auto-init-16.c: Likewise.
>> * c-c++-common/auto-init-2.c: Likewise.
>> * c-c++-common/auto-init-3.c: Likewise.
>> * c-c++-common/auto-init-4.c: Likewise.
>> * c-c++-common/auto-init-5.c: Likewise.
>> * c-c++-common/auto-init-6.c: Likewise.
>> * c-c++-common/auto-init-7.c: Likewise.
>> * c-c++-common/auto-init-8.c: Likewise.
>> * c-c++-common/auto-init-9.c: Likewise.
>> * c-c++-common/auto-init-esra.c: Likewise.
>> * c-c++-common/auto-init-padding-1.c: Likewise.
>> * gcc.target/aarch64/auto-init-2.c: Likewise.
>>
>> ******The complete patch is attached:
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Patch][V3][Patch 1/2]Change the 3rd parameter of function .DEFERRED_INIT from IS_VLA to decl name
2022-01-11 15:38 ` Qing Zhao
@ 2022-01-11 23:22 ` Qing Zhao
0 siblings, 0 replies; 4+ messages in thread
From: Qing Zhao @ 2022-01-11 23:22 UTC (permalink / raw)
To: richard Biener; +Cc: gcc Patches
FYI.
Just committed this patch to gcc12 as:
https://gcc.gnu.org/pipermail/gcc-cvs/2022-January/359118.html
Qing
> On Jan 11, 2022, at 9:38 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
>
>
>> On Jan 11, 2022, at 7:53 AM, Richard Biener <richard.guenther@gmail.com> wrote:
>>
>> On Tue, Jan 11, 2022 at 12:58 AM Qing Zhao <qing.zhao@oracle.com> wrote:
>>>
>>> Hi, Richard,
>>>
>>> I splited the previous patch for “Enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables” into two separate patches.
>>> This is the first one
>>>
>>> This first patch is to fix (or work around ) PR103720, therefore it’s an important change, and need to be go into GCC12.
>>> At the same time, this patch is the preparation for the second patch that will actually enable -Wuninitialized + -ftrivial-auto-var-init for address taken variables.
>>>
>>> The reason I separate the previous patch into two is: most of the previous concern was on the second part of the patch (the change in tree-ssa-uninit.c), I don’t
>>> want those concern prevent this first patch from being approved into GCC12.
>>>
>>>
>>> In this part, I addressed your comments in gimplify.c :
>>>
>>> =====
>>> tree decl_name
>>> + = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)) + 1,
>>> + IDENTIFIER_POINTER (DECL_NAME (decl)));
>>>
>>> you need to deal with DECL_NAME being NULL.
>>> =====
>>>
>>> Please also see the detailed description below for the problem and solution of this patch.
>>>
>>> This first patch has been bootstrapped and regressing tested on both X86 and aarch64.
>>>
>>> Okay for GCC12?
>>
>> +
>> + char *decl_name_anonymous = xasprintf ("D.%u", DECL_UID (decl));
>> + const char *decl_name_str = DECL_NAME (decl)
>> + ? IDENTIFIER_POINTER (DECL_NAME (decl))
>> + : decl_name_anonymous;
>> + tree decl_name
>> + = build_string_literal (strlen (decl_name_str) + 1,
>> + decl_name_str);
>>
>> please avoid the xasprintf in the case DECL_NAME is not NULL, I'd be happy
>> with sth like
>>
>> tree decl_name;
>> if (DECL_NAME (decl))
>> decl_name = build_string_literal (...);
>> else
>> {
>> char *decl_name_anon = xasprintf (...);
>> decl_name = build_string_literal (...);
>> free (decl_name_anon);
>> }
>>
>> otherwise the patch is OK to commit (just do the above change and
>> re-test / push).
>
> Thanks for the comment, will fix this and commit the patch.
>
> Qing
>>
>> Thanks,
>> Richard.
>>
>>> Thanks.
>>>
>>> Qing.
>>>
>>>
>>> =================================
>>>
>>> Change the 3rd parameter of function .DEFERRED_INIT from
>>> IS_VLA to decl name.
>>>
>>> Currently, the 3rd parameter of function .DEFERRED_INIT is IS_VLA, which is
>>> not needed at all;
>>>
>>> In this patch, we change the 3rd parameter from IS_VLA to the name of the var
>>> decl for the following purposes:
>>>
>>> 1. Fix (or work around) PR103720:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103720
>>>
>>> As confirmed in PR103720, with the current definition of .DEFERRED_INIT,
>>>
>>> Dom transformed:
>>> c$a$0_6 = .DEFERRED_INIT (8, 2, 0);
>>> _1 = .DEFERRED_INIT (8, 2, 0);
>>>
>>> into:
>>> c$a$0_6 = .DEFERRED_INIT (8, 2, 0);
>>> _1 = c$a$0_6;
>>>
>>> which is incorrectly done due to Dom treating the two calls to const function
>>> .DEFERRED_INIT as the same call since all actual parameters are the same.
>>>
>>> The same issue has been exposed in PR102608 due to a different optimization VN,
>>> the fix for PR102608 is to specially handle call to .DEFERRED_INIT in VN to
>>> exclude it from CSE.
>>>
>>> To fix PR103720, we could do the same as the fix to PR102608 to specially
>>> handle call to .DEFERRED_INIT in Dom to exclude it from being optimized.
>>>
>>> However, in addition to Dom and VN, there should be other optimizations that
>>> have the same issue as PR103720 or PR102608 (As I built Linux kernel with
>>> -ftrivial-auto-var-init=zero -Werror, I noticed a bunch of bugos warnings).
>>>
>>> Other than identifying all the optimizations and specially handling call to
>>> .DEFERRED_INIT in all these optimizations, changing the 3rd parameter of the
>>> function .DEFERRED_INIT from IS_VLA to the name string of the var decl might
>>> be a better workaround (or a fix). After this change, since the 3rd actual
>>> parameter is the name string of the variable, different calls for different
>>> variables will have different name strings as the 3rd actual, As a result, the
>>> optimization that previously treated the different calls to .DEFERRED_INIT as
>>> the same will be prevented.
>>>
>>> 2. Prepare for enabling -Wuninitialized + -ftrivail-auto-var-init for address
>>> taken variables.
>>>
>>> As discussion in the following thread:
>>>
>>> https://gcc.gnu.org/pipermail/gcc-patches/2021-August/577431.html
>>>
>>> With the current implemenation of -ftrivial-auto-var-init and uninitialized
>>> warning analysis, the uninitialized warning for an address taken auto variable
>>> might be missed since the variable is completely eliminated by optimization and
>>> replaced with a temporary variable in all the uses.
>>>
>>> In order to improve such situation, changing the 3rd parameter of the function
>>> .DEFERRED_INIT to the name string of the variable will provide necessary
>>> information to uninitialized warning analysis to make the missing warning
>>> possible.
>>>
>>> gcc/ChangeLog:
>>>
>>> 2022-01-10 qing zhao <qing.zhao@oracle.com>
>>>
>>> * gimplify.c (gimple_add_init_for_auto_var): Delete the 3rd argument.
>>> Change the 3rd argument of function .DEFERRED_INIT to the name of the
>>> decl.
>>> (gimplify_decl_expr): Delete the 3rd argument when call
>>> gimple_add_init_for_auto_var.
>>> * internal-fn.c (expand_DEFERRED_INIT): Update comments to reflect
>>> the 3rd argument change of function .DEFERRED_INIT.
>>> * tree-cfg.c (verify_gimple_call): Update comments and verification
>>> to reflect the 3rd argument change of function .DEFERRED_INIT.
>>> * tree-sra.c (generate_subtree_deferred_init): Delete the 3rd argument.
>>> (sra_modify_deferred_init): Change the 3rd argument of function
>>> .DEFERRED_INIT to the name of the decl.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2022-01-10 qing zhao <qing.zhao@oracle.com>
>>>
>>> * c-c++-common/auto-init-1.c: Adjust testcase to reflect the 3rd
>>> argument change of function .DEFERRED_INIT.
>>> * c-c++-common/auto-init-10.c: Likewise.
>>> * c-c++-common/auto-init-11.c: Likewise.
>>> * c-c++-common/auto-init-12.c: Likewise.
>>> * c-c++-common/auto-init-13.c: Likewise.
>>> * c-c++-common/auto-init-14.c: Likewise.
>>> * c-c++-common/auto-init-15.c: Likewise.
>>> * c-c++-common/auto-init-16.c: Likewise.
>>> * c-c++-common/auto-init-2.c: Likewise.
>>> * c-c++-common/auto-init-3.c: Likewise.
>>> * c-c++-common/auto-init-4.c: Likewise.
>>> * c-c++-common/auto-init-5.c: Likewise.
>>> * c-c++-common/auto-init-6.c: Likewise.
>>> * c-c++-common/auto-init-7.c: Likewise.
>>> * c-c++-common/auto-init-8.c: Likewise.
>>> * c-c++-common/auto-init-9.c: Likewise.
>>> * c-c++-common/auto-init-esra.c: Likewise.
>>> * c-c++-common/auto-init-padding-1.c: Likewise.
>>> * gcc.target/aarch64/auto-init-2.c: Likewise.
>>>
>>> ******The complete patch is attached:
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-11 23:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-10 23:58 [Patch][V3][Patch 1/2]Change the 3rd parameter of function .DEFERRED_INIT from IS_VLA to decl name Qing Zhao
2022-01-11 13:53 ` Richard Biener
2022-01-11 15:38 ` Qing Zhao
2022-01-11 23:22 ` Qing Zhao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).