Message ID | alpine.LSU.2.11.1407311359250.20733@zhemvz.fhfr.qr |
---|---|
State | New |
Headers | show |
On Thu, 31 Jul 2014, Richard Biener wrote: > > The following fixes PR61950 by properly initializing the _size field > of the static constructor for the vtable init member. Previously > gfortran would use a 64bit integer to initialize the 32bit size field > (not sure why larger aggregates are not considered). > > This breaks more sophisticated constant-folding and previously > inhibited constant folding (which would have worked in this case > been there no mismatch between initializer and field). > > Bootstrap and regtest ongoing on powerpc64-linux-gnu, ok? I'm not > considering a backport as it is only a missed optimization there. Ping? Thanks, Richard. > Thanks, > Richard. > > 2014-07-31 Richard Biener <rguenther@suse.de> > > * trans-expr.c (gfc_conv_structure): Initialize _size with > a value of proper type. > > Index: gcc/fortran/trans-expr.c > =================================================================== > --- gcc/fortran/trans-expr.c (revision 213342) > +++ gcc/fortran/trans-expr.c (working copy) > @@ -6260,7 +6260,9 @@ gfc_conv_structure (gfc_se * se, gfc_exp > else if (cm->ts.u.derived && strcmp (cm->name, "_size") == 0) > { > val = TYPE_SIZE_UNIT (gfc_get_derived_type (cm->ts.u.derived)); > - CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, val); > + CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, > + fold_convert (TREE_TYPE (cm->backend_decl), > + val)); > } > else > { >
Hi Richard, sorry for the belated reply due to spending lazily my time at the sea ... The patch is okay - and I would even claim that it is obvious. Regarding the data type, I have no idea why it uses a hard-coded 32bit integer instead of a size-type one. I have added it as item to https://gcc.gnu.org/wiki/LibgfortranAbiCleanup to change it once we break the ABI. Tobias PS: I wonder where all the other patch reviewers are, given that quite a few patches have accumulated. In particular, I am lacking a patch reviewer myself. Richard Biener wrote: > The following fixes PR61950 by properly initializing the _size field > of the static constructor for the vtable init member. Previously > gfortran would use a 64bit integer to initialize the 32bit size field > (not sure why larger aggregates are not considered). > > This breaks more sophisticated constant-folding and previously > inhibited constant folding (which would have worked in this case > been there no mismatch between initializer and field). > > Bootstrap and regtest ongoing on powerpc64-linux-gnu, ok? I'm not > considering a backport as it is only a missed optimization there. > > Thanks, > Richard. > > 2014-07-31 Richard Biener <rguenther@suse.de> > > * trans-expr.c (gfc_conv_structure): Initialize _size with > a value of proper type. > > Index: gcc/fortran/trans-expr.c > =================================================================== > --- gcc/fortran/trans-expr.c (revision 213342) > +++ gcc/fortran/trans-expr.c (working copy) > @@ -6260,7 +6260,9 @@ gfc_conv_structure (gfc_se * se, gfc_exp > else if (cm->ts.u.derived && strcmp (cm->name, "_size") == 0) > { > val = TYPE_SIZE_UNIT (gfc_get_derived_type (cm->ts.u.derived)); > - CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, val); > + CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, > + fold_convert (TREE_TYPE (cm->backend_decl), > + val)); > } > else > { >
Dear Richie and Tobias, I OK'd this patch on 9th August but I now see that the posting bounced because of mime content emanating from my phone mail reader :-( I also thought that the patch is obvious. Cheers Paul On 13 August 2014 23:22, Tobias Burnus <burnus@net-b.de> wrote: > Hi Richard, > > sorry for the belated reply due to spending lazily my time at the sea ... > > The patch is okay - and I would even claim that it is obvious. > > Regarding the data type, I have no idea why it uses a hard-coded 32bit > integer instead of a size-type one. I have added it as item to > https://gcc.gnu.org/wiki/LibgfortranAbiCleanup to change it once we break > the ABI. > > Tobias > > PS: I wonder where all the other patch reviewers are, given that quite a few > patches have accumulated. In particular, I am lacking a patch reviewer > myself. > > > Richard Biener wrote: >> >> The following fixes PR61950 by properly initializing the _size field >> of the static constructor for the vtable init member. Previously >> gfortran would use a 64bit integer to initialize the 32bit size field >> (not sure why larger aggregates are not considered). >> >> This breaks more sophisticated constant-folding and previously >> inhibited constant folding (which would have worked in this case >> been there no mismatch between initializer and field). >> >> Bootstrap and regtest ongoing on powerpc64-linux-gnu, ok? I'm not >> considering a backport as it is only a missed optimization there. >> >> Thanks, >> Richard. >> >> 2014-07-31 Richard Biener <rguenther@suse.de> >> >> * trans-expr.c (gfc_conv_structure): Initialize _size with >> a value of proper type. >> >> Index: gcc/fortran/trans-expr.c >> =================================================================== >> --- gcc/fortran/trans-expr.c (revision 213342) >> +++ gcc/fortran/trans-expr.c (working copy) >> @@ -6260,7 +6260,9 @@ gfc_conv_structure (gfc_se * se, gfc_exp >> else if (cm->ts.u.derived && strcmp (cm->name, "_size") == 0) >> { >> val = TYPE_SIZE_UNIT (gfc_get_derived_type (cm->ts.u.derived)); >> - CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, val); >> + CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, >> + fold_convert (TREE_TYPE >> (cm->backend_decl), >> + val)); >> } >> else >> { >> >
On Thu, 14 Aug 2014, Paul Richard Thomas wrote: > Dear Richie and Tobias, > > I OK'd this patch on 9th August but I now see that the posting bounced > because of mime content emanating from my phone mail reader :-( > > I also thought that the patch is obvious. I already applied it after your approval. I wasn't sure about the obviousness because of the 32bit thing (the actual bug may be different). Richard. > Cheers > > Paul > > > On 13 August 2014 23:22, Tobias Burnus <burnus@net-b.de> wrote: > > Hi Richard, > > > > sorry for the belated reply due to spending lazily my time at the sea ... > > > > The patch is okay - and I would even claim that it is obvious. > > > > Regarding the data type, I have no idea why it uses a hard-coded 32bit > > integer instead of a size-type one. I have added it as item to > > https://gcc.gnu.org/wiki/LibgfortranAbiCleanup to change it once we break > > the ABI. > > > > Tobias > > > > PS: I wonder where all the other patch reviewers are, given that quite a few > > patches have accumulated. In particular, I am lacking a patch reviewer > > myself. > > > > > > Richard Biener wrote: > >> > >> The following fixes PR61950 by properly initializing the _size field > >> of the static constructor for the vtable init member. Previously > >> gfortran would use a 64bit integer to initialize the 32bit size field > >> (not sure why larger aggregates are not considered). > >> > >> This breaks more sophisticated constant-folding and previously > >> inhibited constant folding (which would have worked in this case > >> been there no mismatch between initializer and field). > >> > >> Bootstrap and regtest ongoing on powerpc64-linux-gnu, ok? I'm not > >> considering a backport as it is only a missed optimization there. > >> > >> Thanks, > >> Richard. > >> > >> 2014-07-31 Richard Biener <rguenther@suse.de> > >> > >> * trans-expr.c (gfc_conv_structure): Initialize _size with > >> a value of proper type. > >> > >> Index: gcc/fortran/trans-expr.c > >> =================================================================== > >> --- gcc/fortran/trans-expr.c (revision 213342) > >> +++ gcc/fortran/trans-expr.c (working copy) > >> @@ -6260,7 +6260,9 @@ gfc_conv_structure (gfc_se * se, gfc_exp > >> else if (cm->ts.u.derived && strcmp (cm->name, "_size") == 0) > >> { > >> val = TYPE_SIZE_UNIT (gfc_get_derived_type (cm->ts.u.derived)); > >> - CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, val); > >> + CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, > >> + fold_convert (TREE_TYPE > >> (cm->backend_decl), > >> + val)); > >> } > >> else > >> { > >> > > > > > >
Index: gcc/fortran/trans-expr.c =================================================================== --- gcc/fortran/trans-expr.c (revision 213342) +++ gcc/fortran/trans-expr.c (working copy) @@ -6260,7 +6260,9 @@ gfc_conv_structure (gfc_se * se, gfc_exp else if (cm->ts.u.derived && strcmp (cm->name, "_size") == 0) { val = TYPE_SIZE_UNIT (gfc_get_derived_type (cm->ts.u.derived)); - CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, val); + CONSTRUCTOR_APPEND_ELT (v, cm->backend_decl, + fold_convert (TREE_TYPE (cm->backend_decl), + val)); } else {