Re: libgfortran.so SONAME and powerpc64le-linux ABI changes

[Segher Boessenkool]

On Thu, Oct 14, 2021 at 02:39:47PM -0500, Bill Schmidt wrote:
> On 10/5/21 12:43 PM, Segher Boessenkool wrote:
> > The last release (version 1.9) was in 2004.  If there is interest in
> > making updates to it that coulde be done of course, it is GFDL, there is
> > no red tape getting in the way.
> >
> > Maybe this could be maintained in the same repository even?
> 
> Well, I'm not sure it's quite this easy; when developing ELFv2, there was 
> enough
> doubt about the provenance/ownership of ELFv1 that we weren't comfortable 
> borrowing
> language from it.  That may have been an excess of caution, or it may not...

I am not suggesting you should share this text with some other ABI.
Just that you can put it in the same repository :-)

> That said, with enough diligence I would hope we would be able to create
> modifications to the ELFv1 document, but we might incur some paperwork.

It is GFDL version 1.1 .  The hardest part should be to find that older
licence version :-P
( fwiw)

(But I understand your situation, heh.  Let the lawyers worry about it?
That is their job :-) )


Segher

Re: libgfortran.so SONAME and powerpc64le-linux ABI changes

[Segher Boessenkool]

Hi!

On Mon, Oct 11, 2021 at 08:11:50PM +, Joseph Myers wrote:
> On Fri, 8 Oct 2021, Segher Boessenkool wrote:
> > But many CPUs do not have hardware floating point in any variant, and
> > their ABIs / calling conventions do not mention floating point at all.
> > Still, this works with GCC just fine: it passes floats and doubles the
> > same as 32-bit resp. 64-bit integers.
> > 
> > binary16 and bfloat16 would be easy to support the same way, but it is a
> > bit harder for binary128, because we do not have a 128-bit integer type
> 
> Supporting passing arguments (and return values) the same as an integer 
> type of the same size is a *choice* (which comes with other choices - in 
> particular, whether to say some or all the higher bits in the register or 
> stack slot are sign-extended, zero-extended or undefined).  It's a choice 
> that should be made explicitly, and documented (in the relevant ABI if one 
> is maintained), and coordinated between implementations when there's more 
> than one implementation for the architecture trying to be compatible.  

I don't disagree at all.  But: GCC makes that choice for you, if you do
not.  Many (embedded and/or older) targets do not.  They get the
defaults, those just work, and /de facto/ become the standard.

In practice most such architectures are purely 32-bit, so there is no
sign/zero extension problem.

> We've had plenty of problems in the past with ABIs that were just what 
> happened to fall out of the implementation (e.g. ABIs that depended on the 
> details of what machine mode was assigned to a structure type...).

Yes.

And we still have problems on older ABIs with e.g. new C++ requirements
that did not exist >25 years ago when the ABIs were written.  Not all of
this can be helped at all.

> On a related note, I'd encourage architecture maintainers to start 
> thinking now about what exactly their ABIs should be for _BitInt 
> (, accepted as 
> a required feature for C23 up to at least the width of unsigned long 
> long), and documenting it and coordinating with other implementations 
> where appropriate.  There's a concrete proposal for x86_64 (see 
> origin/usr/hjl/bitint branch at 
> https://gitlab.com/x86-psABIs/x86-64-ABI.git) that may at least help as an 
> indication of the sort of issues to address in such an ABI.

This should really go on gcc@, in a thread of its own, and a wiki page
might help as well?


Segher

[PATCH] PR fortran/102685 - ICE in output_constructor_regular_field, at varasm.c:5514

[Harald Anlauf via Fortran]

Dear Fortranners,

the attached patch adds a check for the shape of arrays in derived type
constructors.  This brings it in line with other major brands.

Example:

  type t
 integer :: a(1)
  end type
  type(t), parameter :: y(2) = t([1,2])
end

This was silently accepted before, but now gives:

pr102685-z1.f90:4:33:

4 |   type(t), parameter :: y(2) = t([1,2])
  | 1
Error: The shape of component 'a' in the structure constructor at (1) differs 
from the shape of the declared component for dimension 1 (2/1)

During regtesting several previously invalid testcases surfaced
which would be rejected by the present change.  They have been
adjusted along the discussion with Tobias and Paul, see the thread

https://gcc.gnu.org/pipermail/fortran/2021-October/056707.html

