On Sat, Jun 18, 2016 at 09:46:17PM +0200, Bernhard Reutner-Fischer wrote:
> On December 3, 2015 10:46:09 AM GMT+01:00, Janne Blomqvist
> <blomqvist.ja...@gmail.com> wrote:
> >On Tue, Dec 1, 2015 at 6:51 PM, Bernhard Reutner-Fischer
> ><rep.dot....@gmail.com> wrote:
> >> On 1 December 2015 at 15:52, Janne Blomqvist
> ><blomqvist.ja...@gmail.com> wrote:
> >>> On Tue, Dec 1, 2015 at 2:54 PM, Bernhard Reutner-Fischer
> >>> <rep.dot....@gmail.com> wrote:
> >>>> These three function used a hardcoded buffer of 100 but would be
> >better
> >>>> off to base off GFC_MAX_SYMBOL_LEN which denotes the maximum length
> >of a
> >>>> name in any of our supported standards (63 as of f2003 ff.).
> >>>
> >>> Please use xasprintf() instead (and free the result, or course). One
> >>> of my backburner projects is to get rid of these static symbol
> >>> buffers, and use dynamic buffers (or the symbol table) instead. We
> >>> IIRC already have some ugly hacks by using hashing to get around
> >>> GFC_MAX_SYMBOL_LEN when handling mangled symbols. Your patch doesn't
> >>> make the situation worse per se, but if you're going to fix it, lets
> >>> do it properly.
> >>
> >> I see.
> >>
> >> /scratch/src/gcc-6.0.mine/gcc/fortran$ git grep
> >> "^[[:space:]]*char[[:space:]][[:space:]]*[^[;[:space:]]*\[" | wc -l
> >> 142
> >> /scratch/src/gcc-6.0.mine/gcc/fortran$ git grep "xasprintf" | wc -l
> >> 32
> >
> >Yes, that's why it's on the TODO-list rather than on the DONE-list. :)
> >
> >> What about memory fragmentation when switching to heap-based
> >allocation?
> >> Or is there consensus that these are in the noise compared to other
> >> parts of the compiler?
> >
> >Heap fragmentation is an issue, yes. I'm not sure it's that
> >performance-critical, but I don't think there is any consensus. I just
> >want to avoid ugly hacks like symbol hashing to fit within some fixed
> >buffer. Perhaps an good compromise would be something like std::string
> >with small string optimization, but as you have seen there is some
> >resistance to C++. But this is more relevant for mangled symbols, so
> >GFC_MAX_MANGLED_SYMBOL_LEN is more relevant here, and there's only a
> >few of them left. So, well, if you're sure that mangled symbols are
> >never copied into the buffers your patch modifies, please consider
> >your original patch Ok as well. Whichever you prefer.
> >
> >Performance-wise I think a bigger benefit would be to use the symbol
> >table more and then e.g. be able to do pointer comparisons rather than
> >strcmp(). But that is certainly much more work.
>
> Hm, worth a look indeed since that would certainly be a step in the right
> direction.
Installed the initial patch as intermediate step as r253881 for now.
thanks,
>
> >
> >> BTW:
> >> $ git grep APO
> >> io.c: static const char *delim[] = { "APOSTROPHE", "QUOTE", "NONE",
> >NULL };
> >> io.c: static const char *delim[] = { "APOSTROPHE", "QUOTE", "NONE",
> >NULL };
> >
> >? What are you saying?
>
> delim is duplicated, we should remove one instance.
> thanks,
>
