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

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Jakub Jelinek
On Wed, Dec 05, 2018 at 06:27:00PM -0800, Jerry DeLisle wrote:
> I disagree completely. I assume the idea of -fdec-pad-with-spaces is to
> accomodate some old dec fortran code. The only reason to use some other
> character is if someone is writing new dec fortran code, which is implying
> encouraging people to be writing non standard conforming code.
>
> Even if it is conforming in the sense that it is processor dependent you are
> encouraging people to create new non portable code across compilers. Please
> just stay consistent with Intel.

So do you prefer to always use ' ' instead of '\0', or decide based on -fdec
without a separate option controlling that?

        Jakub
Reply | Threaded
Open this post in threaded view
|

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

jerry DeLisle-3
On 12/6/18 2:33 AM, Jakub Jelinek wrote:

> On Wed, Dec 05, 2018 at 06:27:00PM -0800, Jerry DeLisle wrote:
>> I disagree completely. I assume the idea of -fdec-pad-with-spaces is to
>> accomodate some old dec fortran code. The only reason to use some other
>> character is if someone is writing new dec fortran code, which is implying
>> encouraging people to be writing non standard conforming code.
>>
>> Even if it is conforming in the sense that it is processor dependent you are
>> encouraging people to create new non portable code across compilers. Please
>> just stay consistent with Intel.
>
> So do you prefer to always use ' ' instead of '\0', or decide based on -fdec
> without a separate option controlling that?
>

Keep current default of '\0' and use ' ' when -fdec given.

Regards,

Jerry
Reply | Threaded
Open this post in threaded view
|

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

jerry DeLisle-3
In reply to this post by Jakub Jelinek
On 12/6/18 2:54 AM, Mark Eggleston wrote:

> On 06/12/2018 10:33, Jakub Jelinek wrote:
>> On Wed, Dec 05, 2018 at 06:27:00PM -0800, Jerry DeLisle wrote:
>>> I disagree completely. I assume the idea of -fdec-pad-with-spaces is to
>>> accomodate some old dec fortran code. The only reason to use some other
>>> character is if someone is writing new dec fortran code, which is
>>> implying
>>> encouraging people to be writing non standard conforming code.
> I should have made this clear.
>>>
>>> Even if it is conforming in the sense that it is processor dependent
>>> you are
>>> encouraging people to create new non portable code across compilers.
>>> Please
>>> just stay consistent with Intel.
> I agree.
>> So do you prefer to always use ' ' instead of '\0', or decide based on
>> -fdec
>> without a separate option controlling that?
> It would be simpler to dispense with the extra option and just set space
> padding with -fdec. I think it would be odd to use something other than
> a space or '\0' for padding.
>>
>>     Jakub
> Mark
>

Agree with Mark on this.

Jerry
Reply | Threaded
Open this post in threaded view
|

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

Mark Eggleston
In reply to this post by Jakub Jelinek

On 06/12/2018 10:20, Mark Eggleston wrote:
>> Yes. Mark, you'll need to also patch iresolve.c (gfc_resolve_transfer)
>> to affect non-constant resolution.
> Thanks for the hint.

I've looked at gfc_resolve_transfer regarding handling of padding when a
character variable is passed to transfer instead of a literal. This
routine is not called so can't be where a variable would be handled.

Don't yet know where to make the change.

regards,

Mark

--
https://www.codethink.co.uk/privacy.html

Reply | Threaded
Open this post in threaded view
|

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

Fritz Reese
> On Mon, Dec 10, 2018 at 02:09:50PM +0000, Mark Eggleston wrote:
> >
> > On 06/12/2018 10:20, Mark Eggleston wrote:
> > > > Yes. Mark, you'll need to also patch iresolve.c (gfc_resolve_transfer)
> > > > to affect non-constant resolution.
> > > Thanks for the hint.
> >
> > I've looked at gfc_resolve_transfer regarding handling of padding when a
> > character variable is passed to transfer instead of a literal. This routine
> > is not called so can't be where a variable would be handled.
> >
> > Don't yet know where to make the change.