In developing the patch I encountered a difficulty with testcase
dec_structure_6.f90, which uses a DEC extension, namelist "old-style
CLIST initializers in STRUCTURE".  I could not figure out how to
determine the shape of the initializer; it seemed to be always zero.
I've added code to accept this, but only under -fdec-structure, and
added a TODO in a comment.  If somebody reading this could give me
a hint to solve end, I would adjust the patch accordingly.

Regtested on x86_64-pc-linux-gnu.  OK?  Or further comments?

Thanks,
Harald

Fortran: validate shape of arrays in constructors against declarations

gcc/fortran/ChangeLog:

	PR fortran/102685
	* resolve.c (resolve_structure_cons): In a structure constructor,
	compare shapes of array components against declared shape.

gcc/testsuite/ChangeLog:

	PR fortran/102685
	* gfortran.dg/derived_constructor_char_1.f90: Fix invalid code.
	* gfortran.dg/pr70931.f90: Likewise.
	* gfortran.dg/transfer_simplify_2.f90: Likewise.
	* gfortran.dg/pr102685.f90: New test.

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 0d0af39d23f..d035b30ad7d 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "data.h"
 #include "target-memory.h" /* for gfc_simplify_transfer */
 #include "constructor.h"
+#include "parse.h"

 /* Types used in equivalence statements.  */

@@ -1454,6 +1455,42 @@ resolve_structure_cons (gfc_expr *expr, int init)
 	}
 	}

+  /* Validate shape.  We silently accept some cases where the apparent
+	 shape of the constructor is zero, and we cannot check dynamic or PDT
+	 arrays.  */
+  if (cons->expr->expr_type == EXPR_ARRAY && rank == cons->expr->rank
+	  && comp->as && !comp->attr.allocatable && !comp->attr.pointer
+	  && !comp->attr.pdt_array)
+	{
+	  mpz_t len;
+	  mpz_init (len);
+	  for (int n = 0; n < rank; n++)
+	{
+	  gcc_assert (comp->as->upper[n]->expr_type == EXPR_CONSTANT
+			  && comp->as->lower[n]->expr_type == EXPR_CONSTANT);
+	  mpz_set_ui (len, 1);
+	  mpz_add (len, len, comp->as->upper[n]->value.integer);
+	  mpz_sub (len, len, comp->as->lower[n]->value.integer);
+	  /* Shape agrees for this dimension.  */
+	  if (mpz_cmp (cons->expr->shape[n], len) == 0)
+		continue;
+	  /* Accept DEC old-style initializers in STRUCTURE, where shape
+		 is currently not correctly set (it is zero.  Why?).
+		 TODO: Fix this or find a better solution.  */
+	  if (flag_dec_structure
+		  && mpz_cmp_si (cons->expr->shape[n], 0) == 0)
+		continue;
+	  gfc_error ("The shape of component %qs in the structure "
+			 "constructor at %L differs from the shape of the "
+			 "declared component for dimension %d (%ld/%ld)",
+			 comp->name, >expr->where, n+1,
+			 mpz_get_si (cons->expr->shape[n]),
+			 mpz_get_si (len));
+	  t = false;
+	}
+	  mpz_clear (len);
+	}
+
   if (!comp->attr.pointer || comp->attr.proc_pointer
 	  || cons->expr->expr_type == EXPR_NULL)
 	continue;
diff --git a/gcc/testsuite/gfortran.dg/derived_constructor_char_1.f90 b/gcc/testsuite/gfortran.dg/derived_constructor_char_1.f90
index 892a9c9f4c1..91fc4c902d8 100644
--- a/gcc/testsuite/gfortran.dg/derived_constructor_char_1.f90
+++ b/gcc/testsuite/gfortran.dg/derived_constructor_char_1.f90
@@ -5,7 +5,7 @@
 !
 !
   Type :: t5
-character (len=5) :: txt(4)
+character (len=5) :: txt(2)
   End Type t5

   character (len=3), parameter :: str3(2) = [ "ABC", "ZYX" ]
