2014-11-25 14:11 GMT+03:00 Richard Biener <richard.guent...@gmail.com>:
> On Tue, Nov 25, 2014 at 11:19 AM, Ilya Enkovich <enkovich....@gmail.com>
> wrote:
>> 2014-11-25 12:43 GMT+03:00 Richard Biener <richard.guent...@gmail.com>:
>>> On Tue, Nov 25, 2014 at 9:45 AM, Ilya Enkovich <enkovich....@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.enkov...@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.enkov...@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:
>
> Ok, then it's get_for_asmname (). That said - the above loops look
> bogus to me. Honza - any better ideas?
get_for_asmname () returns the first element in a chain of nodes with
the same asm name. May I rely on the order of nodes in this chain?
Probably use ASSEMBLER_NAME as a key in chkp_static_var_bounds hash?
Thanks,
Ilya
>
> Richard.
>
>> (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. */
