Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]
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]
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]
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
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
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
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
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
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
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]
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]
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]
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]
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