| Message ID | 56B65596.3080800@netcologne.de |
|---|---|
| State | New |
| Headers | show |
On Sat, Feb 6, 2016 at 10:20 PM, Thomas Koenig <tkoenig@netcologne.de> wrote: > Hi Andre, > >> In preventing memory clutter I like to advise the use of: >> >> char u_name[GFC_MAX_SYMBOL_LEN + 1]; >> >> and safe us all the dynamic memory allocation/free. > > > We're really talking micro-optimizations here, but well ... ;-) /me joins the micro-optimization bikeshed! > In the attached patch, I have replaced this with alloca. I was going > to use a VLA originally, but apparently C++ doesn't like that, at least > not in the version that we use within C++. IIRC VLA's were proposed for C++14, but were ultimately rejected. The usual arguments against VLA's (and alloca, which is just another way to spell the same thing, really) are that - It reserves an extra register for the frame pointer. - Functions using VLA's aren't inlined by GCC - From a correctness perspective, unless you check the size somehow beforehand, you can easily get a stack overflow crash. So one solution to these correctness and performance issues is to have a reasonably small statically sized buffer on the stack, and if the size is bigger than that, fall back to heap allocation. E.g. something like void foo(..., size_t n) { char tmp[BUFSIZE]; char *buf; if (n <= BUFSIZE) buf = tmp; else buf = xmalloc(n); // Use buf if (n > BUFSIZE) free(buf); } This is roughly what std::string does with the new C++11 compatible libstdc++ ABI. As we're supposed to be C++ nowadays, why aren't we using that?
Hi Janne, [std::string] > As we're supposed to be C++ nowadays, why aren't we > using that? My reason is simple: I know C and Fortran. I hardly know C++, and I don't want to learn it for the sake of some largely irrelevant micro-optimizations. We should restrict C++ usage to those cases where there is a clear and large benefit. Regards Thomas
Hi all, it wasn't my intention to start such a big discussion for such a small thing. Please stop this academic battle and get back to the issue at hand: Thomas is examining a symbol, that - has come through the scanner, i.e., it adheres to the rules of gfortran, especially it is known to be smaller than GFC_MAX_SYMBOL_LEN - is still a valid symbol. When gfortran's internal composition/modification of symbols breaks this rule, we are done earlier already. - its length is not changed in his routine. I was only trying to prevent an alloc/free for this small use case, which should fit on the stack quite easily. The proposed alloca() call has according to the documentation of libc some availability issues on certain platforms, too. Therefore why not going with the fixed size stack array and adding a check for possible overflow to it and be done? Regards, Andre On Fri, 12 Feb 2016 09:02:27 +0100 Thomas Koenig <tkoenig@netcologne.de> wrote: > Hi Janne, > > [std::string] > > > As we're supposed to be C++ nowadays, why aren't we > > using that? > > My reason is simple: I know C and Fortran. I hardly know C++, > and I don't want to learn it for the sake of some largely > irrelevant micro-optimizations. > > We should restrict C++ usage to those cases where there is > a clear and large benefit. > > Regards > > Thomas
On Fri, Feb 12, 2016 at 12:16 PM, Andre Vehreschild <vehre@gmx.de> wrote: > Hi all, > > it wasn't my intention to start such a big discussion for such a small > thing. Please stop this academic battle and get back to the issue at > hand: > > Thomas is examining a symbol, that > > - has come through the scanner, i.e., it adheres to the rules of > gfortran, especially it is known to be smaller than GFC_MAX_SYMBOL_LEN > > - is still a valid symbol. When gfortran's internal > composition/modification of symbols breaks this rule, we are done > earlier already. > > - its length is not changed in his routine. > > I was only trying to prevent an alloc/free for this small use case, > which should fit on the stack quite easily. The proposed alloca() call > has according to the documentation of libc some availability issues on > certain platforms, too. Therefore why not going with the fixed size > stack array and adding a check for possible overflow to it and be done? Yes, I agree. That sounds like the best approach in this case.
Index: decl.c =================================================================== --- decl.c (Revision 232864) +++ decl.c (Arbeitskopie) @@ -1215,10 +1215,30 @@ build_sym (const char *name, gfc_charlen *cl, bool { symbol_attribute attr; gfc_symbol *sym; + int nlen; + char *u_name; + gfc_symtree *st; if (gfc_get_symbol (name, NULL, &sym)) return false; + /* Check if the name has already been defined as a type. The + first letter of the symtree will be in upper case then. */ + + nlen = strlen(name); + u_name = (char *) alloca (nlen + 1); + strncpy (u_name, name, nlen + 1); + u_name[0] = TOUPPER(u_name[0]); + + st = gfc_find_symtree (gfc_current_ns->sym_root, u_name); + + if (st != 0) + { + gfc_error ("Symbol %qs at %C also declared as a type at %L", name, + &st->n.sym->declared_at); + return false; + } + /* Start updating the symbol table. Add basic type attribute if present. */ if (current_ts.type != BT_UNKNOWN && (sym->attr.implicit_type == 0