Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

2022-09-19 Thread Mikael Morin

Le 18/09/2022 à 22:55, Mikael Morin a écrit :
Assumed shape can be non-contiguous so have to be excluded, 

Thinking about it again, assumed shape are probably acceptable, but 
there should be a check for contiguousness on the actual argument.


Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

2022-09-19 Thread Mikael Morin

Le 18/09/2022 à 12:48, Richard Biener a écrit :



Does *(&a[1]) count as a pointer dereference?


Yes, technically.


  Even in the original dump it is already simplified to a straight a[1].


But this not anymore.  The check can probably be relaxed, it stems from the 
dual purpose of CLOBBER.

So the following makes the frontend-emitted IL valid, by handing the 
simplification over to the middle-end, but I can't help thinking that 
behavior won't be more reliable.



diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index f8fcd2d97d9..5fb9a3a536d 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -6544,8 +6544,9 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * 
sym,

  && !sym->attr.elemental)
{
  tree var;
- var = build_fold_indirect_ref_loc (input_location,
-parmse.expr);
+ var = build1_loc (input_location, INDIRECT_REF,
+   TREE_TYPE (TREE_TYPE 
(parmse.expr)),

+   parmse.expr);
  tree clobber = build_clobber (TREE_TYPE (var));
  gfc_add_modify (&clobbers, var, clobber);
}



Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

2022-09-19 Thread Richard Biener via Fortran
On Mon, Sep 19, 2022 at 9:31 AM Mikael Morin  wrote:
>
> Le 18/09/2022 à 12:48, Richard Biener a écrit :
> >
> >> Does *(&a[1]) count as a pointer dereference?
> >
> > Yes, technically.
> >
> >>   Even in the original dump it is already simplified to a straight a[1].
> >
> > But this not anymore.  The check can probably be relaxed, it stems from the 
> > dual purpose of CLOBBER.
> >
> So the following makes the frontend-emitted IL valid, by handing the
> simplification over to the middle-end, but I can't help thinking that
> behavior won't be more reliable.

I think that will just delay the folding.  We are basically relying on
the frontends to
restrict what they produce, the *ptr case was specifically for C++
this in CTOR/DTORs
and during inlining we throw away all clobbers that end up "wrong" but
I guess we
might have latent issues when we obfuscate the address in the caller enough and
get undesired simplification ...

>
> diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
> index f8fcd2d97d9..5fb9a3a536d 100644
> --- a/gcc/fortran/trans-expr.cc
> +++ b/gcc/fortran/trans-expr.cc
> @@ -6544,8 +6544,9 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol *
> sym,
>&& !sym->attr.elemental)
>  {
>tree var;
> - var = build_fold_indirect_ref_loc (input_location,
> -parmse.expr);
> + var = build1_loc (input_location, INDIRECT_REF,
> +   TREE_TYPE (TREE_TYPE
> (parmse.expr)),
> +   parmse.expr);
>tree clobber = build_clobber (TREE_TYPE (var));
>gfc_add_modify (&clobbers, var, clobber);
>  }
>


Re: [PATCH] Fortran: add IEEE_MODES_TYPE, IEEE_GET_MODES and IEEE_SET_MODES

2022-09-19 Thread FX via Fortran
Hi Mikael,

> Looks good, thanks.

Thank you for your reviews. This patch is committed to trunk: 
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=4637a1d293c978816ad622ba33e3a32a78640edd

FX

Re: [PATCH] Fortran 2018 rounding modes changes

2022-09-19 Thread FX via Fortran
Committed as 
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=4637a1d293c978816ad622ba33e3a32a78640edd

FX


> Le 10 sept. 2022 à 12:21, FX  a écrit :
> 
> ping
> (with fix for the typo Bernhard noticed)
> 
> <0001-Fortran-F2018-rounding-modes-changes.patch>
> 
>> Le 31 août 2022 à 20:29, FX  a écrit :
>> 
>> This adds new F2018 features, that are not really enabled (because their 
>> runtime support is optional).
>> 
>> 1. Add the new IEEE_AWAY rounding mode. It is unsupported on all known 
>> targets, but could be supported by glibc and AIX as part of the C2x 
>> proposal. Testing for now is minimal, but once a target supports that 
>> rounding mode, the tests will fail and we can fix them by expanding them.
>> 
>> 2. Add the optional RADIX argument to IEEE_SET_ROUNDING_MODE and 
>> IEEE_GET_ROUNDING_MODE. It is unused for now, because we do not support 
>> floating-point radices other than 2.
>> 
>> 
>> Regression-tested on x86_64-pc-linux-gnu, both 32- and 64-bit.
>> OK to commit?
>> 
>> 
>> <0001-fortran-Fortran-2018-rounding-modes-changes.patch>
> 



