Hi Janne,
So, any other comments about my patch? OK for trunk?- In many cases the copyright notice has "This file is part of the GNU Fortran 95 runtime library (libgfortran)." It's a while since we've called ourselves "GNU Fortran 95", so just remove the "95".
That's what I got for copying over an old version of maxloc (when it still didn't have NAN handling) as a basis for my own patch. This also meant that I had an old copyright notice. Fixed.
- It seems in the library you're using int for string lengths? Please use gfc_charlen_type instead (in the frontend, gfc_charlen_type_node). (Most of the charlen->size_t patch is fixing up places where we're accidentally using int instead of gfc_charlen_type..).
Fixed.
- Why are you using GFC_INTEGER_1 / GFC_INTEGER_4 to loop over the arrays rather than char/gfc_char4_t? Not sure if it makes any difference in practice, but it sure seems confusing..
The reason has to do with evil m4 magic. I used a macro from iparm.m4, atype_code. Changing m4 code should mostly be restricted to those cases where it is _really_ necessary (people, say, not without justification, that m4 is a write-only langauge).
- Not really related to your patch, but memcmp_char4 sure looks redundant. Isn't it the same as memcmp(a, b, size*4), in which case we could use optimized memcmp implementations?
Big/little endian issues.
Consider the following short program:
#include <stdio.h>
#include <string.h>
char a[4] = { 1, 2, 3, 4};
char b[4] = { 4, 3, 2, 1};
int main()
{
int i, j;
memcpy (&i, a, sizeof(i));
memcpy (&j, b, sizeof(j));
printf("memcmp : ");
if (memcmp (&i,&j,sizeof(i)))
printf("larger\n");
else
printf("smaller\n");
printf("Direct comparison: ");
if (i > j)
printf("larger\n");
else
printf("smaller\n");
return 0;
}
On my x86_64 system, this prints
memcmp : larger
Direct comparison: larger
On a big-endian system, this prints
memcmp : larger
Direct comparison: smaller
However, I just learned about the __BYTE_ORDER__ macro.
We could use that (and in places where we currently have
a runtime check, for example in replacing the big_endian
global variable in libgfortran). But that is for another day.
So, attached is a new version of the patch. No update
on the ChangeLog. OK for trunk?
Regards
Thomas
p8.diff.gz
Description: application/gzip
