On Tue, Dec 04, 2018 at 02:47:25PM +0000, Mark Eggleston wrote:
> Here is a patch to considered for incorporation into gfortran adding to its
> legacy support. It pads character to integer conversions using spaces
> instead of zeros when enabled.
> 
> The pad character is 'undefined' or 'processor dependent' depending on which
> standard you read. This makes it 0x20 which matches the Oracle Fortran
> compiler.

Trying fortran.godbolt.org, I think ifort pads this with spaces too.

> Enabled using -fdec-pad-with-spaces and -fdec.

Just a couple of random comments.
-fdec-pad-with-spaces option name doesn't look right, because it doesn't say
what the option affects.  So perhaps have transfer in the option name?
Wouldn't it be better to allow specifying whatever character you want to pad
with, so that users can choose something even different?

> --- a/gcc/fortran/simplify.c
> +++ b/gcc/fortran/simplify.c
> @@ -7830,7 +7830,7 @@ gfc_simplify_transfer (gfc_expr *source, gfc_expr 
> *mold, gfc_expr *size)
>    /* Allocate the buffer to store the binary version of the source.  */
>    buffer_size = MAX (source_size, result_size);
>    buffer = (unsigned char*)alloca (buffer_size);
> -  memset (buffer, 0, buffer_size);
> +  memset (buffer, (flag_dec_pad_with_spaces ? 0x20 : 0x0), buffer_size);

This affects just the simplification when the argument is a known constant.
Shouldn't we handle it also when it is not a constant?  Like I've tried:
program test
  integer(kind=8) :: a, b, c
  character(len=4) :: e
  a = transfer("ABCE", 1_8)
  e = "ABCE"
  b = transfer(e, 1_8)
  c = transfer("ABCE    ", 1_8)
  print *, a, b, c
end
and for a the result is on little-endian indeed z'45434241', for b
the upper 32 bits are completely random:
            D.3854 = 4;
            D.3856 = 8;
            _1 = MIN_EXPR <D.3856, D.3854>;
            _2 = MAX_EXPR <_1, 0>;
            _3 = (unsigned long) _2;
            __builtin_memcpy (&transfer.0, &e, _3);
            transfer.3_4 = transfer.0;
            b = transfer.3_4;
and for c it is the padding with zeros I assume you want for -fdec.
So, what does Oracle fortran or ifort do for this b case above?

> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/dec-pad-with-spaces-1.f90
> @@ -0,0 +1,17 @@
> +! { dg-do run }
> +! { dg-options "-fdec-pad-with-spaces" }
> +!
> +! Test case contributed by Mark Eggleston <mark.eggles...@codethink.com>
> +
> +program test
> +  integer(kind=8) :: a
> +  a = transfer("ABCE", 1_8)
> +  ! If a has not been converted into big endian
> +  ! or little endian integer it has failed.
> +  if ((a.ne.int(z'4142434520202020',kind=8)).and. &
> +      (a.ne.int(z'2020202045434241',kind=8))) then 

The tests look too much big-endian vs. little-endian dependent and
ascii dependent.  We have pdp-endian and doesn't s390x TPF use EBCDIC?

Wouldn't it be better to compare transfer("ABCE", 1_8) with transfer("ABCE    
", 1_8)
?

        Jakub

Reply via email to