Re: [Patch] Fortran: Fixes for kind=4 characters strings [PR107266]

[Harald Anlauf <anlauf@gmx.de>]

Hi Tobias,

the patch LGTM.

Regarding testcase char4_decl-2.f90, I played a little and found that
one could in addition check the storage_size of aa, pp in the main and
compare with storage_size (4_'foo') etc.  Without your patch the
storage sizes look odd.  (Strictly speaking, a comparison like
   if (aa .ne. 4_'foo') stop 123
is not fully sufficient to catch such oddities.)

Thanks,
Harald


Am 14.10.22 um 23:18 schrieb Tobias Burnus:
> Long introduction - but the patch is rather simple: Don't use kind=1
> as type where kind=4 should be used.
>
> Looooong introduction + background, feel free to skip.
>
> ---------------<intro/background>-------------
>
> This popped up for libgomp/testsuite/libgomp.fortran/struct-elem-map-1.f90
> which uses kind=4 characters – if Sandra's "Fortran: delinearize
> multi-dimensional
> array accesses" patch is applied.
>
> Patch: https://gcc.gnu.org/pipermail/gcc-patches/2020-December/562230.html
> Used for OG11:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584716.html
> On the OG12 alias devel/omp/gcc-12 vendor branch, it is used:
> https://gcc.gnu.org/g:39a8c371fda6136cf77c74895a00b136409e0ba3
>
> * * *
>
> For mainline, I did not observe a wrong-code issue at runtime, still:
>
> void frobc (character(kind=4)[1:*_a] * & restrict a, ...
> ...
> static void frobc (character(kind=1) * & restrict, ...
>
> feels odd, i.e. having the definition as kind=4 and the declaration as
> kind=1.
> With the patch, it becomes:
>
> static void frobc (character(kind=4) * & restrict, character(kind=4) *
> &, ...
>
>   * * *
>
> For the following, questionable code (→ PR107266), it is even worse:
>
> character(kind=4) function f(x) bind(C)
>    character(kind=4), value :: x
> end
>
> this gives the following, which has the wrong ABI:
>
> character(kind=1) f (character(kind=1) x)
> {
>    (void) 0;
> }
>
> With the patch, it becomes:
>    character(kind=4) f (character(kind=4) x)
>
>   * * *
>
> I think that all only exercises the trans-type.cc patch;
> the trans-expr.cc code gets called – as an assert shows,
> but I fail to get a dump where this goes wrong.
>
> However, for struct-elem-map-1.f90 with mainline or with
> OG12 and the patch:
>    #pragma omp target map(tofrom:var.uni2[40 / 20] [len: 20])
>
> while on OG12 without the attached patch:
>    #pragma omp target map(tofrom:var.uni2[40 / 5] [len: 5])
>
> where the problem is that TYPE_SIZE_UNIT is wrong. Whether
> this only affects OG12 due to the delinearizer patch or
> some code on mainline as well, I don't know.
>
> Still, I think it should be fixed ...
>
> ---------------<END of intro/background>-------------
>
> OK for mainline?
>
> Tobias
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201,
> 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer:
> Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München;
> Registergericht München, HRB 106955