diff --git a/gcc/testsuite/gfortran.dg/pr102685.f90 b/gcc/testsuite/gfortran.dg/pr102685.f90
new file mode 100644
index 000..d325c27b32a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr102685.f90
@@ -0,0 +1,30 @@
+! { dg-do compile }
+! PR fortran/102685
+
+program p
+  type t
+ integer :: a(2)
+  end type
+  type(t), parameter :: x0= t([2]) ! { dg-error "shape of component" }
+  type(t), parameter :: x1(2) = t([2]) ! { dg-error "shape of component" }
+  type(t), parameter :: x(2)  = t([integer::]) ! { dg-error "shape of component" }
+
+  type u
+ integer :: a
+ integer :: b(0)
+  end type
+  type(u), parameter :: z0(2) = u(1, 

Re: libgfortran.so SONAME and powerpc64le-linux ABI changes

[Bill Schmidt via Fortran]

On 10/5/21 12:43 PM, Segher Boessenkool wrote:
> Hi Joseph,
>
> On Mon, Oct 04, 2021 at 07:24:31PM +, Joseph Myers wrote:
>> On Mon, 4 Oct 2021, Segher Boessenkool wrote:
>>> Some current Power GCC targets support neither.  Some support only
>>> double-double.  Making IEEE QP float work on those (that is, those that
>>> are < power8) will require some work still: it should use libquadmath or
>>> similar, but that needs to be put into the ABIs, to define how parameter
>>> passing works for those types.  Just treating it like a struct or an
>>> array of ints will work fine, but it needs to be written down.  This is
>>> more than just Fortran.
>> Is the 64-bit BE (ELFv1) ABI maintained somewhere?  (The 32-bit ABI, 
>> covering both hard float and soft float, is 
>>  - no activity lately, but I think 
>> Ryan said he'd given write access to someone still involved with Power.)

Just FYI, that person is me.  I've never tried to use my powers, either for good
or evil, since no proposals for updates have arisen since Ryan left; but my
credentials should still work.

> The last release (version 1.9) was in 2004.  If there is interest in
> making updates to it that coulde be done of course, it is GFDL, there is
> no red tape getting in the way.
>
> Maybe this could be maintained in the same repository even?

Well, I'm not sure it's quite this easy; when developing ELFv2, there was enough
doubt about the provenance/ownership of ELFv1 that we weren't comfortable 
borrowing
language from it.  That may have been an excess of caution, or it may not...

That said, with enough diligence I would hope we would be able to create
modifications to the ELFv1 document, but we might incur some paperwork.

Bill

>
>
> Segher

Re: [RFC] Fixing PR102685 and testsuite issues

[Harald Anlauf via Fortran]

Dear Tobias, Paul, all,

thanks for the feedback!

> Gesendet: Donnerstag, 14. Oktober 2021 um 11:26 Uhr
> Von: "Tobias Burnus" 

> > (1) gfortran.dg/derived_constructor_char_1.f90
> > The constructor is shorter than the array component txt in DT t5.
> > Commit r0-101989.
> > @Tobias: can you comment?
> Simply change 'txt(4)' to 'txt(2)'.

Will do.

> > (2) gfortran.dg/pr70931.f90
> > Committed by Richard Biener, fixing an ICE on an invalid testcase by 
> > Gerhard.
> > I think we safely can mark this one as invalid and emit an error, right?
> 
> No - we still want to check the bug fix in gcc/dwarf2out.c's 
> native_encode_initializer,
> i.e.
> - if (val
> + if (val && fieldsize != 0
> 
> I think you should just change:
> +   type(t), parameter :: z = t(1, [2])
> to ... = t(1, [])

Will change to ... = t(1, [integer::]), as [] has no valid type.

> > (3) gfortran.dg/transfer_simplify_2.f90

> I think you can simply remove the ',3.0, 4.0' in the 'dt1' line.

Will do.

> > Along these lines, I think we should always reject code having too many 
> > elements
> > in a constructor.  Is there agreement on this?  This would need fixing case 
> > (3).
> >
> > Is there a reason to accept cases where the constructor is shorter than the 
> > array
> > component in the DT?  Some extension?  Shall we give a warning for these 
> > cases
> > instead of an error?  Based on -std flags?
> 
> At the moment I do not see any good reason for having
> too many or too few elements. Also the testsuite use
> indicates rather bugs and not intentional use.

Good.

> Thus, I would simply give an error in either case.
> I think that also matches the behavior of other
> compilers such as ifort but I have not tested it.
> (I think you alluded that you did test it with ifort
> and it gave errors for both too long and too short
> array constructors.)

Testcase with e.g. too few elements:

program p
  type t
 integer :: a(2)
  end type
  type(t), parameter :: x(2) = t([2])
end

Intel:

pr102685.f90(5): error #6595: The shape of a component in a structure 
constructor differs from the shape of the component of the derived type.   [T]
  type(t), parameter :: x(2) = t([2])
---^
compilation aborted for pr102685.f90 (code 1)

NAG:

NAG Fortran Compiler Release 7.0(Yurakucho) Build 7009
Warning: pr102685.f90, line 5: Unused PARAMETER X
Error: pr102685.f90, line 5: Dimension 1 of value for array A has extent 1 
instead of 2
Errors in declarations, no further processing for P

NVIDIA:

NVFORTRAN-S-0066-Too few data constants in initialization statement 
(pr102685.f90: 5)
NVFORTRAN-S-0066-Too few data constants in initialization statement 
(pr102685.f90: 5)
NVFORTRAN-S-0066-Too few data constants in initialization statement 
(pr102685.f90: 5)
NVFORTRAN-S-0066-Too few data constants in initialization statement 
(pr102685.f90: 5)
  0 inform,   0 warnings,   4 severes, 0 fatal for p

Now that one is really convincing!

> I would change the testcases as mentioned, but maybe
> it makes sense to add the invalid use in the new
> testcase as some might exercise some corner cases
> (size = 0, ...)?

Will do.

> As you had "derived_constructor_char_1.f90" in the list:
> 
> Having ["abc", "a", "ab cdefg"] is also invalid – as either the
> strings have to be changed (space-padded on the right) to have
> the same length or a typespec as in
>[ character(len=) :: "", ".." ]
> has to be used. But I can see that it easily can happen that the
> strings are of different length and the user then expects that
> the compiler handles it correctly™, especially before the
> typespec was permitted. (GCC attempts to do this, including
> warnings/errors in that case.)

There's at least one related nasty PR on my list...

> > Any constructive comment appreciated!
> 
> I wonder why you are only interested in constructive ones ... ;-)

... that I got distracted from by the almost constant inflow of
PRs with alleged regressions...

Thanks,
Harald

> Thanks,
> 
> 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
>

Re: [PATCH] openmp, fortran: Add support for declare variant in Fortran

[Jakub Jelinek via Fortran]

On Thu, Oct 14, 2021 at 11:04:59AM +0100, Kwok Cheung Yeung wrote:
> I have now dropped this. This affects test2 in
> gfortran.dg/gomp/declare-variant-8.f90, which I have added a comment to.

Thanks.

> I have added Fortran-specific tests as
> gfortran.dg/gomp/declare-variant-15.f90 to declare-variant-19.f90.

What I still miss is tests for the (proc_name : variant_name) syntax
in places where proc_name : is optional, but is supplied and is valid, like
e.g. in interface, or in subroutine/function and where proc_name specifies
the name of the containing interface or subroutine/function.
I see that syntax tested in some places with dg-error on that line and
in spaces where it isn't optional (e.g. at module scope before contains).
But if you want, that can be added incrementally.

> From ab03cf08c6ee4a0a6323189313cae911483a2257 Mon Sep 17 00:00:00 2001
> From: Kwok Cheung Yeung 
> Date: Wed, 13 Oct 2021 22:39:20 +0100
> Subject: [PATCH] openmp, fortran: Add support for OpenMP declare variant
>  directive in Fortran
> 
> 2021-10-13  Kwok Cheung Yeung  
> 
> gcc/c-family/
> 
>   * c-omp.c (c_omp_check_context_selector): Rename to
>   omp_check_context_selector and move to omp-general.c.
>   (c_omp_mark_declare_variant): Rename to omp_mark_declare_variant and
>   move to omp-general.c.
> 
> gcc/c/
> 
>   * c-parser.c (c_finish_omp_declare_variant): Change call from
>   c_omp_check_context_selector to omp_check_context_selector. Change
>   call from c_omp_mark_declare_variant to omp_mark_declare_variant.
> 
> gcc/cp/
> 
>   * decl.c (omp_declare_variant_finalize_one): Change call from
>   c_omp_mark_declare_variant to omp_mark_declare_variant.
>   * parser.c (cp_finish_omp_declare_variant): Change call from
>   c_omp_check_context_selector to omp_check_context_selector.
> 
> gcc/fortran/
> 
>   * gfortran.h (enum gfc_statement): Add ST_OMP_DECLARE_VARIANT.
>   (enum gfc_omp_trait_property_kind): New.
>   (struct gfc_omp_trait_property): New.
>   (gfc_get_omp_trait_property): New macro.
>   (struct gfc_omp_selector): New.
>   (gfc_get_omp_selector): New macro.
>   (struct gfc_omp_set_selector): New.
>   (gfc_get_omp_set_selector): New macro.
>   (struct gfc_omp_declare_variant): New.
>   (gfc_get_omp_declare_variant): New macro.
>   (struct gfc_namespace): Add omp_declare_variant field.
>   (gfc_free_omp_declare_variant_list): New prototype.
>   * match.h (gfc_match_omp_declare_variant): New prototype.
>   * openmp.c (gfc_free_omp_trait_property_list): New.
>   (gfc_free_omp_selector_list): New.
>   (gfc_free_omp_set_selector_list): New.
>   (gfc_free_omp_declare_variant_list): New.
>   (gfc_match_omp_clauses): Add extra optional argument.  Handle end of
>   clauses for context selectors.
>   (omp_construct_selectors, omp_device_selectors,
>   omp_implementation_selectors, omp_user_selectors): New.
>   (gfc_match_omp_context_selector): New.
>   (gfc_match_omp_context_selector_specification): New.
>   (gfc_match_omp_declare_variant): New.
>   * parse.c: Include tree-core.h and omp-general.h.
>   (decode_omp_directive): Handle 'declare variant'.
>   (case_omp_decl): Include ST_OMP_DECLARE_VARIANT.
>   (gfc_ascii_statement): Handle ST_OMP_DECLARE_VARIANT.
>   (gfc_parse_file): Initialize omp_requires_mask.
>   * symbol.c (gfc_free_namespace): Call
>   gfc_free_omp_declare_variant_list.
>   * trans-decl.c (gfc_get_extern_function_decl): Call
>   gfc_trans_omp_declare_variant.
>   (gfc_create_function_decl): Call gfc_trans_omp_declare_variant.
>   * trans-openmp.c (gfc_trans_omp_declare_variant): New.
>   * trans-stmt.h (gfc_trans_omp_declare_variant): New prototype.
> 
> gcc/
> 
>   * omp-general.c (omp_check_context_selector):  Move from c-omp.c.
>   (omp_mark_declare_variant): Move from c-omp.c.
>   (omp_context_name_list_prop): Update for Fortran strings.
>   * omp-general.h (omp_check_context_selector): New prototype.
>   (omp_mark_declare_variant): New prototype.
> 
> gcc/testsuite/
> 
>   * gfortran.dg/gomp/declare-variant-1.f90: New test.
>   * gfortran.dg/gomp/declare-variant-10.f90: New test.
>   * gfortran.dg/gomp/declare-variant-11.f90: New test.
>   * gfortran.dg/gomp/declare-variant-12.f90: New test.
>   * gfortran.dg/gomp/declare-variant-13.f90: New test.
>   * gfortran.dg/gomp/declare-variant-14.f90: New test.
>   * gfortran.dg/gomp/declare-variant-15.f90: New test.
>   * gfortran.dg/gomp/declare-variant-16.f90: New test.
>   * gfortran.dg/gomp/declare-variant-17.f90: New test.
>   * gfortran.dg/gomp/declare-variant-18.f90: New test.
>   * gfortran.dg/gomp/declare-variant-19.f90: New test.
>   * gfortran.dg/gomp/declare-variant-2.f90: New test.
>   * gfortran.dg/gomp/declare-variant-2a.f90: New test.
>   * 

Re: [RFC] Fixing PR102685 and testsuite issues

[Paul Richard Thomas via Fortran]

Hi Harald,

The overfilled constructor in transfer_simplify_2.f90 is indeed an error.

The error is picked up correctly for arrays in
expr.c(gfc_check_conformance):3579 but not for array components.

Regards

Paul


On Thu, 14 Oct 2021 at 10:26, Tobias Burnus  wrote:

> Dear all, hello Harald,
>
> On 12.10.21 22:50, Harald Anlauf wrote:
> > while working on a fix for PR102685, I encountered issues with the
> testsuite.
> ...
> > (1) gfortran.dg/derived_constructor_char_1.f90
> > The constructor is shorter than the array component txt in DT t5.
> > Commit r0-101989.
> > @Tobias: can you comment?
> Simply change 'txt(4)' to 'txt(2)'.
> > (2) gfortran.dg/pr70931.f90
> > Committed by Richard Biener, fixing an ICE on an invalid testcase by
> Gerhard.
> > I think we safely can mark this one as invalid and emit an error, right?
>
> No - we still want to check the bug fix in gcc/dwarf2out.c's
> native_encode_initializer,
> i.e.
> - if (val
> + if (val && fieldsize != 0
>
> I think you should just change:
> +   type(t), parameter :: z = t(1, [2])
> to ... = t(1, [])
>
> > (3) gfortran.dg/transfer_simplify_2.f90
> >
> > The constructor has more elements than the array component in the DT and
> is
> > invalid.
> >
> > Commit r0-80854.
>
> We had already before:
> "Fixed invalid initialization" and "Fixed invalid array constructor."
> for that file. Thus, changing it again to fix a bug is fine :-)
>
> That looks like a combined pasto + thinko. In
> subroutine dt_to_integer1
>  real, parameter   :: r1(4) = (/1.0_4,2.0_4,3.0_4,4.0_4/)
>  type :: mytype
>integer(4) :: i(4)
>real(4) :: x(4)
>  end type mytype
>
> and in the next one:
>subroutine character16_to_dt
>  character(16), parameter ::  c1 = "abcdefghijklmnop"
>  character(16)::  c2 = c1
>  type :: mytype
>real(4) :: x(2)
>  end type mytype
>
>  type (mytype), parameter :: dt1(2) = transfer (c1, mytype
> ((/1.0,2.0,3.0,4.0/)), 2)
> ...
>  if (any (dt1(1)%x .ne. dt2(1)%x)) STOP 12
>  if (any (dt1(2)%x .ne. dt2(2)%x)) STOP 13
>end subroutine character16_to_dt
>
> I think you can simply remove the ',3.0, 4.0' in the 'dt1' line.
>
> > Along these lines, I think we should always reject code having too many
> elements
> > in a constructor.  Is there agreement on this?  This would need fixing
> case (3).
> >
> > Is there a reason to accept cases where the constructor is shorter than
> the array
> > component in the DT?  Some extension?  Shall we give a warning for these
> cases
> > instead of an error?  Based on -std flags?
>
> At the moment I do not see any good reason for having
> too many or too few elements. Also the testsuite use
> indicates rather bugs and not intentional use.
>
> Thus, I would simply give an error in either case.
> I think that also matches the behavior of other
> compilers such as ifort but I have not tested it.
> (I think you alluded that you did test it with ifort
> and it gave errors for both too long and too short
> array constructors.)
>
> I would change the testcases as mentioned, but maybe
> it makes sense to add the invalid use in the new
> testcase as some might exercise some corner cases
> (size = 0, ...)?
>
> As you had "derived_constructor_char_1.f90" in the list:
>
> Having ["abc", "a", "ab cdefg"] is also invalid – as either the
> strings have to be changed (space-padded on the right) to have
> the same length or a typespec as in
>[ character(len=) :: "", ".." ]
> has to be used. But I can see that it easily can happen that the
> strings are of different length and the user then expects that
> the compiler handles it correctly™, especially before the
> typespec was permitted. (GCC attempts to do this, including
> warnings/errors in that case.)
>
> But for array elements, it seems to be unlikely that there is
> size mismatch on purpose or by accidentally expecting that
> the compiler corrects this.
>
> > Any constructive comment appreciated!
>
> I wonder why you are only interested in constructive ones ... ;-)
>
> Thanks,
>
> 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
>


-- 
"If you can't explain it simply, you don't understand it well enough" -
Albert Einstein

Re: [PATCH, OpenMP 5.1, Fortran] Strictly-structured block support for OpenMP directives

[Jakub Jelinek via Fortran]

On Thu, Oct 14, 2021 at 12:20:51PM +0200, Jakub Jelinek via Gcc-patches wrote:
> Thinking more about the Fortran case for !$omp sections, there is an
> ambiguity.
> !$omp sections
> block
>   !$omp section
> end block
> is clear and !$omp end sections is optional, but
> !$omp sections
> block
> end block
> is ambiguous during parsing, it could be either followed by !$omp section
> and then the BLOCK would be first section, or by !$omp end sections and then
> it would be clearly the whole sections, with first section being empty
> inside of the block, or if it is followed by something else, it is
> ambiguous whether the block ... end block is part of the first section,
> followed by something and then we should be looking later for either
> !$omp section or !$omp end section to prove that, or if
> !$omp sections
> block
> end block
> was the whole sections construct and we shouldn't await anything further.
> I'm afraid back to the drawing board.

And I have to correct myself, there is no ambiguity in 5.2 here,
the important fact is hidden in sections/parallel sections being
block-associated constructs.  That means the body of the whole construct
has to be a structured-block, and by the 5.1+ definition of Fortran
structured block, it is either block ... end block or something that
doesn't start with block.
So,
!$omp sections
block
end block
a = 1
is only ambiguous in whether it is actually
!$omp sections
block
  !$omp section
end block
a = 1
or
!$omp sections
!$omp section
block
end block
!$omp end sections
a = 1
but both actually do the same thing, work roughly as !$omp single.
If one wants block statement as first in structured-block-sequence
of the first section, followed by either some further statements
or by other sections, then one needs to write
!$omp sections
!$omp section
block
end block
a = 1
...
!$omp end sections
or
!$omp sections
block
  block
  end block
  a = 1
...
end block

Your patch probably already handles it that way, but we again need
testsuite coverage to prove it is handled the way it should in all these
cases (and that we diagnose what is invalid).

Jakub

Re: [PATCH, OpenMP 5.1, Fortran] Strictly-structured block support for OpenMP directives

[Jakub Jelinek via Fortran]

On Thu, Oct 07, 2021 at 07:09:07PM +0200, Jakub Jelinek wrote:
> The workshare/parallel workshare case is unclear, I've filed
> https://github.com/OpenMP/spec/issues/3153
> for it.  Either don't allow block if workshare_stmts_only for now
> until that is clarified, or if we do, we need to make sure that it does the
> expected thing, does that gfc_trans_block_construct call ensure it?
> 
> Then we have the
> https://github.com/OpenMP/spec/issues/3154
> issue Tobias discovered, if that issue is resolved to end always applying to
> the directive before the block statement, I think your patch handles it that
> way but we want testsuite coverage for some of those cases.

Just want to follow-up on this, we now have resolutions of the
https://github.com/OpenMP/spec/issues/3153
https://github.com/OpenMP/spec/issues/3154
https://github.com/OpenMP/spec/issues/3155
issues and we can use that to guide this patch.
BLOCK is now explicitly allowed for workshare around the body of
workshare/parallel workshare or around the body of critical in it but not
arbitrarily nested.  My understanding of the patch is that it most likely
implements that, just we need a testsuite coverage that
!$omp workshare
block
  a = 1
  b = 2
  !$omp critical
  block
c = 3
  end block
end block
is fine (also with !$omp end {criticial,workshare} after the block), but
that
!$omp workshare
a = 1
block
  b = 2
  c = 3
end block
!$omp end workshare
etc. is diagnosed.
For Tobias' issue that !$omp end whatever after end block for strictly
structured block binds to the directive above the strictly structured block
I think the patch also implements it but we want again testsuite coverage,
that
subroutine foo
!$omp parallel
!$omp parallel
block
end block
!$omp end parallel
!$omp end parallel
end subroutine foo
subroutine bar
!$omp teams
!$omp parallel
block
end block
!$omp end teams
end subroutine bar
is fine while e.g.
subroutine baz
!$omp parallel
!$omp parallel
block
end block
!$omp end parallel
end subroutine baz
is not (!$omp end parallel pairs with the inner parallel rather than outer,
and the outer parallel's body doesn't start with BLOCK, so needs to be
paired with its !$omp end parallel).
And lastly, the 3rd ticket clarifies that for the separating directives
for Fortran basically the 5.0 state remains except that the body can be now
also optionally wrapped in a single BLOCK.
(And for C/C++ allows no statements at all in between the separating
directives or after/before them but still requires the {}s around it like
5.1 and earlier.  Here we implement the 5.1 wording and let's stay with
that.)
Thinking more about the Fortran case for !$omp sections, there is an
ambiguity.
!$omp sections
block
  !$omp section
end block
is clear and !$omp end sections is optional, but
!$omp sections
block
end block
is ambiguous during parsing, it could be either followed by !$omp section
and then the BLOCK would be first section, or by !$omp end sections and then
it would be clearly the whole sections, with first section being empty
inside of the block, or if it is followed by something else, it is
ambiguous whether the block ... end block is part of the first section,
followed by something and then we should be looking later for either
!$omp section or !$omp end section to prove that, or if
!$omp sections
block
end block
was the whole sections construct and we shouldn't await anything further.
I'm afraid back to the drawing board.

Jakub

Re: [RFC] Fixing PR102685 and testsuite issues

[Tobias Burnus]

Dear all, hello Harald,

On 12.10.21 22:50, Harald Anlauf wrote:

while working on a fix for PR102685, I encountered issues with the testsuite.

...

(1) gfortran.dg/derived_constructor_char_1.f90
The constructor is shorter than the array component txt in DT t5.
Commit r0-101989.
@Tobias: can you comment?

Simply change 'txt(4)' to 'txt(2)'.

(2) gfortran.dg/pr70931.f90
Committed by Richard Biener, fixing an ICE on an invalid testcase by Gerhard.
I think we safely can mark this one as invalid and emit an error, right?


No - we still want to check the bug fix in gcc/dwarf2out.c's 
native_encode_initializer,
i.e.
- if (val
+ if (val && fieldsize != 0

I think you should just change:
+   type(t), parameter :: z = t(1, [2])
to ... = t(1, [])


(3) gfortran.dg/transfer_simplify_2.f90

The constructor has more elements than the array component in the DT and is
invalid.

Commit r0-80854.


We had already before:
"Fixed invalid initialization" and "Fixed invalid array constructor."
for that file. Thus, changing it again to fix a bug is fine :-)

That looks like a combined pasto + thinko. In
   subroutine dt_to_integer1
real, parameter   :: r1(4) = (/1.0_4,2.0_4,3.0_4,4.0_4/)
type :: mytype
  integer(4) :: i(4)
  real(4) :: x(4)
end type mytype

and in the next one:
  subroutine character16_to_dt
character(16), parameter ::  c1 = "abcdefghijklmnop"
character(16)::  c2 = c1
type :: mytype
  real(4) :: x(2)
end type mytype

type (mytype), parameter :: dt1(2) = transfer (c1, mytype 
((/1.0,2.0,3.0,4.0/)), 2)
...
if (any (dt1(1)%x .ne. dt2(1)%x)) STOP 12
if (any (dt1(2)%x .ne. dt2(2)%x)) STOP 13
  end subroutine character16_to_dt

I think you can simply remove the ',3.0, 4.0' in the 'dt1' line.


Along these lines, I think we should always reject code having too many elements
in a constructor.  Is there agreement on this?  This would need fixing case (3).

Is there a reason to accept cases where the constructor is shorter than the 
array
component in the DT?  Some extension?  Shall we give a warning for these cases
instead of an error?  Based on -std flags?


At the moment I do not see any good reason for having
too many or too few elements. Also the testsuite use
indicates rather bugs and not intentional use.

Thus, I would simply give an error in either case.
I think that also matches the behavior of other
compilers such as ifort but I have not tested it.
(I think you alluded that you did test it with ifort
and it gave errors for both too long and too short
array constructors.)

I would change the testcases as mentioned, but maybe
it makes sense to add the invalid use in the new
testcase as some might exercise some corner cases
(size = 0, ...)?

As you had "derived_constructor_char_1.f90" in the list:

Having ["abc", "a", "ab cdefg"] is also invalid – as either the
strings have to be changed (space-padded on the right) to have
the same length or a typespec as in
  [ character(len=) :: "", ".." ]
has to be used. But I can see that it easily can happen that the
strings are of different length and the user then expects that
the compiler handles it correctly™, especially before the
typespec was permitted. (GCC attempts to do this, including
warnings/errors in that case.)

But for array elements, it seems to be unlikely that there is
size mismatch on purpose or by accidentally expecting that
the compiler corrects this.


Any constructive comment appreciated!


I wonder why you are only interested in constructive ones ... ;-)

Thanks,

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

Re: [PATCH] PR fortran/102716 - ICE in gfc_validate_kind(): Got bad kind

[Thomas Koenig via Fortran]

Hi Harald,


another simple and obvious fix: we need to reorder the argument checks
to the SHAPE intrinsic so that invalid KIND arguments can be detected.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

As I consider this a safe fix, I'd like to backport to suitable branches.


Also OK for both.

Thanks a lot for the patch!

Best regards

Thomas

Re: [PATCH] PR fortran/102717 - ICE in gfc_simplify_reshape, at fortran/simplify.c:6843

[Thomas Koenig via Fortran]

H Harald,


when simplifying RESHAPE we hit a gcc_assert for negative entries in the
SHAPE array.  Obvious solution: replace gcc_assert by an error message.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

As this is a safe fix, I'd like to backport to suitable branches.


OK for both.

Thanks for the patch!

Best regards

Thomas