It actually is called through a function pointer
(gfc_intrinsic_sym->resolve) in intrinsic.c (resolve_intrinsic), as
with all intrinsics. You can find the backtrace by running f951
(`gfortran -print-prog-name=f951`) through gdb and setting a
breakpoint on gfc_resolve_transfer. That being said...

On Mon, Dec 10, 2018 at 12:12 PM Jakub Jelinek <[hidden email]> wrote:
> I think you want to change gfc_conv_intrinsic_transfer

... in this case Jakub is right. I hadn't looked at the body of the
resolve function yet, but peeking at it tells me the transfer
expression doesn't contain support for padding information, so you'd
have to deal with it yourself in translation.

> I guess you want to add after this, guarded with flag_dec only,
> code to compare (at runtime) if extent < dest_word_len and if so,
> use fill_with_spaces to pad it with spaces at the end (from
> that first memcpy's argument + extent dest_word_len - extent bytes),
> with a comment why you are doing it.  Guess the array case will need
> something similar.

Alternatively you could call BUILT_IN_MEMSET(&tmpdecl, '\x20',
dest_word_len) just prior to the MEMCPY whenever flag_dec is set.

Fritz
Reply | Threaded
Open this post in threaded view
|

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

Jakub Jelinek
In reply to this post by Mark Eggleston
On Wed, Dec 12, 2018 at 11:37:23AM +0000, Mark Eggleston wrote:

> Before delving into the code to make changes to handle the case when passing
> a variable into transfer instead of a literal I revised the the test cases.
> The results indicate to me that this patch as originally intended is
> erroneous.
>
> When a character literal is assigned to character variable e.g.
>
> character(len=8) :: a
> a = 'ab'
>
> The value is padded with spaces.

Sure, that is the standard behavior of character assignments.

What we are talking about is the case when transfer is used from a smaller
object to a larger result, whether it is e.g. 'ab' literal or character(len=2)
variable.  SOURCE doesn't have to be a character scalar (or array), it can
be anything.

The Fortran standard says here:
"If the physical representation of the result is longer than that
of SOURCE, the physical representation of the leading part is that of SOURCE and the remainder is processor
dependent."
Current gfortran choice of the processor dependent is 0 if SOURCE is a
constant and uninitialized bytes if it is not a constant.

So, does Oracle fortran / ifort etc. pad with spaces only if SOURCE has
character type, or in other cases too?

What about:
  integer(kind=2) :: a
  a = -1
  print *, transfer (1_2, 1_8), transfer (a, 1_8)
end
?

        Jakub
Reply | Threaded
Open this post in threaded view
|

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

Jakub Jelinek
On Wed, Dec 12, 2018 at 12:06:12PM +0000, Mark Eggleston wrote:

>
> On 12/12/2018 11:52, Jakub Jelinek wrote:
> > What about:
> >    integer(kind=2) :: a
> >    a = -1
> >    print *, transfer (1_2, 1_8), transfer (a, 1_8)
> > end
> > ?
>
> I assume you meant transfer (-1_2, 1_8), the result from gfortran is 65535
> for both transfers.

It doesn't really matter that much, the question is what is in the upper
bits and mainly a) what you get with the vendor compilers b) what you get
with your patch.  Because by my reading if you use there 0x20, it would
print 2314885530818510847 or 2314885530818445313 or something similar.

>
> I'm about to build the compiler with
>
>   memset (buffer, 0x20, buffer_size);
>
> instead of
>
>   memset (buffer, (flag_dec_pad_with_spaces ? 0x20 : 0x0), buffer_size);
>
> and will check again, if necessary a padding variable can be used instead
> initially set to zero and changed to 0x20 when it is known that the source
> is character.

        Jakub