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?

Reply via email to