> > This thing simply stays in GIMPLE and serves no purpose. I tried to remove
> > it at one point, but run into some regressions. I plan to return to this.
> > Have no idea how this was intended to work.
> > Some bits was added by:
> > https://gcc.gnu.org/ml/gcc-patches/2005-04/txt00100.txt
> > Code in build_objc_method_call seems predating changelogs.
>
> I see. But the above should be harmless unless you throw it onto the ODR
> predicates?
Yes, but it is what I do two lines bellow...
>
> >>
> >> > + /* Match the type information stored in the wrapper. */
> >> > +
> >> > + flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF);
> >> > + return (tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg0))
> >> > + == tree_to_uhwi (OBJ_TYPE_REF_TOKEN (arg1))
> >>
> >> Err, please use tree_int_cst_eq ()
> >
> > OK updated in my local copy and, I will update other places too, this was
> > directly copied from ipa-icf-gimple.
> >>
> >> > + && types_same_for_odr (obj_type_ref_class (arg0),
> >> > + obj_type_ref_class (arg1))
> >>
> >> Why do you need this check? The args of OBJ_TYPE_REF should be fully
> >> specifying the object, no?
> >
> > This is the type of THIS pointer that is extracted from pointer type of
> > OBJ_TYPE_REF. Because we otherwise consider different pointers compatible,
> > I think we need to check it here.
>
> Hum, this is fold-const.c and thus GENERIC. GENERIC doesn't consider pointer
> types compatible in general. So I think it is not needed. As you say
> elsewhere
> the caller is responsible for restricting typeof the trees it asks for
> equality.
...
My understanding is that operand_equal_p should check that the value of operand
is the same and will be the same after subsequentoptimizations.
For this reason it does its own type matching (comparsion of TYPE_MODE for
expressions, TYPE_UNSIGNED where it matters). It also compares alias
information for MEM_REF to be sure that both reads from memory will always
return the same even after later optimizations. OBJ_TYPE_REF type info is no
different from TBAA alias info. It is just additional information which is
used for later optimization and therefore it must match across the two copies.
Modulo the fact that we don't seem to fold these, I think one way to confuse us
is:
test ? ((derivedobject1 *)ptr)->foo () : ((derivedobject *)ptr)->foo ()
Here we have two NOP_EXPR that converts PTR to two derived objects and
OBJ_TYPE_REF
will look equivalent modulo having different type. If return value is the same,
I think it w
Plus operand_equal_p is used for both GENERIC and GIMPLE (which is somewhat
ugly indeed. We may just fork the implementation eventally if wemove away
completely from sharing code paths between GENERIC and GIMPLE).
Ok, I tried to fix my testcase to be optimized and modify it to case where we
need to type match. I can't do that with current tree but I think it is just
a lazyness elsewhere that should be fixed. Doing:
struct foo {
virtual int test () __attribute__ ((pure));
};
int ret (struct foo *f, int v)
{
return v ? f->test() : f->test();
}
We actually match both OBJ_TYPE_REFs in GENERIC folding:
#0 operand_equal_p (arg0=0x7ffff6ad6b10, arg1=0x7ffff6ad6a50,
flags=flags@entry=0) at ../../gcc/fold-const.c:2706
#1 0x00000000009fa979 in operand_equal_p (arg0=arg0@entry=0x7ffff6ae20e0,
arg1=arg1@entry=0x7ffff6ae20a8, flags=flags@entry=0) at
../../gcc/fold-const.c:3130
#2 0x0000000001097e78 in generic_simplify_COND_EXPR (code=COND_EXPR,
op2=0x7ffff6ae20e0, op1=0x7ffff6ae20a8, op0=0x7ffff6c35758,
type=0x7ffff6adb7e0, loc=1042) at generic-match.c:24047
#3 generic_simplify (loc=loc@entry=1042, code=code@entry=COND_EXPR,
type=type@entry=0x7ffff6adb7e0, op0=op0@entry=0x7ffff6c35758,
op1=op1@entry=0x7ffff6ae20a8,
op2=op2@entry=0x7ffff6ae20e0) at generic-match.c:24230
#4 0x0000000000a066cd in fold_ternary_loc (loc=loc@entry=1042,
code=code@entry=COND_EXPR, type=type@entry=0x7ffff6adb7e0, op0=0x7ffff6c35758,
op1=0x7ffff6ae20a8, op2=0x7ffff6ae20e0)
at ../../gcc/fold-const.c:11377
#5 0x0000000000a28c85 in fold (expr=<optimized out>) at
../../gcc/fold-const.c:11996
#6 0x000000000079118a in fold_if_not_in_template (expr=<optimized out>) at
../../gcc/cp/tree.c:4287
This will not result in the desired optimization because we give up in the
otuer operand_equal_p matching CALL_EXPR:
0x00000000009fa979 in operand_equal_p (arg0=arg0@entry=0x7ffff6ae20e0,
arg1=arg1@entry=0x7ffff6ae20a8, flags=flags@entry=0) at
../../gcc/fold-const.c:3130
3130 if (! operand_equal_p (CALL_EXPR_FN (arg0), CALL_EXPR_FN
(arg1),
Value returned is $13 = 1
(gdb) l
3125 }
3126 else
3127 {
3128 /* If the CALL_EXPRs call different functions, then they
are not
3129 equal. */
3130 if (! operand_equal_p (CALL_EXPR_FN (arg0), CALL_EXPR_FN
(arg1),
3131 flags))
3132 return 0;
3133 }
3134
(gdb)
3135 {
3136 unsigned int cef = call_expr_flags (arg0);
3137 if (flags & OEP_PURE_SAME)
3138 cef &= ECF_CONST | ECF_PURE;
3139 else
3140 cef &= ECF_CONST;
3141 if (!cef)
3142 return 0;
3143 }
Here we end up with flags=0 (why? It seems that generic_simplify_COND_EXPR
could be happy
about pure/const functions and in fact it should be happy about side effects,
too,
because only one branc of the ternary operation is going to be executed).
I think for uses where we know only one code path is going to be thaken, we
want OEP_MATCH_SIDE_EFFECTS flag to operand_equal_p telling it that two operands
can be considered equvalent if the side effects are.
Extending my testcase as follows:
struct foo {
virtual int test () __attribute__ ((pure));
};
struct foo1:foo {
virtual int test () __attribute__ ((pure));
};
struct foo2:foo {
virtual int test () __attribute__ ((pure));
};
int ret (struct foo *f, int v)
{
return v ? ((foo1 *)f)->test() : ((foo2 *)f)->test();
}
Will led to false on comparing OBJ_TYPE_REF calling OP_SAME(1) (so no ODR type
matching needed). I think this is just bug (missed optimization at least
with -fno-strict-aliasing): the reason is that we match
<component_ref 0x7ffff6ad6c90
type <record_type 0x7ffff6c3d930 foo addressable tree_2 needs-constructing
type_5 type_6 BLK
size <integer_cst 0x7ffff6ad7e58 constant 64>
unit size <integer_cst 0x7ffff6ad7e70 constant 8>
align 64 symtab 0 alias set -1 canonical type 0x7ffff6c3d930
fields <field_decl 0x7ffff6c3a4c0 _vptr.foo type <pointer_type
0x7ffff6c34540>
unsigned virtual DI file t.C line 3 col 8 size <integer_cst
0x7ffff6ad7e58 64> unit size <integer_cst 0x7ffff6ad7e70 8>
align 64 offset_align 128
offset <integer_cst 0x7ffff6ad7e88 constant 0>
bit offset <integer_cst 0x7ffff6ad7ed0 constant 0> context
<record_type 0x7ffff6c3d930 foo> chain <type_decl 0x7ffff6c3a428 foo>> context
<translation_unit_decl 0x7ffff7ff81e0 D.1>
full-name "struct foo"
needs-constructor X() X(constX&) this=(X&) n_parents=0 use_template=0
interface-unknown
pointer_to_this <pointer_type 0x7ffff6c3dbd0> reference_to_this
<reference_type 0x7ffff6c3de70> chain <type_decl 0x7ffff6c3a390 foo>>
arg 0 <indirect_ref 0x7ffff6c49da0
type <record_type 0x7ffff6c513f0 foo2 addressable tree_2
needs-constructing type_5 type_6 BLK size <integer_cst 0x7ffff6ad7e58 64> unit
size <integer_cst 0x7ffff6ad7e70 8>
align 64 symtab 0 alias set -1 canonical type 0x7ffff6c513f0 fields
<field_decl 0x7ffff6c3aab0 D.2317> context <translation_unit_decl
0x7ffff7ff81e0 D.1>
full-name "struct foo2"
needs-constructor X() X(constX&) this=(X&) n_parents=1
use_template=0 interface-unknown
pointer_to_this <pointer_type 0x7ffff6c51690> reference_to_this
<reference_type 0x7ffff6c51930> chain <type_decl 0x7ffff6c3a980 foo2>>
arg 0 <nop_expr 0x7ffff6c49d60 type <pointer_type 0x7ffff6c51690>
arg 0 <parm_decl 0x7ffff6c4cf00 f>>>
arg 1 <field_decl 0x7ffff6c3aab0 D.2317 type <record_type 0x7ffff6c3d930
foo>
ignored decl_6 BLK file t.C line 9 col 8 size <integer_cst
0x7ffff6ad7e58 64> unit size <integer_cst 0x7ffff6ad7e70 8>
align 64 offset_align 128 offset <integer_cst 0x7ffff6ad7e88 0> bit
offset <integer_cst 0x7ffff6ad7ed0 0> context <record_type 0x7ffff6c513f0 foo2>
chain <type_decl 0x7ffff6c3aa18 foo2 type <record_type 0x7ffff6c51498
foo2>
nonlocal decl_4 VOID file t.C line 9 col 17
align 1 context <record_type 0x7ffff6c513f0 foo2> result
<record_type 0x7ffff6c513f0 foo2>
>>>
Here we mismatch FIELD_DECL because currently we insist on them being
equivalent as declarations. In one expression it comes from foo1 and in other
from foo2.
I think this is just missed optimization (ipa-icf-gimple just compare that
FIELD_DECLS represents same offset and subtype and I think operand_equal_p
should do, too). If you rememember, I have a patch for this in queue.
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg02112.html
With -fno-strict-aliasing I think we ought to match both memory accesses as
they really are equivalent and in longer run we ought to teach ipa-icf-gimple
to merge alias infos in cases it merges memory accesses with mismatching TBAA
info. This code also hits in LTO where two same types ends up not merged
by tree merging.
Honza
