This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, Fortran] pad char to int conversions with spaces instead of zeros (legacy)

On 04/12/2018 15:11, Jakub Jelinek wrote:
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
Trying, 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?
Fritz Reese agrees with you here and suggests -ftransfer-pad-char= with -fdec setting it to 0x20.

--- 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
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.
Clearly insufficient testing let this through. The padding should be done for both literals and variables.
So, what does Oracle fortran or ifort do for this b case above?
Don't have access to either of those compilers. It may be possible to check the ifort compiler.

--- /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 <>
+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 (('4142434520202020',kind=8)).and. &
+      ('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?
I hadn't considered those.

Wouldn't it be better to compare transfer("ABCE", 1_8) with transfer("ABCE    ", 1_8)
That simplifies things.


Thanks for the feedback. I inherited this patch, my addition of -fdec-pad-with-spaces and improved testing were insufficient.

Will resubmit the patch after taking these issues into account.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]