Re: [PATCH] Fortran 2018 rounding modes changes

2022-09-19 Thread Mikael Morin

Hello,

I'm coming (late) to this.

Le 31/08/2022 à 20:29, FX via Fortran a écrit :

This adds new F2018 features, that are not really enabled (because their 
runtime support is optional).


(...)


2. Add the optional RADIX argument to IEEE_SET_ROUNDING_MODE and 
IEEE_GET_ROUNDING_MODE. It is unused for now, because we do not support 
floating-point radices other than 2.


If we accept the argument, we have to support it somehow.
So for IEEE_GET_ROUNDING_MODE (*, 10), we should return IEEE_OTHER, 
shouldn't we?
There is no problem for IEEE_SET_ROUNDING_MODE (*, 10) as there is no 
way this to be a valid call if radix 10 is not supported, but changing a 
compile time error to a runtime unexpected behavior seems like a step 
backwards.




Re: [PATCH] Fortran 2018 rounding modes changes

2022-09-19 Thread FX via Fortran
Hi,

>> 2. Add the optional RADIX argument to IEEE_SET_ROUNDING_MODE and 
>> IEEE_GET_ROUNDING_MODE. It is unused for now, because we do not support 
>> floating-point radices other than 2.
> If we accept the argument, we have to support it somehow.
> So for IEEE_GET_ROUNDING_MODE (*, 10), we should return IEEE_OTHER, shouldn't 
> we?

I think I disagree. We do not support any real kind with radix 10, so calling 
IEEE_GET_ROUNDING_MODE with RADIX=10 is invalid. Or, in another interpretation, 
the rounding mode is whatever-you-want-to-call it, since you cannot perform 
arithmetic in that radix anyway.


> There is no problem for IEEE_SET_ROUNDING_MODE (*, 10) as there is no way 
> this to be a valid call if radix 10 is not supported, but changing a compile 
> time error to a runtime unexpected behavior seems like a step backwards.

What do you mean by that? The behavior is not unexpected, the value returned by 
IEEE_GET_ROUNDING_MODE for RADIX=10 is irrelevant and cannot be used for 
anything useful.

FX

Re: [PATCH] Fortran 2018 rounding modes changes

2022-09-19 Thread Mikael Morin

Le 19/09/2022 à 18:17, FX a écrit :

Hi,


2. Add the optional RADIX argument to IEEE_SET_ROUNDING_MODE and 
IEEE_GET_ROUNDING_MODE. It is unused for now, because we do not support 
floating-point radices other than 2.

If we accept the argument, we have to support it somehow.
So for IEEE_GET_ROUNDING_MODE (*, 10), we should return IEEE_OTHER, shouldn't 
we?


I think I disagree. We do not support any real kind with radix 10, so calling 
IEEE_GET_ROUNDING_MODE with RADIX=10 is invalid. Or, in another interpretation, 
the rounding mode is whatever-you-want-to-call it, since you cannot perform 
arithmetic in that radix anyway.


Hmm, not really convinced, but that's a possible interpretation, I guess.



There is no problem for IEEE_SET_ROUNDING_MODE (*, 10) as there is no way this 
to be a valid call if radix 10 is not supported, but changing a compile time 
error to a runtime unexpected behavior seems like a step backwards.


What do you mean by that? The behavior is not unexpected, the value returned by 
IEEE_GET_ROUNDING_MODE for RADIX=10 is irrelevant and cannot be used for 
anything useful.



My sentence was about IEEE_*S*ET_ROUNDING_MODE actually.
My point that we previously reported an error (at compile time) to a 
user that would try to pass a radix 10 while we now silently do... 
something else that was requested (i.e. set the radix 2 rounding mode).


I think it's worth at least documenting that only radix 2 is supported.


Re: [PATCH] Fortran 2018 rounding modes changes

2022-09-19 Thread FX via Fortran
Hi,

> Hmm, not really convinced, but that's a possible interpretation, I guess.

It seems to me to be in line with the handling of all other IEEE intrinsics: 
the user is responsible for querying support before calling any relevant 
routine. I admit that there is no explicit language in the case of the rounding 
mode, I will try to find time to ask for an official interpretation.


> My sentence was about IEEE_*S*ET_ROUNDING_MODE actually.
> My point that we previously reported an error (at compile time) to a user 
> that would try to pass a radix 10 while we now silently do... something else 
> that was requested (i.e. set the radix 2 rounding mode).

Oh sorry, I see what you mean and you’re totally right. This is an easy 
improvement, and while I believe it is undefined behaviour in any case, we can 
easily improve that.


