> On 11 August 2016 at 18:25, Jan Hubicka <hubi...@ucw.cz> wrote:
> >> @@ -266,6 +267,38 @@ private:
> >> bool meet_with_1 (unsigned new_align, unsigned new_misalign);
> >> };
> >>
> >> +/* Lattice of known bits, only capable of holding one value.
> >> + Similar to ccp_lattice_t, mask represents which bits of value are
> >> constant.
> >> + If a bit in mask is set to 0, then the corresponding bit in
> >> + value is known to be constant. */
> >> +
> >> +class ipcp_bits_lattice
> >> +{
> >> +public:
> >> + bool bottom_p () { return m_lattice_val == IPA_BITS_VARYING; }
> >> + bool top_p () { return m_lattice_val == IPA_BITS_UNDEFINED; }
> >> + bool constant_p () { return m_lattice_val == IPA_BITS_CONSTANT; }
> >> + bool set_to_bottom ();
> >> + bool set_to_constant (widest_int, widest_int);
> >> +
> >> + widest_int get_value () { return m_value; }
> >> + widest_int get_mask () { return m_mask; }
> >> +
> >> + bool meet_with (ipcp_bits_lattice& other, unsigned, signop,
> >> + enum tree_code, tree);
> >> +
> >> + bool meet_with (widest_int, widest_int, unsigned);
> >> +
> >> + void print (FILE *);
> >> +
> >> +private:
> >> + enum { IPA_BITS_UNDEFINED, IPA_BITS_CONSTANT, IPA_BITS_VARYING }
> >> m_lattice_val;
> >> + widest_int m_value, m_mask;
> >
> > Please add comment for these, like one in tree-ssa-ccp and mention they are
> > the same
> > values.
> >
> >> +
> >> + /* For K&R C programs, ipa_get_type() could return NULL_TREE.
> >> + Avoid the transform for these cases. */
> >> + if (!parm_type)
> >> + return dest_lattice->set_to_bottom ();
> >
> > Please add TDF_DETAILS dump for this so we notice if we drop useful info
> > for no
> > good reasons. It also happens for variadic functions but hopefully not
> > much more.
> >
> > The patch is OK with those changes.
> Hi,
> The patch broke bootstrap due to segfault while compiling
> libsupc++/eh_alloc.cc
> in ipa_get_type() because callee_info->descriptors had 0 length in
> propagate_bits_accross_call.
>
> After debugging a bit, I realized it was incorrect to use cs->callee and
> using cs->callee->function_symbol() fixed it:
> (that seemed to match with value of 'callee' variable in
> propagate_constants_accross_call).
Yes, callee may be alias and in that case you want to look into its target.
>
> - struct ipa_node_params *callee_info = IPA_NODE_REF (cs->callee);
> + enum availability availability;
> + cgraph_node *callee = cs->callee->function_symbol (&availability);
> + struct ipa_node_params *callee_info = IPA_NODE_REF (callee);
> tree parm_type = ipa_get_type (callee_info, idx);
>
> Similarly I wonder if cs->caller->function_symbol() should be used
> instead of cs->caller in following place while obtaining lattices of
> source param ?
>
> if (jfunc->type == IPA_JF_PASS_THROUGH)
> {
> struct ipa_node_params *caller_info = IPA_NODE_REF (cs->caller);
For callers you do not need to do that, because only real functions can call
(not aliases).
>
>
> The patch segfaults with -flto for gcc.c-torture/execute/920302-1.c in
> ipcp_propagate_stage ()
> while populating info->descriptors[k].decl_or_type because t becomes
> NULL and we dereference
> it with TREE_VALUE (t)
> The test-case has K&R style param declaration.
> The following change seems to fix it for me:
>
> @@ -3235,7 +3235,7 @@ ipcp_propagate_stage (struct ipa_topo_info *topo)
> if (in_lto_p)
> {
> tree t = TYPE_ARG_TYPES (TREE_TYPE (node->decl));
> - for (int k = 0; k < ipa_get_param_count (info); k++)
> + for (int k = 0; k < ipa_get_param_count (info) && t; k++)
> {
> gcc_assert (t != void_list_node);
> info->descriptors[k].decl_or_type = TREE_VALUE (t);
>
> Is that change OK ?
Yes, this also looks fine to me.
Honza
>
> PS: I am on vacation for next week, will get back to working on the
> patch after returning.
>
> Thanks,
> Prathamesh
> >
> > thanks,
> > Honza
