This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, PR63995, CHKP] Use single static bounds var for varpool nodes sharing asm name
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 25 Nov 2014 13:19:30 +0300
- Subject: Re: [PATCH, PR63995, CHKP] Use single static bounds var for varpool nodes sharing asm name
- Authentication-results: sourceware.org; auth=none
- References: <20141125084543 dot GD9490 at msticlxl57 dot ims dot intel dot com> <CAFiYyc14BPU3pwQT_02vvTsOFTHbdZji+AnHhnvWf3+-dEooyg at mail dot gmail dot com>
2014-11-25 12:43 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
> On Tue, Nov 25, 2014 at 9:45 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> Hi,
>>
>> This patch partly fixes PR bootstrap/63995 by avoiding duplicating static bounds vars. With this fix bootstrap still fails at stage 2 and 3 comparison.
>>
>> Bootstrapped and checked on x86_64-unknown-linux-gnu. OK for trunk?
>>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2014-11-25 Ilya Enkovich <ilya.enkovich@intel.com>
>>
>> PR bootstrap/63995
>> * tree-chkp (chkp_make_static_bounds): Share bounds var
>> between nodes sharing assembler name.
>>
>> gcc/testsuite
>>
>> 2014-11-25 Ilya Enkovich <ilya.enkovich@intel.com>
>>
>> PR bootstrap/63995
>> * g++.dg/dg.exp: Add mpx-dg.exp.
>> * g++.dg/pr63995-1.C: New.
>>
>>
>> diff --git a/gcc/testsuite/g++.dg/dg.exp b/gcc/testsuite/g++.dg/dg.exp
>> index 14beae1..44eab0c 100644
>> --- a/gcc/testsuite/g++.dg/dg.exp
>> +++ b/gcc/testsuite/g++.dg/dg.exp
>> @@ -18,6 +18,7 @@
>>
>> # Load support procs.
>> load_lib g++-dg.exp
>> +load_lib mpx-dg.exp
>>
>> # If a testcase doesn't have special options, use these.
>> global DEFAULT_CXXFLAGS
>> diff --git a/gcc/testsuite/g++.dg/pr63995-1.C b/gcc/testsuite/g++.dg/pr63995-1.C
>> new file mode 100644
>> index 0000000..82e7606
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/pr63995-1.C
>> @@ -0,0 +1,16 @@
>> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
>> +/* { dg-require-effective-target mpx } */
>> +/* { dg-options "-O2 -g -fcheck-pointer-bounds -mmpx" } */
>> +
>> +int test1 (int i)
>> +{
>> + extern const int arr[10];
>> + return arr[i];
>> +}
>> +
>> +extern const int arr[10];
>> +
>> +int test2 (int i)
>> +{
>> + return arr[i];
>> +}
>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>> index 3e38691..d425084 100644
>> --- a/gcc/tree-chkp.c
>> +++ b/gcc/tree-chkp.c
>> @@ -2727,9 +2727,29 @@ chkp_make_static_bounds (tree obj)
>> /* First check if we already have required var. */
>> if (chkp_static_var_bounds)
>> {
>> - slot = chkp_static_var_bounds->get (obj);
>> - if (slot)
>> - return *slot;
>> + /* If there is a symbol sharing assembler name with obj,
>> + we may use its bounds. */
>> + if (TREE_CODE (obj) == VAR_DECL)
>> + {
>> + varpool_node *node = varpool_node::get_create (obj);
>> +
>> + while (node->previous_sharing_asm_name)
>> + node = (varpool_node *)node->previous_sharing_asm_name;
>> +
>> + while (node)
>> + {
>> + slot = chkp_static_var_bounds->get (node->decl);
>> + if (slot)
>> + return *slot;
>> + node = (varpool_node *)node->next_sharing_asm_name;
>> + }
>
> Hum. varpool_node::get returns the ultimate alias target thus the
> walking shouldn't be necessary. Just
>
> node = varpool_node::get_create (obj);
> slot = chkp_static_var_bounds->get (node->decl);
> if (slot)
> return *slot;
>
> and then making sure to set the decl also for node->decl. I suppose
> it really asks for making chkp_static_var_bounds->get based on
> a varpool node and not a decl so you consistently use the ultimate
> alias target.
varpool_node::get just returns symtab_node::get which returns
decl->decl_with_vis.symtab_node - thus no aliases walkthrough. Also
none of two varpool_nodes is an alias. The only connection between
these nodes seems to be {next,previous}_sharing_asm_name. Here is how
these nodes look:
(gdb) p *$2
$3 = {<symtab_node> = {type = SYMTAB_VARIABLE, resolution =
LDPR_UNKNOWN, definition = 0, alias = 0, weakref = 0,
cpp_implicit_alias = 0, analyzed = 0, writeonly = 0,
refuse_visibility_changes = 0, externally_visible = 0,
no_reorder = 0, force_output = 0, forced_by_abi = 0, unique_name =
0, implicit_section = 0, body_removed = 1, used_from_other_partition =
0, in_other_partition = 0, address_taken = 0, in_init_priority_hash =
0,
need_lto_streaming = 0, offloadable = 0, order = 3, decl =
0x7ffff7dd2cf0, next = 0x7ffff7f46200, previous = 0x7ffff7dd67a8,
next_sharing_asm_name = 0x0, previous_sharing_asm_name =
0x7ffff7f46200, same_comdat_group = 0x0,
ref_list = {references = 0x0, referring = {m_vec = 0x23856b0}},
alias_target = 0x0, lto_file_data = 0x0, aux = 0x0, x_comdat_group =
0x0, x_section = 0x0}, output = 0, need_bounds_init = 0,
dynamically_initialized = 0,
tls_model = TLS_MODEL_NONE, used_by_single_function = 0}
(gdb) p *$5
$6 = {<symtab_node> = {type = SYMTAB_VARIABLE, resolution =
LDPR_UNKNOWN, definition = 0, alias = 0, weakref = 0,
cpp_implicit_alias = 0, analyzed = 0, writeonly = 0,
refuse_visibility_changes = 0, externally_visible = 0,
no_reorder = 0, force_output = 0, forced_by_abi = 0, unique_name =
0, implicit_section = 0, body_removed = 1, used_from_other_partition =
0, in_other_partition = 0, address_taken = 0, in_init_priority_hash =
0,
need_lto_streaming = 0, offloadable = 0, order = 2, decl =
0x7ffff7dd2bd0, next = 0x7ffff7dd6620, previous = 0x7ffff7f46300,
next_sharing_asm_name = 0x7ffff7f46300, previous_sharing_asm_name =
0x0, same_comdat_group = 0x0,
ref_list = {references = 0x0, referring = {m_vec = 0x238bdd0}},
alias_target = 0x0, lto_file_data = 0x0, aux = 0x0, x_comdat_group =
0x0, x_section = 0x0}, output = 0, need_bounds_init = 0,
dynamically_initialized = 0,
tls_model = TLS_MODEL_NONE, used_by_single_function = 0}
Thanks,
Ilya
>
> Richard.
>
>> + }
>> + else
>> + {
>> + slot = chkp_static_var_bounds->get (obj);
>> + if (slot)
>> + return *slot;
>> + }
>> }
>>
>> /* Build decl for bounds var. */