| Message ID | 20121104223946.GC5617@virgil.arch.suse.de |
|---|---|
| State | New |
| Headers | show |
> Hi, > > the patch below disables generation of aggregate jump functions from > bit-field stores because currently we depend on type size of the value > to determine the size of the stored value and that does not work with > bit-fields, making it impossible for IPA-CP to organize them in their > lattices. > > If we ever decide aggregate jump functions for bit-fields are worth > the hassle, we might remove this limitation by storing and streaming > the size of the memory reference alongside the offset in the jump > functions (and IPA-CP lattices). > > Bootstrapped and tested on x86_64-linux, needed for the aggregate > IPA-CP. OK for trunk? > > Thanks, > > Martin > > > 2012-11-02 Martin Jambor <mjambor@suse.cz> > > * ipa-prop.c (determine_known_aggregate_parts): Do not create > aggregate jump functions for bit-fields. > > Index: src/gcc/ipa-prop.c > =================================================================== > --- src.orig/gcc/ipa-prop.c > +++ src/gcc/ipa-prop.c > @@ -1295,7 +1295,10 @@ determine_known_aggregate_parts (gimple > > lhs = gimple_assign_lhs (stmt); > rhs = gimple_assign_rhs1 (stmt); > - if (!is_gimple_reg_type (rhs)) > + if (!is_gimple_reg_type (rhs) > + || TREE_CODE (lhs) == BIT_FIELD_REF > + || (TREE_CODE (lhs) == COMPONENT_REF > + && DECL_BIT_FIELD (TREE_OPERAND (lhs, 1)))) I am not sure I understand motivation of this patch properly. First I think BIT_FIELD_REF can be hidden inside chain of other REFs so you should probably look into all handled components. What exactly goes wrong when your type size is bigger than the bitfield? Honza
On Mon, Nov 05, 2012 at 05:20:46PM +0100, Jan Hubicka wrote: > > Hi, > > > > the patch below disables generation of aggregate jump functions from > > bit-field stores because currently we depend on type size of the value > > to determine the size of the stored value and that does not work with > > bit-fields, making it impossible for IPA-CP to organize them in their > > lattices. > > > > If we ever decide aggregate jump functions for bit-fields are worth > > the hassle, we might remove this limitation by storing and streaming > > the size of the memory reference alongside the offset in the jump > > functions (and IPA-CP lattices). > > > > Bootstrapped and tested on x86_64-linux, needed for the aggregate > > IPA-CP. OK for trunk? > > > > Thanks, > > > > Martin > > > > > > 2012-11-02 Martin Jambor <mjambor@suse.cz> > > > > * ipa-prop.c (determine_known_aggregate_parts): Do not create > > aggregate jump functions for bit-fields. > > > > Index: src/gcc/ipa-prop.c > > =================================================================== > > --- src.orig/gcc/ipa-prop.c > > +++ src/gcc/ipa-prop.c > > @@ -1295,7 +1295,10 @@ determine_known_aggregate_parts (gimple > > > > lhs = gimple_assign_lhs (stmt); > > rhs = gimple_assign_rhs1 (stmt); > > - if (!is_gimple_reg_type (rhs)) > > + if (!is_gimple_reg_type (rhs) > > + || TREE_CODE (lhs) == BIT_FIELD_REF > > + || (TREE_CODE (lhs) == COMPONENT_REF > > + && DECL_BIT_FIELD (TREE_OPERAND (lhs, 1)))) > > I am not sure I understand motivation of this patch properly. First I think > BIT_FIELD_REF can be hidden inside chain of other REFs so you should probably > look into all handled components. What exactly goes wrong when your type size > is bigger than the bitfield? > I hit an assert in the patch I am about to post (I am writing the changelog right now) I hit an assert checking that stuff does not overlap in IPA-CP aggregate lattices. My initial reaction was that I did not want to deal with bit-fields, unless really necessary ;-) However, that assert can be easily changed to bottomizing the lattice, I will re-test with that and if there are no problems, this patch can be omitted. Thanks, Martin
Index: src/gcc/ipa-prop.c =================================================================== --- src.orig/gcc/ipa-prop.c +++ src/gcc/ipa-prop.c @@ -1295,7 +1295,10 @@ determine_known_aggregate_parts (gimple lhs = gimple_assign_lhs (stmt); rhs = gimple_assign_rhs1 (stmt); - if (!is_gimple_reg_type (rhs)) + if (!is_gimple_reg_type (rhs) + || TREE_CODE (lhs) == BIT_FIELD_REF + || (TREE_CODE (lhs) == COMPONENT_REF + && DECL_BIT_FIELD (TREE_OPERAND (lhs, 1)))) break; lhs_base = get_ref_base_and_extent (lhs, &lhs_offset, &lhs_size,