https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63587

--- Comment #9 from Martin Liška <mliska at suse dot cz> ---
On 10/20/2014 09:48 AM, rguenther at suse dot de wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63587
>
> --- Comment #4 from rguenther at suse dot de <rguenther at suse dot de> ---
> On Sun, 19 Oct 2014, mliska at suse dot cz wrote:
>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63587
>>
>> --- Comment #2 from Martin Liška <mliska at suse dot cz> ---
>> Following two functions are merged:
>> static boost::log::make_output_actor<ActorT<LeftExprT>, RightT, ValueT>::type
>> boost::log::make_output_actor<ActorT<LeftExprT>, RightT,
>> ValueT>::make(ActorT<LeftExprT>, RightT&) [with ActorT = boost::actor;
>> LeftExprT = int; RightT = boost::log::attribute_actor<int,
>> boost::log::value_extractor, void, boost::actor>; ValueT = int;
>> boost::log::make_output_actor<ActorT<LeftExprT>, RightT, ValueT>::type =
>> boost::actor<boost::log::attribute_output_terminal<boost::actor<int>,
>> boost::log::to_log_fun> >] (struct actor left, struct attribute_actor & 
>> right)
>>
>>
>> static boost::log::make_output_actor<ActorT<LeftExprT>, RightT, ValueT>::type
>> boost::log::make_output_actor<ActorT<LeftExprT>, RightT,
>> ValueT>::make(ActorT<LeftExprT>, RightT&) [with ActorT = boost::actor;
>> LeftExprT = int; RightT = boost::log::attribute_actor<{anonymous}::my_class,
>> boost::log::value_extractor, void, boost::actor>; ValueT = int;
>> boost::log::make_output_actor<ActorT<LeftExprT>, RightT, ValueT>::type =
>> boost::actor<boost::log::attribute_output_terminal<boost::actor<int>,
>> boost::log::to_log_fun> >] (struct actor left, struct attribute_actor & 
>> right)
>>
>> with following body:
>> {
>>    struct type D.3826;
>>    struct to_log_fun D.3825;
>>    struct attribute_name D.3824;
>>    int SR.9;
>>    struct actor left;
>>
>>    <bb 2>:
>>    left = left;
>>    SR.9_4 = MEM[(struct attribute_terminal *)right_2(D)];
>>    MEM[(struct attribute_name *)&D.3824] = SR.9_4;
>>    boost::log::attribute_output_terminal<boost::actor<int>,
>> boost::log::to_log_fun>::attribute_output_terminal<int> (&D.3826, left, 
>> D.3824,
>> D.3825, 0);
>>    D.3826 ={v} {CLOBBER};
>>    return;
>>
>> }
>>
>>
>>
>> As I was debugging ao_ref_alias_sets, there's MEM_REF where we have different
>> template arguments: attribute_actor<int,...> vs.
>> attribute_actor<{anonymous}::my_class,...>.
>> What do you think Richard about these record_types from alias set 
>> perspective:
>>
>> (gdb) p debug_tree(t1)
>>   <mem_ref 0x7ffff6ab4000
>>      type <integer_type 0x7ffff6c33690 int public type_6 SI
>>          size <integer_cst 0x7ffff6c51048 constant 32>
>>          unit size <integer_cst 0x7ffff6c51060 constant 4>
>>          align 32 symtab 0 alias set 4 canonical type 0x7ffff6c33690 
>> precision
>> 32 min <integer_cst 0x7ffff6c51000 -2147483648> max <integer_cst 
>> 0x7ffff6c51018
>> 2147483647>
>>          pointer_to_this <pointer_type 0x7ffff6c55738>>
>>
>>      arg 0 <ssa_name 0x7ffff6aae678
>>          type <reference_type 0x7ffff6e20d20 type <record_type 0x7ffff6de7dc8
>> attribute_actor>
>>              unsigned DI
>>              size <integer_cst 0x7ffff6c2fdf8 constant 64>
>>              unit size <integer_cst 0x7ffff6c2fe10 constant 8>
>>              align 64 symtab 0 alias set 7 canonical type 0x7ffff6e20d20>
>>          visited var <parm_decl 0x7ffff6e1eb00 right>def_stmt GIMPLE_NOP
>>
>>          version 2
>>          ptr-info 0x7ffff6a7e3d8>
>>      arg 1 <integer_cst 0x7ffff6a4ee28 type <pointer_type 0x7ffff6dbfa80>
>> constant 0>>
>> $1 = void
>> (gdb) p debug_tree(t2)
>>   <mem_ref 0x7ffff6aa1ac8
>>      type <integer_type 0x7ffff6c33690 int public type_6 SI
>>          size <integer_cst 0x7ffff6c51048 constant 32>
>>          unit size <integer_cst 0x7ffff6c51060 constant 4>
>>          align 32 symtab 0 alias set 4 canonical type 0x7ffff6c33690 
>> precision
>> 32 min <integer_cst 0x7ffff6c51000 -2147483648> max <integer_cst 
>> 0x7ffff6c51018
>> 2147483647>
>>          pointer_to_this <pointer_type 0x7ffff6c55738>>
>>
>>      arg 0 <ssa_name 0x7ffff6a77dc8
>>          type <reference_type 0x7ffff6e20540 type <record_type 0x7ffff6ddd888
>> attribute_actor>
>>              unsigned DI
>>              size <integer_cst 0x7ffff6c2fdf8 constant 64>
>>              unit size <integer_cst 0x7ffff6c2fe10 constant 8>
>>              align 64 symtab 0 alias set 7 canonical type 0x7ffff6e20540>
>>          visited var <parm_decl 0x7ffff6e1ea00 right>def_stmt GIMPLE_NOP
>>
>>          version 2
>>          ptr-info 0x7ffff6a7e300>
>>      arg 1 <integer_cst 0x7ffff6a4ee28 type <pointer_type 0x7ffff6dbfa80>
>> constant 0>>
>>
>> these types are called for alias_set comparison, with following record_types:
>> (gdb) p debug_tree((tree_node*)0x7ffff6de7dc8)
>>   <record_type 0x7ffff6de7dc8 attribute_actor needs-constructing type_5 
>> type_6
>> SI
>>      size <integer_cst 0x7ffff6c51048 type <integer_type 0x7ffff6c33150
>> bitsizetype> constant 32>
>>      unit size <integer_cst 0x7ffff6c51060 type <integer_type 0x7ffff6c330a8
>> sizetype> constant 4>
>>      align 32 symtab 0 alias set 17 canonical type 0x7ffff6de7dc8
>>      fields <field_decl 0x7ffff6de4ed8 D.2798
>>          type <record_type 0x7ffff6dddb28 actor needs-constructing type_5 
>> type_6
>> SI size <integer_cst 0x7ffff6c51048 32> unit size <integer_cst 0x7ffff6c51060
>> 4>
>>              align 32 symtab 0 alias set 15 canonical type 0x7ffff6dddb28 
>> fields
>> <field_decl 0x7ffff6de01c8 proto_expr_> context <namespace_decl 
>> 0x7ffff6d8d2f8
>> boost>
>>              full-name "struct boost::actor<boost::log::attribute_terminal>"
>>              needs-constructor X() X(constX&) this=(X&) n_parents=0
>> use_template=1 interface-unknown
>>              pointer_to_this <pointer_type 0x7ffff6e03930> reference_to_this
>> <reference_type 0x7ffff6dff0a8> chain <type_decl 0x7ffff6de0098 actor>>
>>          ignored decl_6 SI file ../../PR33754.c line 167 col 7 size 
>> <integer_cst
>> 0x7ffff6c51048 32> unit size <integer_cst 0x7ffff6c51060 4>
>>          align 32 offset_align 128
>>          offset <integer_cst 0x7ffff6c2fe28 constant 0>
>>          bit offset <integer_cst 0x7ffff6c2fe70 constant 0> context 
>> <record_type
>> 0x7ffff6de7dc8 attribute_actor>
>>          chain <type_decl 0x7ffff6de4da8 attribute_actor type <record_type
>> 0x7ffff6de80a8 attribute_actor>
>>              external nonlocal suppress-debug decl_4 VOID file 
>> ../../PR33754.c
>> line 168 col 1
>>              align 8 context <record_type 0x7ffff6de7dc8 attribute_actor> 
>> result
>> <record_type 0x7ffff6de7dc8 attribute_actor>
>>              chain <type_decl 0x7ffff6de4e40 value_type>>> context
>> <namespace_decl 0x7ffff6dbec78 log>
>>      full-name "class boost::log::attribute_actor<int,
>> boost::log::value_extractor, void, boost::actor>"
>>      needs-constructor X() X(constX&) this=(X&) n_parents=1 use_template=1
>> interface-unknown
>>      pointer_to_this <pointer_type 0x7ffff6de81f8> reference_to_this
>> <reference_type 0x7ffff6e20d20> chain <type_decl 0x7ffff6de4d10
>> attribute_actor>>
>> $3 = void
>> (gdb) p debug_tree((tree_node*)0x7ffff6ddd888)
>>   <record_type 0x7ffff6ddd888 attribute_actor needs-constructing type_5 
>> type_6
>> SI
>>      size <integer_cst 0x7ffff6c51048 type <integer_type 0x7ffff6c33150
>> bitsizetype> constant 32>
>>      unit size <integer_cst 0x7ffff6c51060 type <integer_type 0x7ffff6c330a8
>> sizetype> constant 4>
>>      align 32 symtab 0 alias set 14 canonical type 0x7ffff6ddd888
>>      fields <field_decl 0x7ffff6de0b48 D.2756
>>          type <record_type 0x7ffff6dddb28 actor needs-constructing type_5 
>> type_6
>> SI size <integer_cst 0x7ffff6c51048 32> unit size <integer_cst 0x7ffff6c51060
>> 4>
>>              align 32 symtab 0 alias set 15 canonical type 0x7ffff6dddb28 
>> fields
>> <field_decl 0x7ffff6de01c8 proto_expr_> context <namespace_decl 
>> 0x7ffff6d8d2f8
>> boost>
>>              full-name "struct boost::actor<boost::log::attribute_terminal>"
>>              needs-constructor X() X(constX&) this=(X&) n_parents=0
>> use_template=1 interface-unknown
>>              pointer_to_this <pointer_type 0x7ffff6e03930> reference_to_this
>> <reference_type 0x7ffff6dff0a8> chain <type_decl 0x7ffff6de0098 actor>>
>>          ignored decl_6 SI file ../../PR33754.c line 167 col 7 size 
>> <integer_cst
>> 0x7ffff6c51048 32> unit size <integer_cst 0x7ffff6c51060 4>
>>          align 32 offset_align 128
>>          offset <integer_cst 0x7ffff6c2fe28 constant 0>
>>          bit offset <integer_cst 0x7ffff6c2fe70 constant 0> context 
>> <record_type
>> 0x7ffff6ddd888 attribute_actor>
>>          chain <type_decl 0x7ffff6de0a18 attribute_actor type <record_type
>> 0x7ffff6de12a0 attribute_actor>
>>              external nonlocal suppress-debug decl_4 VOID file 
>> ../../PR33754.c
>> line 168 col 1
>>              align 8 context <record_type 0x7ffff6ddd888 attribute_actor> 
>> result
>> <record_type 0x7ffff6ddd888 attribute_actor>
>>              chain <type_decl 0x7ffff6de0ab0 value_type>>> context
>> <namespace_decl 0x7ffff6dbec78 log>
>>      full-name "class boost::log::attribute_actor<{anonymous}::my_class,
>> boost::log::value_extractor, void, boost::actor>"
>>      needs-constructor X() X(constX&) this=(X&) n_parents=1 use_template=1
>> interface-unknown
>>      pointer_to_this <pointer_type 0x7ffff6de13f0> reference_to_this
>> <reference_type 0x7ffff6e20540> chain <type_decl 0x7ffff6dd2ed8
>> attribute_actor>>
>>
>> If the alias sets can be considered to be equal, we must face some latent bug
>> in inliner?
> Maybe.  Btw, I see some issues in func_checker::compare_operand with
> regard to it doing redundant checks:
>
>          if (!func_checker::compatible_types_p (TREE_TYPE (x1), TREE_TYPE
> (x2)))
>            return return_false ();
>
> Redundant with the following
>
>          if (!compare_operand (x1, x2))
>            return return_false_with_msg ("");
>
> ...
>
>          if (get_alias_set (TREE_TYPE (y1)) != get_alias_set (TREE_TYPE
> (y2)))
>            return return_false_with_msg ("alias set for MEM_REF offsets are
> different");
>
> Redundant with the following
>
>          ao_ref r1, r2;
>          ao_ref_init (&r1, t1);
>          ao_ref_init (&r2, t2);
>          if (ao_ref_alias_set (&r1) != ao_ref_alias_set (&r2)
>              || ao_ref_base_alias_set (&r1) != ao_ref_base_alias_set (&r2))
>            return return_false_with_msg ("ao alias sets are different");
>
>
>          /* Type of the offset on MEM_REF does not matter.  */
>          return wi::to_offset  (y1) == wi::to_offset  (y2);
>
> Use tree_int_cst_equal (y1, y2).
>
>      case COMPONENT_REF:
>        {
>          x1 = TREE_OPERAND (t1, 0);
>          x2 = TREE_OPERAND (t2, 0);
>          y1 = TREE_OPERAND (t1, 1);
>          y2 = TREE_OPERAND (t2, 1);
>
>          ret = compare_operand (x1, x2)
>                && compare_operand (y1, y2);
>
> comparing FIELD_DECLs like you do looks dangerous to me (and it looks
> redundant with the get_addr_base_and_unit_offset you do - which btw
> also defeats the ao_ref alias checks as you effectively drop the
> get_alias_set () compare).
>
> IMHO folding memory access comparison and general operand comparison
> into one function makes it very much less clear what is going on...
>
> So - can you please split this into the general compare_operand
> and a compare_memory_access function?

Hello.

I come up with first version of transformation (ipa-icf-gimple.c 
attachment).
I would like to ask which TREE_CODE reside in the newly create function. 
MEM_REF must be there
to introduce a single place where alias_set comparison will be called. 
I'm wondering about get_addr_base_and_unit_offset, for which group of 
operations should I call it?

If you remember a patch where I added support for volatility check, 
should it be really
placed to compare_memory_access, or would be efficient handling in 
gimple_call/assign check
function?

Thank you,
Martin


>

Reply via email to