> I think it's worth at least documenting that only radix 2 is supported.

That also makes sense. I’ll propose a patch.

Thanks,
FX

Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

2022-09-19 Thread Harald Anlauf via Fortran

Am 18.09.22 um 22:55 schrieb Mikael Morin:

Le 18/09/2022 à 20:32, Harald Anlauf a écrit :


Assumed shape will be on the easy side,
while assumed size likely needs to be excluded for clobbering.


Isn’t it the converse that is true?
Assumed shape can be non-contiguous so have to be excluded, but assumed
size are contiguous, so valid candidates for clobbering. No?


I really was referring here to *dummies*, as in the following example:

program p
  integer :: a(4)
  a = 1
  call sub (a(1), 2)
  print *, a
contains
  subroutine sub (b, k)
integer, intent(in)  :: k
integer, intent(out) :: b(*)
!   integer, intent(out) :: b(k)
if (k > 2) b(k) = k
  end subroutine sub
end program p

Assumed size (*) is just a contiguous hunk of memory of possibly
unknown size, which can be zero.  So you couldn't set a clobber
for the a(1) actual argument.


No way, really, arrays are going to be a maze of complexity.


Agreed.




Proxy ping [PATCH] Fortran: Fix function attributes [PR100132]

2022-09-19 Thread Harald Anlauf via Fortran
Dear all,

the following patch was submitted by Jose but never reviewed:

https://gcc.gnu.org/pipermail/fortran/2021-April/055946.html

Before, we didn't set function attributes properly when
passing polymorphic pointers, which could lead to
mis-optimization.

The patch is technically fine and regtests ok, although it
can be shortened slightly, which makes it more readable,
see attached.

When testing the suggested testcase I found that it was
accepted (and working fine) with NAG, but it was rejected
by both Intel and Cray.  This troubled me, but I think
it is standard conforming (F2018:15.5.2.7), while the
error messages issued by Intel

PR100132.f90(61): error #8300: If a dummy argument is allocatable or a pointer, 
and the dummy or its associated actual argument is polymorphic, both dummy and 
actual must be polymorphic with the same declared type or both must be 
unlimited polymorphic.   [S]
call set(s)
-^

and a similar one by Cray, suggest that they refer to
F2018:15.5.2.5, which IMHO does not apply here.
(The text in the error message seems very related to
the reasoning in Note 1 of that subsection).

I'd like to hear (read: read) a second opinion on that.

Thanks,
Harald

From 0b19cfc098554572279c8d19997df4823b426191 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Mon, 19 Sep 2022 22:00:45 +0200
Subject: [PATCH] Fortran: Fix function attributes [PR100132]

gcc/fortran/ChangeLog:

	PR fortran/100132
	* trans-types.cc (create_fn_spec): Fix function attributes when
	passing polymorphic pointers.

gcc/testsuite/ChangeLog:

	PR fortran/100132
	* gfortran.dg/PR100132.f90: New test.
---
 gcc/fortran/trans-types.cc | 15 +-
 gcc/testsuite/gfortran.dg/PR100132.f90 | 75 ++
 2 files changed, 88 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/PR100132.f90

