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?
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. */
