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 >