diff --git a/gcc/fortran/trans-types.cc b/gcc/fortran/trans-types.cc
index 0ea7c74a6f1..c062a5b29d7 100644
--- a/gcc/fortran/trans-types.cc
+++ b/gcc/fortran/trans-types.cc
@@ -3054,12 +3054,23 @@ create_fn_spec (gfc_symbol *sym, tree fntype)
   for (f = gfc_sym_get_dummy_args (sym); f; f = f->next)
 if (spec_len < sizeof (spec))
   {
-	if (!f->sym || f->sym->attr.pointer || f->sym->attr.target
+	bool is_class = false;
+	bool is_pointer = false;
+
+	if (f->sym)
+	  {
+	is_class = f->sym->ts.type == BT_CLASS && CLASS_DATA (f->sym)
+	  && f->sym->attr.class_ok;
+	is_pointer = is_class ? CLASS_DATA (f->sym)->attr.class_pointer
+  : f->sym->attr.pointer;
+	  }
+
+	if (f->sym == NULL || is_pointer || f->sym->attr.target
 	|| f->sym->attr.external || f->sym->attr.cray_pointer
 	|| (f->sym->ts.type == BT_DERIVED
 		&& (f->sym->ts.u.derived->attr.proc_pointer_comp
 		|| f->sym->ts.u.derived->attr.pointer_comp))
-	|| (f->sym->ts.type == BT_CLASS
+	|| (is_class
 		&& (CLASS_DATA (f->sym)->ts.u.derived->attr.proc_pointer_comp
 		|| CLASS_DATA (f->sym)->ts.u.derived->attr.pointer_comp))
 	|| (f->sym->ts.type == BT_INTEGER && f->sym->ts.is_c_interop))
diff --git a/gcc/testsuite/gfortran.dg/PR100132.f90 b/gcc/testsuite/gfortran.dg/PR100132.f90
new file mode 100644
index 000..78ae6702810
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/PR100132.f90
@@ -0,0 +1,75 @@
+! { dg-do run }
+!
+! Test the fix for PR100132
+!
+
+module main_m
+  implicit none
+
+  private
+
+  public :: &
+foo_t
+
+  public :: &
+set,&
+get
+
+  type :: foo_t
+integer :: i
+  end type foo_t
+
+  type(foo_t), save, pointer :: data => null()
+
+contains
+
+  subroutine set(this)
+class(foo_t), pointer, intent(in) :: this
+
+if(associated(data)) stop 1
+data => this
+  end subroutine set
+
+  subroutine get(this)
+type(foo_t), pointer, intent(out) :: this
+
+if(.not.associated(data)) stop 4
+this => data
+nullify(data)
+  end subroutine get
+
+end module main_m
+
+program main_p
+
+  use :: main_m, only: &
+foo_t, set, get
+
+  implicit none
+
+  integer, parameter :: n = 1000
+
+  type(foo_t), pointer :: ps
+  type(foo_t),  target :: s
+  integer  :: i, j, yay, nay
+
+  yay = 0
+  nay = 0
+  do i = 1, n
+s%i = i
+call set(s)
+call get(ps)
+if(.not.associated(ps)) stop 13
+j = ps%i
+if(i/=j) stop 14
+if(i/=s%i) stop 15
+if(ps%i/=s%i) stop 16
+if(associated(ps, s))then
+  yay = yay + 1
+else
+  nay = nay + 1
+end if
+  end do
+  if((yay/=n).or.(nay/=0)) stop 17
+
+end program main_p
--
2.35.3



Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

2022-09-19 Thread Mikael Morin

Le 19/09/2022 à 21:46, Harald Anlauf a écrit :

Am 18.09.22 um 22:55 schrieb Mikael Morin:

Le 18/09/2022 à 20:32, Harald Anlauf a écrit :


Assumed shape will be on the easy side,
while assumed size likely needs to be excluded for clobbering.


Isn’t it the converse that is true?
Assumed shape can be non-contiguous so have to be excluded, but assumed
size are contiguous, so valid candidates for clobbering. No?


I really was referring here to *dummies*, as in the following example:

program p
   integer :: a(4)
   a = 1
   call sub (a(1), 2)
   print *, a
contains
   subroutine sub (b, k)
     integer, intent(in)  :: k
     integer, intent(out) :: b(*)
!   integer, intent(out) :: b(k)
     if (k > 2) b(k) = k
   end subroutine sub
end program p

Assumed size (*) is just a contiguous hunk of memory of possibly
unknown size, which can be zero.  So you couldn't set a clobber
for the a(1) actual argument.

Couldn't you clobber A entirely?  If no element of B is initialized in 
SUB, well, A has undefined values on return from SUB.  That's how 
INTENT(OUT) works.


Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

2022-09-19 Thread Thomas Koenig via Fortran



On 19.09.22 22:50, Mikael Morin wrote:

Le 19/09/2022 à 21:46, Harald Anlauf a écrit :

Am 18.09.22 um 22:55 schrieb Mikael Morin:

Le 18/09/2022 à 20:32, Harald Anlauf a écrit :


Assumed shape will be on the easy side,
while assumed size likely needs to be excluded for clobbering.


Isn’t it the converse that is true?
Assumed shape can be non-contiguous so have to be excluded, but assumed
size are contiguous, so valid candidates for clobbering. No?


I really was referring here to *dummies*, as in the following example:

program p
   integer :: a(4)
   a = 1
   call sub (a(1), 2)
   print *, a
contains
   subroutine sub (b, k)
 integer, intent(in)  :: k
 integer, intent(out) :: b(*)
!   integer, intent(out) :: b(k)
 if (k > 2) b(k) = k
   end subroutine sub
end program p

Assumed size (*) is just a contiguous hunk of memory of possibly
unknown size, which can be zero.  So you couldn't set a clobber
for the a(1) actual argument.

Couldn't you clobber A entirely?  If no element of B is initialized in 
SUB, well, A has undefined values on return from SUB.  That's how 
INTENT(OUT) works.


Yes, I think so - you are passing the starting element of an array
to an assumed-size array via storage association rules.

It has to be an explicit interface, of course, otherwise it is
unclear if an array or an array element is passed.

Best regards

Thomas