* [PATCH] Fix fortran/PR114024
@ 2024-02-21 18:30 Steve Kargl
2024-02-21 19:41 ` Jerry D
0 siblings, 1 reply; 14+ messages in thread
From: Steve Kargl @ 2024-02-21 18:30 UTC (permalink / raw)
To: fortran, gcc-patches
I have attached a patch to PR114024, see
https://gcc.gnu.org/pipermail/gcc-bugs/2024-February/854651.html
The patch contains a new testcase and passes regression
testing on x86_64-*-freebsd. Could someone castr an eye
over the patch and commit it?
--
Steve
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix fortran/PR114024
2024-02-21 18:30 [PATCH] Fix fortran/PR114024 Steve Kargl
@ 2024-02-21 19:41 ` Jerry D
2024-02-21 20:28 ` Harald Anlauf
0 siblings, 1 reply; 14+ messages in thread
From: Jerry D @ 2024-02-21 19:41 UTC (permalink / raw)
To: sgk, fortran, gcc-patches
On 2/21/24 10:30 AM, Steve Kargl wrote:
> I have attached a patch to PR114024, see
>
> https://gcc.gnu.org/pipermail/gcc-bugs/2024-February/854651.html
>
> The patch contains a new testcase and passes regression
> testing on x86_64-*-freebsd. Could someone castr an eye
> over the patch and commit it?
>
Hi Steve,
I looked it over and looks reasonable. I will try to apply it next few
days and test here. If OK, I will commit.
Jerry
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix fortran/PR114024
2024-02-21 19:41 ` Jerry D
@ 2024-02-21 20:28 ` Harald Anlauf
2024-02-21 20:31 ` Jerry D
2024-02-21 21:00 ` Steve Kargl
0 siblings, 2 replies; 14+ messages in thread
From: Harald Anlauf @ 2024-02-21 20:28 UTC (permalink / raw)
To: Jerry D, sgk, fortran, gcc-patches
On 2/21/24 20:41, Jerry D wrote:
> On 2/21/24 10:30 AM, Steve Kargl wrote:
>> I have attached a patch to PR114024, see
>>
>> https://gcc.gnu.org/pipermail/gcc-bugs/2024-February/854651.html
>>
>> The patch contains a new testcase and passes regression
>> testing on x86_64-*-freebsd. Could someone castr an eye
>> over the patch and commit it?
>>
>
> Hi Steve,
>
> I looked it over and looks reasonable. I will try to apply it next few
> days and test here. If OK, I will commit.
>
> Jerry
>
Actually the patch has two issues:
- a minor one: a new front-end memleak which can be avoided by
using either gfc_replace_expr (see its other uses)
Hint: try valgrind on f951
- it still fails on the following code, because the traversal
of the refs is incomplete / wrong:
program foo
implicit none
complex :: cmp(3)
real, pointer :: pp(:)
class(*), allocatable :: uu(:)
type t
real :: re
real :: im
end type t
type u
type(t) :: tt(3)
end type u
type(u) :: cc
cmp = (3.45,6.78)
cc% tt% re = cmp% re
cc% tt% im = cmp% im
allocate (pp, source = cc% tt% im) ! ICE
print *, pp
allocate (uu, source = cc% tt% im) ! ICE
end
This still crashes for me for the indicated cases.
Harald
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix fortran/PR114024
2024-02-21 20:28 ` Harald Anlauf
@ 2024-02-21 20:31 ` Jerry D
2024-02-21 21:00 ` Steve Kargl
1 sibling, 0 replies; 14+ messages in thread
From: Jerry D @ 2024-02-21 20:31 UTC (permalink / raw)
To: Harald Anlauf, sgk, fortran, gcc-patches
On 2/21/24 12:28 PM, Harald Anlauf wrote:
> On 2/21/24 20:41, Jerry D wrote:
>> On 2/21/24 10:30 AM, Steve Kargl wrote:
>>> I have attached a patch to PR114024, see
>>>
>>> https://gcc.gnu.org/pipermail/gcc-bugs/2024-February/854651.html
>>>
>>> The patch contains a new testcase and passes regression
>>> testing on x86_64-*-freebsd. Could someone castr an eye
>>> over the patch and commit it?
>>>
>>
>> Hi Steve,
>>
>> I looked it over and looks reasonable. I will try to apply it next few
>> days and test here. If OK, I will commit.
>>
>> Jerry
>>
>
> Actually the patch has two issues:
>
> - a minor one: a new front-end memleak which can be avoided by
> using either gfc_replace_expr (see its other uses)
> Hint: try valgrind on f951
Yes, I am learning to do that.
>
> - it still fails on the following code, because the traversal
> of the refs is incomplete / wrong:
>
> program foo
> implicit none
> complex :: cmp(3)
> real, pointer :: pp(:)
> class(*), allocatable :: uu(:)
> type t
> real :: re
> real :: im
> end type t
> type u
> type(t) :: tt(3)
> end type u
> type(u) :: cc
>
> cmp = (3.45,6.78)
> cc% tt% re = cmp% re
> cc% tt% im = cmp% im
> allocate (pp, source = cc% tt% im) ! ICE
> print *, pp
> allocate (uu, source = cc% tt% im) ! ICE
> end
>
> This still crashes for me for the indicated cases.
>
> Harald
>
Good catch. I will hold off until that is figured out.
Jerry
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix fortran/PR114024
2024-02-21 20:28 ` Harald Anlauf
2024-02-21 20:31 ` Jerry D
@ 2024-02-21 21:00 ` Steve Kargl
2024-02-21 21:20 ` Harald Anlauf
1 sibling, 1 reply; 14+ messages in thread
From: Steve Kargl @ 2024-02-21 21:00 UTC (permalink / raw)
To: Harald Anlauf; +Cc: Jerry D, fortran, gcc-patches
On Wed, Feb 21, 2024 at 09:28:16PM +0100, Harald Anlauf wrote:
> On 2/21/24 20:41, Jerry D wrote:
> > On 2/21/24 10:30 AM, Steve Kargl wrote:
> > > I have attached a patch to PR114024, see
> > >
> > > https://gcc.gnu.org/pipermail/gcc-bugs/2024-February/854651.html
> > >
> > > The patch contains a new testcase and passes regression
> > > testing on x86_64-*-freebsd. Could someone castr an eye
> > > over the patch and commit it?
> > >
> >
> > Hi Steve,
> >
> > I looked it over and looks reasonable. I will try to apply it next few
> > days and test here. If OK, I will commit.
> >
> > Jerry
> >
>
> Actually the patch has two issues:
>
> - a minor one: a new front-end memleak which can be avoided by
> using either gfc_replace_expr (see its other uses)
> Hint: try valgrind on f951
Unfortunately, valgrind does not work on AMD FX-8350 cpu.
memleak vs ICE. I think I'll take one over the other.
Probably need to free code->expr3 before the copy.
I tried gfc_replace_expr in an earlier patch. It did not
work.
> - it still fails on the following code, because the traversal
> of the refs is incomplete / wrong:
>
> program foo
> implicit none
> complex :: cmp(3)
> real, pointer :: pp(:)
> class(*), allocatable :: uu(:)
> type t
> real :: re
> real :: im
> end type t
> type u
> type(t) :: tt(3)
> end type u
> type(u) :: cc
>
> cmp = (3.45,6.78)
> cc% tt% re = cmp% re
> cc% tt% im = cmp% im
> allocate (pp, source = cc% tt% im) ! ICE
cc%tt%im isn't a complex-part-ref, so this seems to
be a different (maybe related) issue. Does the code
compile with 'source = (cc%tt%im)'? If so, perhaps,
detecting a component reference and doing the simply
wrapping with parentheses can be done.
> print *, pp
> allocate (uu, source = cc% tt% im) ! ICE
Ditto. Not to mention I know nothing about the implementation
of CLASS in gfortran.
--
Steve
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix fortran/PR114024
2024-02-21 21:00 ` Steve Kargl
@ 2024-02-21 21:20 ` Harald Anlauf
2024-02-21 21:42 ` Steve Kargl
0 siblings, 1 reply; 14+ messages in thread
From: Harald Anlauf @ 2024-02-21 21:20 UTC (permalink / raw)
To: sgk; +Cc: Jerry D, fortran, gcc-patches
On 2/21/24 22:00, Steve Kargl wrote:
> Unfortunately, valgrind does not work on AMD FX-8350 cpu.
Do you mean valgrind does not work at all?
For gcc, you need to configure --enable-valgrind-annotations
to not get bogus warnings.
> memleak vs ICE. I think I'll take one over the other.
> Probably need to free code->expr3 before the copy.
Yep.
> I tried gfc_replace_expr in an earlier patch. It did not
> work.
>> - it still fails on the following code, because the traversal
>> of the refs is incomplete / wrong:
>>
>> program foo
>> implicit none
>> complex :: cmp(3)
>> real, pointer :: pp(:)
>> class(*), allocatable :: uu(:)
>> type t
>> real :: re
>> real :: im
>> end type t
>> type u
>> type(t) :: tt(3)
>> end type u
>> type(u) :: cc
>>
>> cmp = (3.45,6.78)
>> cc% tt% re = cmp% re
>> cc% tt% im = cmp% im
>> allocate (pp, source = cc% tt% im) ! ICE
>
> cc%tt%im isn't a complex-part-ref, so this seems to
> be a different (maybe related) issue. Does the code
> compile with 'source = (cc%tt%im)'? If so, perhaps,
> detecting a component reference and doing the simply
> wrapping with parentheses can be done.
Yes, that's why I tried to make up the above example.
I think %re and %im are not too special, they work
here pretty much like component refs elsewhere.
>
>> print *, pp
>> allocate (uu, source = cc% tt% im) ! ICE
>
> Ditto. Not to mention I know nothing about the implementation
> of CLASS in gfortran.
>
You can ignore this one for now. It works if one places
parens around the source expr as for the other cases.
Harald
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix fortran/PR114024
2024-02-21 21:20 ` Harald Anlauf
@ 2024-02-21 21:42 ` Steve Kargl
2024-02-22 0:52 ` Steve Kargl
0 siblings, 1 reply; 14+ messages in thread
From: Steve Kargl @ 2024-02-21 21:42 UTC (permalink / raw)
To: Harald Anlauf; +Cc: Jerry D, fortran, gcc-patches
On Wed, Feb 21, 2024 at 10:20:43PM +0100, Harald Anlauf wrote:
> On 2/21/24 22:00, Steve Kargl wrote:
> > Unfortunately, valgrind does not work on AMD FX-8350 cpu.
>
> Do you mean valgrind does not work at all?
> For gcc, you need to configure --enable-valgrind-annotations
> to not get bogus warnings.
It does not work at all unless one tracks done an obscure
patch for valgrind. The FX-8350 has a tbm instruction.
Everything on the system would need be compiled with -mno-tbm
(or -fno-tbm).
% valgrind ./z
...
==88861== Process terminating with default action of signal 4 (SIGILL):
dumping core
==88861== Illegal opcode at address 0x4D30D87
==88861== at 0x4D30D87: ??? (in /lib/libc.so.7)
==88861== by 0x4D213DE: ??? (in /lib/libc.so.7)
==88861== by 0x4D2B935: ??? (in /lib/libc.so.7)
==88861== by 0x4D2C34E: ??? (in /lib/libc.so.7)
==88861== by 0x400AB8C: ??? (in /libexec/ld-elf.so.1)
==88861== by 0x4009828: ??? (in /libexec/ld-elf.so.1)
==88861== by 0x4006AE8: ??? (in /libexec/ld-elf.so.1)
> > memleak vs ICE. I think I'll take one over the other.
> > Probably need to free code->expr3 before the copy.
>
> Yep.
>
> > I tried gfc_replace_expr in an earlier patch. It did not
> > work.
>
>
>
> > > - it still fails on the following code, because the traversal
> > > of the refs is incomplete / wrong:
> > >
> > > program foo
> > > implicit none
> > > complex :: cmp(3)
> > > real, pointer :: pp(:)
> > > class(*), allocatable :: uu(:)
> > > type t
> > > real :: re
> > > real :: im
> > > end type t
> > > type u
> > > type(t) :: tt(3)
> > > end type u
> > > type(u) :: cc
> > >
> > > cmp = (3.45,6.78)
> > > cc% tt% re = cmp% re
> > > cc% tt% im = cmp% im
> > > allocate (pp, source = cc% tt% im) ! ICE
> >
> > cc%tt%im isn't a complex-part-ref, so this seems to
> > be a different (maybe related) issue. Does the code
> > compile with 'source = (cc%tt%im)'? If so, perhaps,
> > detecting a component reference and doing the simply
> > wrapping with parentheses can be done.
>
> Yes, that's why I tried to make up the above example.
> I think %re and %im are not too special, they work
> here pretty much like component refs elsewhere.
>
I see. The %re and %im complex-part-ref correspond to
ref->u.i == INQUIRY_RE and INQUIRY_IM, respectively.
A part-ref for a user-defined type doesn't have an
INQUIRY_xxx, so we'll need to see if there is a way to
easily identify, e.g., cc%tt%re from your testcase.
--
Steve
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix fortran/PR114024
2024-02-21 21:42 ` Steve Kargl
@ 2024-02-22 0:52 ` Steve Kargl
2024-02-22 20:22 ` Harald Anlauf
0 siblings, 1 reply; 14+ messages in thread
From: Steve Kargl @ 2024-02-22 0:52 UTC (permalink / raw)
To: Harald Anlauf; +Cc: Jerry D, fortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2403 bytes --]
On Wed, Feb 21, 2024 at 01:42:32PM -0800, Steve Kargl wrote:
> On Wed, Feb 21, 2024 at 10:20:43PM +0100, Harald Anlauf wrote:
> > On 2/21/24 22:00, Steve Kargl wrote:
> > > memleak vs ICE. I think I'll take one over the other.
> > > Probably need to free code->expr3 before the copy.
> >
> > Yep.
> >
> > > I tried gfc_replace_expr in an earlier patch. It did not
> > > work.
I tried freeing code->expr3 before assigning the new expression.
That leads to
% gfcx -c ~/gcc/gccx/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90
pid 69473 comm f951 has trashed its stack, killing
gfortran: internal compiler error: Illegal instruction signal terminated program f951
If I don't free code->expr3 but simply assign the new
expression from gfc_get_parentheses(), your example
now compiles are executes are expected. It now
allocate_with_source_28.f90. Caveat: I don't know
how to test the CLASS uu.
> > > > - it still fails on the following code, because the traversal
> > > > of the refs is incomplete / wrong:
> > > >
> > > > program foo
> > > > implicit none
> > > > complex :: cmp(3)
> > > > real, pointer :: pp(:)
> > > > class(*), allocatable :: uu(:)
> > > > type t
> > > > real :: re
> > > > real :: im
> > > > end type t
> > > > type u
> > > > type(t) :: tt(3)
> > > > end type u
> > > > type(u) :: cc
> > > >
> > > > cmp = (3.45,6.78)
> > > > cc% tt% re = cmp% re
> > > > cc% tt% im = cmp% im
> > > > allocate (pp, source = cc% tt% im) ! ICE
> > >
> > > cc%tt%im isn't a complex-part-ref, so this seems to
> > > be a different (maybe related) issue. Does the code
> > > compile with 'source = (cc%tt%im)'? If so, perhaps,
> > > detecting a component reference and doing the simply
> > > wrapping with parentheses can be done.
> >
> > Yes, that's why I tried to make up the above example.
> > I think %re and %im are not too special, they work
> > here pretty much like component refs elsewhere.
> >
>
> I see. The %re and %im complex-part-ref correspond to
> ref->u.i == INQUIRY_RE and INQUIRY_IM, respectively.
> A part-ref for a user-defined type doesn't have an
> INQUIRY_xxx, so we'll need to see if there is a way to
> easily identify, e.g., cc%tt%re from your testcase.
The attach patch uses ref->type == REF_COMPONENT to deal
with the above code.
--
Steve
[-- Attachment #2: pr114024.diff --]
[-- Type: text/x-diff, Size: 3134 bytes --]
diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc
index 5247d3d39d7..414248fe2e5 100644
--- a/gcc/fortran/trans-stmt.cc
+++ b/gcc/fortran/trans-stmt.cc
@@ -6354,9 +6354,35 @@ gfc_trans_allocate (gfc_code * code, gfc_omp_namelist *omp_allocate)
al = al->next)
vtab_needed = (al->expr->ts.type == BT_CLASS);
+ /* When expr3 is a variable, i.e., a very simple expression, then
+ convert it once here. */
+
gfc_init_se (&se, NULL);
- /* When expr3 is a variable, i.e., a very simple expression,
- then convert it once here. */
+
+ /* If one has source = z%re or z%im with z a complex array or
+ source = a%b%c where a or b is an array of a derived type, then
+ things can go sideways with the complex-part-refi or part-ref, so
+ wrap the entity in parentheses to force evaluation of an expression.
+ That is, the else-branch in the following if-else-stmt is entered. */
+
+ if (code->expr3->expr_type == EXPR_VARIABLE
+ && code->expr3->ts.type == BT_REAL
+ && code->expr3->ref)
+ {
+ gfc_ref *ref = code->expr3->ref;
+
+ while (ref->next)
+ ref = ref->next;
+
+ if (ref->u.i == INQUIRY_IM || ref->u.i == INQUIRY_RE
+ || ref->type == REF_COMPONENT)
+ {
+ gfc_expr *etmp = gfc_get_parentheses (code->expr3);
+ code->expr3 = gfc_copy_expr (etmp);
+ gfc_free_expr (etmp);
+ }
+ }
+
if (code->expr3->expr_type == EXPR_VARIABLE
|| code->expr3->expr_type == EXPR_ARRAY
|| code->expr3->expr_type == EXPR_CONSTANT)
diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90 b/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90
new file mode 100644
index 00000000000..d0f0f3c4a84
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90
@@ -0,0 +1,20 @@
+!
+! { dg-do run }
+!
+! fortran/PR114024
+! https://github.com/fujitsu/compiler-test-suite
+! Modified from Fortran/0093/0093_0130.f90
+!
+program foo
+ implicit none
+ complex :: cmp(3)
+ real, allocatable :: xx(:), yy(:), zz(:)
+ cmp = (3., 6.78)
+ allocate(xx, source = cmp%re) ! This caused an ICE.
+ allocate(yy, source = cmp(1:3)%re) ! This caused an ICE.
+ allocate(zz, source = (cmp%re))
+ if (any(xx /= [3., 3., 3.])) stop 1
+ if (any(yy /= [3., 3., 3.])) stop 2
+ if (any(zz /= [3., 3., 3.])) stop 3
+end program foo
+
diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 b/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90
new file mode 100644
index 00000000000..5eeeeb167cd
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90
@@ -0,0 +1,27 @@
+!
+! { dg-do run }
+! Testcase from Harald Anlauf
+! fortran/pr114024
+!
+program foo
+
+ implicit none
+
+ complex cmp(3)
+ real, pointer :: pp(:)
+ class(*), allocatable :: uu(:)
+ type t
+ real re, im
+ end type t
+ type u
+ type(t) tt(3)
+ end type u
+ type(u) :: cc
+
+ cmp = (3.,4.)
+ cc%tt%re = cmp%re
+ cc%tt%im = cmp%im
+ allocate (pp, source = cc%tt%im) ! ICE
+ if (any(pp /= [4., 4., 4.])) stop 1
+ allocate (uu, source = cc%tt%im) ! ICE
+end
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix fortran/PR114024
2024-02-22 0:52 ` Steve Kargl
@ 2024-02-22 20:22 ` Harald Anlauf
2024-02-22 21:01 ` Steve Kargl
0 siblings, 1 reply; 14+ messages in thread
From: Harald Anlauf @ 2024-02-22 20:22 UTC (permalink / raw)
To: sgk; +Cc: Jerry D, fortran, gcc-patches
Hi Steve!
On 2/22/24 01:52, Steve Kargl wrote:
> On Wed, Feb 21, 2024 at 01:42:32PM -0800, Steve Kargl wrote:
>> On Wed, Feb 21, 2024 at 10:20:43PM +0100, Harald Anlauf wrote:
>>> On 2/21/24 22:00, Steve Kargl wrote:
>>>> memleak vs ICE. I think I'll take one over the other.
>>>> Probably need to free code->expr3 before the copy.
>>>
>>> Yep.
>>>
>>>> I tried gfc_replace_expr in an earlier patch. It did not
>>>> work.
>
>
> I tried freeing code->expr3 before assigning the new expression.
> That leads to
>
> % gfcx -c ~/gcc/gccx/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90
> pid 69473 comm f951 has trashed its stack, killing
> gfortran: internal compiler error: Illegal instruction signal terminated program f951
Right. I also don't see what the lifetimes of the expressions are.
But is the gfc_copy_expr really needed? Wouldn't the following suffice?
code->expr3 = gfc_get_parentheses (code->expr3);
> If I don't free code->expr3 but simply assign the new
> expression from gfc_get_parentheses(), your example
> now compiles are executes are expected. It now
> allocate_with_source_28.f90. Caveat: I don't know
> how to test the CLASS uu.
>
>>>>> - it still fails on the following code, because the traversal
>>>>> of the refs is incomplete / wrong:
>>>>>
>>>>> program foo
>>>>> implicit none
>>>>> complex :: cmp(3)
>>>>> real, pointer :: pp(:)
>>>>> class(*), allocatable :: uu(:)
>>>>> type t
>>>>> real :: re
>>>>> real :: im
>>>>> end type t
>>>>> type u
>>>>> type(t) :: tt(3)
>>>>> end type u
>>>>> type(u) :: cc
>>>>>
>>>>> cmp = (3.45,6.78)
>>>>> cc% tt% re = cmp% re
>>>>> cc% tt% im = cmp% im
>>>>> allocate (pp, source = cc% tt% im) ! ICE
>>>>
>>>> cc%tt%im isn't a complex-part-ref, so this seems to
>>>> be a different (maybe related) issue. Does the code
>>>> compile with 'source = (cc%tt%im)'? If so, perhaps,
>>>> detecting a component reference and doing the simply
>>>> wrapping with parentheses can be done.
>>>
>>> Yes, that's why I tried to make up the above example.
>>> I think %re and %im are not too special, they work
>>> here pretty much like component refs elsewhere.
>>>
>>
>> I see. The %re and %im complex-part-ref correspond to
>> ref->u.i == INQUIRY_RE and INQUIRY_IM, respectively.
>> A part-ref for a user-defined type doesn't have an
>> INQUIRY_xxx, so we'll need to see if there is a way to
>> easily identify, e.g., cc%tt%re from your testcase.
>
> The attach patch uses ref->type == REF_COMPONENT to deal
> with the above code.
I actually wanted to draw your attention away from the
real/complex stuff, because that is not really the point.
When do we actually need to enforce the parentheses?
I tried the following, and it seems to work:
if (code->expr3->expr_type == EXPR_VARIABLE
&& is_subref_array (code->expr3))
code->expr3 = gfc_get_parentheses (code->expr3);
(Beware: this is not regtested!)
On the positive side, it not only seems to fix the cases in question,
but also substring references etc., like the following:
program foo
implicit none
complex :: cmp(3) = (3.45,6.78)
real, pointer :: pp(:)
integer, allocatable :: aa(:)
class(*), allocatable :: uu(:), vv(:)
type t ! pseudo "complex" type
real :: re
real :: im
end type t
type ci ! "complex integer" type
integer :: re
integer :: im
end type ci
type u
type(t) :: tt(3)
type(ci) :: ii(3)
end type u
type(u) :: cc
character(3) :: str(3) = ["abc","def","ghi"]
character(:), allocatable :: ac(:)
allocate (ac, source=str(1::2)(2:3))
print *, str(1::2)(2:3)
call my_print (ac)
cc% tt% re = cmp% re
cc% tt% im = cmp% im
cc% ii% re = nint (cmp% re)
cc% ii% im = nint (cmp% im)
print *, "re=", cc% tt% re
print *, "im=", cc% tt% im
allocate (pp, source = cc% tt% re)
print *, pp
allocate (uu, source = cc% tt% im)
call my_print (uu)
allocate (vv, source = cc% ii% im)
call my_print (vv)
contains
subroutine my_print (x)
class(*), intent(in) :: x(:)
select type (x)
type is (real)
print *, "'real':", x
type is (integer)
print *, "'integer':", x
type is (character(*))
print *, "'character':", x
end select
end subroutine my_print
end
Cheers,
Harald
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix fortran/PR114024
2024-02-22 20:22 ` Harald Anlauf
@ 2024-02-22 21:01 ` Steve Kargl
2024-02-22 21:32 ` Harald Anlauf
0 siblings, 1 reply; 14+ messages in thread
From: Steve Kargl @ 2024-02-22 21:01 UTC (permalink / raw)
To: Harald Anlauf; +Cc: Jerry D, fortran, gcc-patches
On Thu, Feb 22, 2024 at 09:22:37PM +0100, Harald Anlauf wrote:
> Hi Steve!
>
> On 2/22/24 01:52, Steve Kargl wrote:
> > On Wed, Feb 21, 2024 at 01:42:32PM -0800, Steve Kargl wrote:
> > > On Wed, Feb 21, 2024 at 10:20:43PM +0100, Harald Anlauf wrote:
> > > > On 2/21/24 22:00, Steve Kargl wrote:
> > > > > memleak vs ICE. I think I'll take one over the other.
> > > > > Probably need to free code->expr3 before the copy.
> > > >
> > > > Yep.
> > > >
> > > > > I tried gfc_replace_expr in an earlier patch. It did not
> > > > > work.
> >
> > I tried freeing code->expr3 before assigning the new expression.
> > That leads to
> >
> > % gfcx -c ~/gcc/gccx/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90
> > pid 69473 comm f951 has trashed its stack, killing
> > gfortran: internal compiler error: Illegal instruction signal terminated program f951
>
> Right. I also don't see what the lifetimes of the expressions are.
>
> But is the gfc_copy_expr really needed? Wouldn't the following suffice?
>
> code->expr3 = gfc_get_parentheses (code->expr3);
It's been awhile since I use gfc_copy_expr, gfc_replace_expr, etc.
I did not try the above. If that works, then we should use that
for simplicity.
> > If I don't free code->expr3 but simply assign the new
> > expression from gfc_get_parentheses(), your example
> > now compiles are executes are expected. It now
> > allocate_with_source_28.f90. Caveat: I don't know
> > how to test the CLASS uu.
> >
> > > > > > - it still fails on the following code, because the traversal
> > > > > > of the refs is incomplete / wrong:
> > > > > >
> > > > > > program foo
> > > > > > implicit none
> > > > > > complex :: cmp(3)
> > > > > > real, pointer :: pp(:)
> > > > > > class(*), allocatable :: uu(:)
> > > > > > type t
> > > > > > real :: re
> > > > > > real :: im
> > > > > > end type t
> > > > > > type u
> > > > > > type(t) :: tt(3)
> > > > > > end type u
> > > > > > type(u) :: cc
> > > > > >
> > > > > > cmp = (3.45,6.78)
> > > > > > cc% tt% re = cmp% re
> > > > > > cc% tt% im = cmp% im
> > > > > > allocate (pp, source = cc% tt% im) ! ICE
> > > > >
> > > > > cc%tt%im isn't a complex-part-ref, so this seems to
> > > > > be a different (maybe related) issue. Does the code
> > > > > compile with 'source = (cc%tt%im)'? If so, perhaps,
> > > > > detecting a component reference and doing the simply
> > > > > wrapping with parentheses can be done.
> > > >
> > > > Yes, that's why I tried to make up the above example.
> > > > I think %re and %im are not too special, they work
> > > > here pretty much like component refs elsewhere.
> > > >
> > >
> > > I see. The %re and %im complex-part-ref correspond to
> > > ref->u.i == INQUIRY_RE and INQUIRY_IM, respectively.
> > > A part-ref for a user-defined type doesn't have an
> > > INQUIRY_xxx, so we'll need to see if there is a way to
> > > easily identify, e.g., cc%tt%re from your testcase.
> >
> > The attach patch uses ref->type == REF_COMPONENT to deal
> > with the above code.
>
> I actually wanted to draw your attention away from the
> real/complex stuff, because that is not really the point.
> When do we actually need to enforce the parentheses?
This is essentially my concern. I was inserting parentheses
only if I determined they were needed (to avoid unnecessary
temporary variable). The code paththat enters the else portion
of the following if-else-stmt, where a temporary is created.
That is,
allocate(x, source=z%re) becomes allocate(x, source=(z%re))
and from code generation viewpoint this is
tmp = (z%re)
allocate(x, sourcer=tmp)
deallocate(tmp)
> I tried the following, and it seems to work:
>
> if (code->expr3->expr_type == EXPR_VARIABLE
> && is_subref_array (code->expr3))
> code->expr3 = gfc_get_parentheses (code->expr3);
>
> (Beware: this is not regtested!)
>
> On the positive side, it not only seems to fix the cases in question,
> but also substring references etc., like the following:
If the above passes a regression test, then by all means we should
use it. I did not consider the substring case. Even if unneeded
parentheses are inserted, which may cause generation of a temporary
variable, I hope users are not using 'allocate(x,source=z%re)' is
some deeply nested crazy loops structure.
BTW, my patch and I suspect your improved patch also
fixes 'allocate(x,mold=z%re)'. Consider,
complex z(3)
real, allocatable :: x(:)
z = 42
allocate(x, mold=z%re)
print *, size(x)
end
% gfortran13 -o z a.f90
a.f90:9:25:
9 | allocate(x, mold=z%re)
| 1
internal compiler error: in retrieve_last_ref, at fortran/trans-array.cc:6070
0x247d7a679 __libc_start1
/usr/src/lib/libc/csu/libc_start1.c:157
% gfcx -o z a.f90 && ./z
3
--
Steve
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Fix fortran/PR114024
2024-02-22 21:01 ` Steve Kargl
@ 2024-02-22 21:32 ` Harald Anlauf
2024-02-23 21:15 ` [PATCH, v2] " Harald Anlauf
0 siblings, 1 reply; 14+ messages in thread
From: Harald Anlauf @ 2024-02-22 21:32 UTC (permalink / raw)
To: sgk; +Cc: Jerry D, fortran, gcc-patches
On 2/22/24 22:01, Steve Kargl wrote:
> On Thu, Feb 22, 2024 at 09:22:37PM +0100, Harald Anlauf wrote:
>> On the positive side, it not only seems to fix the cases in question,
>> but also substring references etc., like the following:
>
> If the above passes a regression test, then by all means we should
> use it. I did not consider the substring case. Even if unneeded
> parentheses are inserted, which may cause generation of a temporary
> variable, I hope users are not using 'allocate(x,source=z%re)' is
> some deeply nested crazy loops structure.
First thing is code correctness. There are cases where the
allocation shall preserve the array bounds, which is where
we must avoid the parentheses at all cost. But these cases
should be very limited. (There are some code comments/TODOs
regarding this and an open PR by Tobias(?)).
The cases we are currently discussing are even requiring(!)
the resetting of the lower bounds to 1, so your suggestion
to enforce parentheses does not look unreasonable.
BTW: If someone uses allocate in a tight loop, he/she deserves
to be punished anyway...
> BTW, my patch and I suspect your improved patch also
> fixes 'allocate(x,mold=z%re)'. Consider,
>
> complex z(3)
> real, allocatable :: x(:)
> z = 42ha
> allocate(x, mold=z%re)
> print *, size(x)
> end
>
> % gfortran13 -o z a.f90
> a.f90:9:25:
>
> 9 | allocate(x, mold=z%re)
> | 1
> internal compiler error: in retrieve_last_ref, at fortran/trans-array.cc:6070
> 0x247d7a679 __libc_start1
> /usr/src/lib/libc/csu/libc_start1.c:157
>
> % gfcx -o z a.f90 && ./z
> 3
>
Nice! I completely forgot about MOLD...
So the only missing pieces are a really comprehensive testcase
and successful regtests...
Cheers,
Harald
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH, v2] Fix fortran/PR114024
2024-02-22 21:32 ` Harald Anlauf
@ 2024-02-23 21:15 ` Harald Anlauf
2024-02-23 21:32 ` rep.dot.nop
2024-02-23 21:34 ` Steve Kargl
0 siblings, 2 replies; 14+ messages in thread
From: Harald Anlauf @ 2024-02-23 21:15 UTC (permalink / raw)
To: gcc-patches; +Cc: fortran
[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]
Hi Steve, all,
here's an updated patch with an enhanced testcase that also
checks MOLD= besides SOURCE=.
Regtested on x86_64-pc-linux-gnu. Is it OK for mainline?
Cheers,
Harald
On 2/22/24 22:32, Harald Anlauf wrote:
> On 2/22/24 22:01, Steve Kargl wrote:
>> BTW, my patch and I suspect your improved patch also
>> fixes 'allocate(x,mold=z%re)'. Consider,
>>
>> complex z(3)
>> real, allocatable :: x(:)
>> z = 42ha
>> allocate(x, mold=z%re)
>> print *, size(x)
>> end
>>
>> % gfortran13 -o z a.f90
>> a.f90:9:25:
>>
>> 9 | allocate(x, mold=z%re)
>> | 1
>> internal compiler error: in retrieve_last_ref, at
>> fortran/trans-array.cc:6070
>> 0x247d7a679 __libc_start1
>> /usr/src/lib/libc/csu/libc_start1.c:157
>>
>> % gfcx -o z a.f90 && ./z
>> 3
>>
>
> Nice! I completely forgot about MOLD...
>
> So the only missing pieces are a really comprehensive testcase
> and successful regtests...
[-- Attachment #2: pr114024-v2.diff --]
[-- Type: text/x-patch, Size: 5446 bytes --]
From a176c2f44f812d82aeb430fadf23ab4b6dd5bd65 Mon Sep 17 00:00:00 2001
From: Steve Kargl <kargl@gcc.gnu.org>
Date: Fri, 23 Feb 2024 22:05:04 +0100
Subject: [PATCH] Fortran: ALLOCATE statement, SOURCE/MOLD expressions with
subrefs [PR114024]
PR fortran/114024
gcc/fortran/ChangeLog:
* trans-stmt.cc (gfc_trans_allocate): When a source expression has
substring references, part-refs, or %re/%im inquiries, wrap the
entity in parentheses to force evaluation of the expression.
gcc/testsuite/ChangeLog:
* gfortran.dg/allocate_with_source_27.f90: New test.
* gfortran.dg/allocate_with_source_28.f90: New test.
Co-Authored-By: Harald Anlauf <anlauf@gmx.de>
---
gcc/fortran/trans-stmt.cc | 10 ++-
.../gfortran.dg/allocate_with_source_27.f90 | 20 +++++
.../gfortran.dg/allocate_with_source_28.f90 | 90 +++++++++++++++++++
3 files changed, 118 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_27.f90
create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_28.f90
diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc
index 5247d3d39d7..e09828e218b 100644
--- a/gcc/fortran/trans-stmt.cc
+++ b/gcc/fortran/trans-stmt.cc
@@ -6355,8 +6355,14 @@ gfc_trans_allocate (gfc_code * code, gfc_omp_namelist *omp_allocate)
vtab_needed = (al->expr->ts.type == BT_CLASS);
gfc_init_se (&se, NULL);
- /* When expr3 is a variable, i.e., a very simple expression,
- then convert it once here. */
+ /* When expr3 is a variable, i.e., a very simple expression, then
+ convert it once here. If one has a source expression that has
+ substring references, part-refs, or %re/%im inquiries, wrap the
+ entity in parentheses to force evaluation of the expression. */
+ if (code->expr3->expr_type == EXPR_VARIABLE
+ && is_subref_array (code->expr3))
+ code->expr3 = gfc_get_parentheses (code->expr3);
+
if (code->expr3->expr_type == EXPR_VARIABLE
|| code->expr3->expr_type == EXPR_ARRAY
|| code->expr3->expr_type == EXPR_CONSTANT)
diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90 b/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90
new file mode 100644
index 00000000000..d0f0f3c4a84
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90
@@ -0,0 +1,20 @@
+!
+! { dg-do run }
+!
+! fortran/PR114024
+! https://github.com/fujitsu/compiler-test-suite
+! Modified from Fortran/0093/0093_0130.f90
+!
+program foo
+ implicit none
+ complex :: cmp(3)
+ real, allocatable :: xx(:), yy(:), zz(:)
+ cmp = (3., 6.78)
+ allocate(xx, source = cmp%re) ! This caused an ICE.
+ allocate(yy, source = cmp(1:3)%re) ! This caused an ICE.
+ allocate(zz, source = (cmp%re))
+ if (any(xx /= [3., 3., 3.])) stop 1
+ if (any(yy /= [3., 3., 3.])) stop 2
+ if (any(zz /= [3., 3., 3.])) stop 3
+end program foo
+
diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 b/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90
new file mode 100644
index 00000000000..976c567cf22
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90
@@ -0,0 +1,90 @@
+! { dg-do run }
+!
+! PR fortran/114024
+
+program foo
+ implicit none
+ complex :: cmp(3) = (3.,4.)
+ type ci ! pseudo "complex integer" type
+ integer :: re
+ integer :: im
+ end type ci
+ type cr ! pseudo "complex" type
+ real :: re
+ real :: im
+ end type cr
+ type u
+ type(ci) :: ii(3)
+ type(cr) :: rr(3)
+ end type u
+ type(u) :: cc
+
+ cc% ii% re = nint (cmp% re)
+ cc% ii% im = nint (cmp% im)
+ cc% rr% re = cmp% re
+ cc% rr% im = cmp% im
+
+ call test_substring ()
+ call test_int_real ()
+ call test_poly ()
+
+contains
+
+ subroutine test_substring ()
+ character(4) :: str(3) = ["abcd","efgh","ijkl"]
+ character(:), allocatable :: ac(:)
+ allocate (ac, source=str(1::2)(2:4))
+ if (size (ac) /= 2 .or. len (ac) /= 3) stop 11
+ if (ac(2) /= "jkl") stop 12
+ deallocate (ac)
+ allocate (ac, mold=str(1::2)(2:4))
+ if (size (ac) /= 2 .or. len (ac) /= 3) stop 13
+ deallocate (ac)
+ end
+
+ subroutine test_int_real ()
+ integer, allocatable :: aa(:)
+ real, pointer :: pp(:)
+ allocate (aa, source = cc% ii% im)
+ if (size (aa) /= 3) stop 21
+ if (any (aa /= cmp% im)) stop 22
+ allocate (pp, source = cc% rr% re)
+ if (size (pp) /= 3) stop 23
+ if (any (pp /= cmp% re)) stop 24
+ deallocate (aa, pp)
+ end
+
+ subroutine test_poly ()
+ class(*), allocatable :: uu(:), vv(:)
+ allocate (uu, source = cc% ii% im)
+ allocate (vv, source = cc% rr% re)
+ if (size (uu) /= 3) stop 31
+ if (size (vv) /= 3) stop 32
+ call check (uu)
+ call check (vv)
+ deallocate (uu, vv)
+ allocate (uu, mold = cc% ii% im)
+ allocate (vv, mold = cc% rr% re)
+ if (size (uu) /= 3) stop 33
+ if (size (vv) /= 3) stop 34
+ deallocate (uu, vv)
+ end
+
+ subroutine check (x)
+ class(*), intent(in) :: x(:)
+ select type (x)
+ type is (integer)
+ if (any (x /= cmp% im)) then
+ print *, "'integer':", x
+ stop 41
+ end if
+ type is (real)
+ if (any (x /= cmp% re)) then
+ print *, "'real':", x
+ stop 42
+ end if
+ type is (character(*))
+ print *, "'character':", x
+ end select
+ end
+end
--
2.35.3
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH, v2] Fix fortran/PR114024
2024-02-23 21:15 ` [PATCH, v2] " Harald Anlauf
@ 2024-02-23 21:32 ` rep.dot.nop
2024-02-23 21:34 ` Steve Kargl
1 sibling, 0 replies; 14+ messages in thread
From: rep.dot.nop @ 2024-02-23 21:32 UTC (permalink / raw)
To: fortran, Harald Anlauf, gcc-patches
On 23 February 2024 22:15:17 CET, Harald Anlauf <anlauf@gmx.de> wrote:
>Hi Steve, all,
>
>here's an updated patch with an enhanced testcase that also
>checks MOLD= besides SOURCE=.
>
>Regtested on x86_64-pc-linux-gnu. Is it OK for mainline?
LGTM
cheers
>
>Cheers,
>Harald
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH, v2] Fix fortran/PR114024
2024-02-23 21:15 ` [PATCH, v2] " Harald Anlauf
2024-02-23 21:32 ` rep.dot.nop
@ 2024-02-23 21:34 ` Steve Kargl
1 sibling, 0 replies; 14+ messages in thread
From: Steve Kargl @ 2024-02-23 21:34 UTC (permalink / raw)
To: Harald Anlauf; +Cc: gcc-patches, fortran
On Fri, Feb 23, 2024 at 10:15:17PM +0100, Harald Anlauf wrote:
> Hi Steve, all,
>
> here's an updated patch with an enhanced testcase that also
> checks MOLD= besides SOURCE=.
>
> Regtested on x86_64-pc-linux-gnu. Is it OK for mainline?
>
From my viewpoint, yes.
Thanks for finding a better solution than I had conjured.
--
Steve
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-02-23 21:34 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-21 18:30 [PATCH] Fix fortran/PR114024 Steve Kargl
2024-02-21 19:41 ` Jerry D
2024-02-21 20:28 ` Harald Anlauf
2024-02-21 20:31 ` Jerry D
2024-02-21 21:00 ` Steve Kargl
2024-02-21 21:20 ` Harald Anlauf
2024-02-21 21:42 ` Steve Kargl
2024-02-22 0:52 ` Steve Kargl
2024-02-22 20:22 ` Harald Anlauf
2024-02-22 21:01 ` Steve Kargl
2024-02-22 21:32 ` Harald Anlauf
2024-02-23 21:15 ` [PATCH, v2] " Harald Anlauf
2024-02-23 21:32 ` rep.dot.nop
2024-02-23 21:34 ` Steve Kargl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).