Re: [patch, fortran] Implement maxloc and minloc for character

[Thomas Koenig]

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




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

memcmp : larger

p8.diff.gz
Description: application/gzip