This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 3/5] IPA ICF pass
- From: Jan Hubicka <hubicka at ucw dot cz>
- To: Martin Liška <mliska at suse dot cz>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, gcc-patches at gcc dot gnu dot org
- Date: Sat, 11 Oct 2014 10:26:15 +0200
- Subject: Re: [PATCH 3/5] IPA ICF pass
- Authentication-results: sourceware.org; auth=none
- References: <c5c2463c07186b4ba35b10f3063ecdd8f8d46d63 dot 1402913001 dot git dot mliska at suse dot cz> <ac1da49f0ee78643bc4521580862fa92e1051764 dot 1402913001 dot git dot mliska at suse dot cz> <20140620073156 dot GC12633 at tsaunders-iceball dot corp dot tor1 dot mozilla dot com> <alpine dot LSU dot 2 dot 11 dot 1407052337210 dot 30120 at tuna dot site> <20140705225351 dot GK16837 at kam dot mff dot cuni dot cz> <53C7E626 dot 8080400 at suse dot cz> <54255A09 dot 1090305 at suse dot cz> <20140926222729 dot GE26922 at atrey dot karlin dot mff dot cuni dot cz> <543875BC dot 7030206 at suse dot cz>
>
> Hello.
>
> Yeah, you are right. But even Richard advised me to put it to a single place. Maybe we are a bit
> more strict than it would be necessary. But I hope that's fine ;)
OK, lets do extra checking now and play with this incrementally.
>
> Good point. Do you mean cases like, foo (alias_foo) and bar (alias_bar). If we prove that foo equals to bar, can we also merge aliases?
> I am curious if such comparison can really save something? Are there any interesting cases?
What probably matters is that you recognize the equivalence to know that uses of alias_foo can be merged with uses alias_bar.
Similarly for thunks. Again something to do incrementally I guess.
Honza
>
> Martin
>
>
> >> + case INTEGER_CST:
> >> + {
> >> + ret = types_are_compatible_p (TREE_TYPE (t1), TREE_TYPE (t2))
> >> + && wi::to_offset (t1) == wi::to_offset (t2);
> >
> > tree_int_cst_equal
> >
> >> + case FIELD_DECL:
> >> + {
> >> + tree fctx1 = DECL_FCONTEXT (t1);
> >> + tree fctx2 = DECL_FCONTEXT (t2);
> >
> > DECL_FCONTEXT has no semantic meaning; so you can skip comparing it.
> >> +
> >> + tree offset1 = DECL_FIELD_OFFSET (t1);
> >> + tree offset2 = DECL_FIELD_OFFSET (t2);
> >> +
> >> + tree bit_offset1 = DECL_FIELD_BIT_OFFSET (t1);
> >> + tree bit_offset2 = DECL_FIELD_BIT_OFFSET (t2);
> >> +
> >> + ret = compare_operand (fctx1, fctx2)
> >> + && compare_operand (offset1, offset2)
> >> + && compare_operand (bit_offset1, bit_offset2);
> >
> > You probably want to compare type here?
> >> + case CONSTRUCTOR:
> >> + {
> >> + unsigned len1 = vec_safe_length (CONSTRUCTOR_ELTS (t1));
> >> + unsigned len2 = vec_safe_length (CONSTRUCTOR_ELTS (t2));
> >> +
> >> + if (len1 != len2)
> >> + return false;
> >> +
> >> + for (unsigned i = 0; i < len1; i++)
> >> + if (!sem_variable::equals (CONSTRUCTOR_ELT (t1, i)->value,
> >> + CONSTRUCTOR_ELT (t2, i)->value))
> >> + return false;
> >
> > You want to compare ->index, too.
> >> + case INTEGER_CST:
> >> + return func_checker::types_are_compatible_p (TREE_TYPE (t1), TREE_TYPE (t2),
> >> + true)
> >> + && wi::to_offset (t1) == wi::to_offset (t2);
> > again ;)
> >
> > This is where I stopped for now. Generally the patch seems OK to me with few of these
> > details fixed.
> >
> > Honza
> >