Re: [patch, fortram] Bug 120049 - ICE when using IS_C_ASSOCIATED ()

[Paul Richard Thomas]

HI Jerry,

The patch looks good to me. OK for mainline and for backporting. I never
quite know what to suggest for delaying backporting and so I will leave it
to your judgement.

Thanks for the patch.

Paul


On Tue, 6 May 2025 at 04:30, Jerry D  wrote:

> Attached patch fixes this by checking for BT_VOID and EXPR_FUNCTION.
>
> Thank you for guidance from Steve in the PR and Vincent for
> identifying the problem.
>
> Two test case files added to the testsuite.
>
> Regression tested on x86_64.
>
> OK for mainline?
>
> Since this breakage impacts gtk-fortran I would also like to backport to
> 14 and 15.
>
> Best regards,
>
> Jerry
>
> Author: Jerry DeLisle 
> Date:   Mon May 5 20:05:22 2025 -0700
>
>  Fortran: Fix ICE with use of c_associated.
>
>  PR fortran/120049
>
>  gcc/fortran/ChangeLog:
>
> * check.cc (gfc_check_c_associated): Modify checks to avoid
> ICE and allow use, intrinsic :: iso_c_binding from a separate
> module file.
>
>  gcc/testsuite/ChangeLog:
>
> * gfortran.dg/pr120049_a.f90: New test.
> * gfortran.dg/pr120049_b.f90: New test.
>
>

Re: [patch, Fortran] Fix PR 119928, rejects-valid 15/16 regression

[Thomas Koenig]

Hi Harald,


It appears that something is not right and generates wrong code with
the check enabled.  Can you have another look?


The problem was indeed that generating a formal from an actual
arglist is a bad idea when classes are involved.  Fixed in the
attached patch.  I think it still makes sense to remove the checks
when the other attributes are present (or PR96073 may come back
in different guise, even if I have to test case at present).


this is probably the best solution.  So let's go with it.


I have also converted the test to a run-time check.

Ok for trunk and backport to gcc-15?


OK for both.  Thanks for the patch!


Committed as r16-423-ge7a2b8b76ae0c8f1e49c780aa82ebb5f0325f515 so
far, will commit to gcc-15 in a few days.

Thanks for the review!

Best regards

Thomas

Re: [patch, fortram] Bug 120049 - ICE when using IS_C_ASSOCIATED ()

[Steve Kargl]

On Mon, May 05, 2025 at 08:30:09PM -0700, Jerry D wrote:
> Attached patch fixes this by checking for BT_VOID and EXPR_FUNCTION.
> 
> Thank you for guidance from Steve in the PR and Vincent for
> identifying the problem.
> 
> Two test case files added to the testsuite.
> 
> Regression tested on x86_64.
> 
> OK for mainline?
> 

I see Paul has already given you the ok, which is fine with me.

One small nit below.

> @@ -5955,30 +5955,40 @@ gfc_check_c_sizeof (gfc_expr *arg)
>  bool
>  gfc_check_c_associated (gfc_expr *c_ptr_1, gfc_expr *c_ptr_2)
>  {
> -  if (c_ptr_1->ts.type != BT_DERIVED
> -  || c_ptr_1->ts.u.derived->from_intmod != INTMOD_ISO_C_BINDING
> -  || (c_ptr_1->ts.u.derived->intmod_sym_id != ISOCBINDING_PTR
> -   && c_ptr_1->ts.u.derived->intmod_sym_id != ISOCBINDING_FUNPTR))
> +  if (c_ptr_1)

This test is not needed. c_ptr_1 is an required argument
to c_associated() and is guaranteed to be non-NULL at
this point.

>  {



>  }
>  
> +  if (c_ptr_2)

This test is needed because c_ptr_2 is an optional argument.
If it is not present, it is guaranteed to be NULL.

-- 
Steve

Re: [patch, fortram] Bug 120049 - ICE when using IS_C_ASSOCIATED ()

[Harald Anlauf]

Hi Jerry, all,

the new logic misses the following bad code:

  print *, c_associated(c_loc(val), 42)

This now ICEs here.

I suggest to not 'return true' too early before all arguments
have been checked.

Cheers,
Harald


On 5/6/25 19:15, Jerry D wrote:

On 5/6/25 12:44 AM, Paul Richard Thomas wrote:

HI Jerry,

The patch looks good to me. OK for mainline and for backporting. I 
never quite know what to suggest for delaying backporting and so I 
will leave it to your judgement.


Thanks for the patch.

Paul


Thanks Paul, committed as:

commit r16-428-gd0571638a6bad932b226ada98b167fa47a47d838

Jerry
--- snip ---

Re: [patch, fortram] Bug 120049 - ICE when using IS_C_ASSOCIATED ()

[Steve Kargl]

On Tue, May 06, 2025 at 07:43:41PM +0200, Harald Anlauf wrote:
> 
> the new logic misses the following bad code:
> 
>   print *, c_associated(c_loc(val), 42)
> 
> This now ICEs here.
> 
> I suggest to not 'return true' too early before all arguments
> have been checked.
> 

Good catch, Harald.  We probably need to check c_ptr_2 first
if it is present.

F2023, 18.2.3.2 has

  C_PTR_1 shall be a scalar of type C_PTR or C_FUNPTR.
  C_PTR_2 (optional) shall be a scalar of the same type as C_PTR_1.

if (c_ptr_2)
  {
 if (!same_type_check (c_ptr_1, 0, c_ptr_2, 1)
return false;
 ...
  }
-- 
Steve

Re: [patch, fortram] Bug 120049 - ICE when using IS_C_ASSOCIATED ()

[Jerry D]

On 5/6/25 12:44 AM, Paul Richard Thomas wrote:

HI Jerry,

The patch looks good to me. OK for mainline and for backporting. I never 
quite know what to suggest for delaying backporting and so I will leave 
it to your judgement.


Thanks for the patch.

Paul


Thanks Paul, committed as:

commit r16-428-gd0571638a6bad932b226ada98b167fa47a47d838

Jerry
--- snip ---

Re: [patch, fortram] Bug 120049 - ICE when using IS_C_ASSOCIATED ()

[Jerry D]

On 5/6/25 9:51 AM, Steve Kargl wrote:

On Mon, May 05, 2025 at 08:30:09PM -0700, Jerry D wrote:

Attached patch fixes this by checking for BT_VOID and EXPR_FUNCTION.

Thank you for guidance from Steve in the PR and Vincent for
identifying the problem.

Two test case files added to the testsuite.

Regression tested on x86_64.

OK for mainline?



I see Paul has already given you the ok, which is fine with me.

One small nit below.


@@ -5955,30 +5955,40 @@ gfc_check_c_sizeof (gfc_expr *arg)
  bool
  gfc_check_c_associated (gfc_expr *c_ptr_1, gfc_expr *c_ptr_2)
  {
-  if (c_ptr_1->ts.type != BT_DERIVED
-  || c_ptr_1->ts.u.derived->from_intmod != INTMOD_ISO_C_BINDING
-  || (c_ptr_1->ts.u.derived->intmod_sym_id != ISOCBINDING_PTR
- && c_ptr_1->ts.u.derived->intmod_sym_id != ISOCBINDING_FUNPTR))
+  if (c_ptr_1)




I mostly did this for code readability. I noticed it was not being 
checked before. It helps old buggers to organize their thoughts.



This test is not needed. c_ptr_1 is an required argument
to c_associated() and is guaranteed to be non-NULL at
this point.


  {





  }
  
+  if (c_ptr_2)


This test is needed because c_ptr_2 is an optional argument.
If it is not present, it is guaranteed to be NULL.



Understood.

Thanks Steve for your assist.

Jerry

Breakage on specifics_1.f90

[Jerry D]

I am seeing this today. I do not think it is related to my patch.

Running /home/jerry/dev/trunk/gcc/testsuite/gfortran.dg/dg.exp ...
FAIL: gfortran.dg/specifics_1.f90   -O2  execution test
FAIL: gfortran.dg/specifics_1.f90   -O3 -fomit-frame-pointer 
-funroll-loops -fpeel-loops -ftracer -finline-functions  execution test

FAIL: gfortran.dg/specifics_1.f90   -O3 -g  execution test

=== gfortran Summary ===

# of expected passes9
# of unexpected failures3

Regards,

Jerry

Re: Breakage on specifics_1.f90

[Sam James]

See https://gcc.gnu.org/PR120099.

Re: Breakage on specifics_1.f90

[Toon Moene]

On 5/6/25 19:25, Jerry D wrote:


I am seeing this today. I do not think it is related to my patch.

Running /home/jerry/dev/trunk/gcc/testsuite/gfortran.dg/dg.exp ...
FAIL: gfortran.dg/specifics_1.f90   -O2  execution test
FAIL: gfortran.dg/specifics_1.f90   -O3 -fomit-frame-pointer -funroll- 
loops -fpeel-loops -ftracer -finline-functions  execution test

FAIL: gfortran.dg/specifics_1.f90   -O3 -g  execution test

     === gfortran Summary ===

# of expected passes    9
# of unexpected failures    3


Not related - I saw it yesterday:

https://gcc.gnu.org/pipermail/gcc-testresults/2025-May/846097.html

Kind regards,

--
Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands

[PATCH] Fortran: fix passing of inquiry ref of complex array to TRANSFER [PR102891]

[Harald Anlauf]

Dear all,

here's another rather obvious case where a temporary is needed for
an inquiry reference of a complex array which is a component of a
derived type.  In contrast to PR119986, the argument is handled
within the code for the intrinsic TRANSFER, so that the other
patch did not catch the present one.

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

I'd also like to backport this one to 15-branch if this is ok.

Thanks,
Harald

From 981aa53bc258d3c3b75ecdcd33d993346db1fd12 Mon Sep 17 00:00:00 2001
From: Harald Anlauf 
Date: Tue, 6 May 2025 20:59:48 +0200
Subject: [PATCH] Fortran: fix passing of inquiry ref of complex array to
 TRANSFER [PR102891]

	PR fortran/102891

gcc/fortran/ChangeLog:

	* dependency.cc (gfc_ref_needs_temporary_p): Within an array
	reference, inquiry references of complex variables generally
	need a temporary.

gcc/testsuite/ChangeLog:

	* gfortran.dg/transfer_array_subref.f90: New test.
---
 gcc/fortran/dependency.cc |  6 ++-
 .../gfortran.dg/transfer_array_subref.f90 | 48 +++
 2 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gfortran.dg/transfer_array_subref.f90

diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
index 57c0c49391b..aa8a57a80e0 100644
--- a/gcc/fortran/dependency.cc
+++ b/gcc/fortran/dependency.cc
@@ -944,8 +944,12 @@ gfc_ref_needs_temporary_p (gfc_ref *ref)
 	   types), not in characters.  */
 	return subarray_p;
 
-  case REF_COMPONENT:
   case REF_INQUIRY:
+	/* Within an array reference, inquiry references of complex
+	   variables generally need a temporary.  */
+	return subarray_p;
+
+  case REF_COMPONENT:
 	break;
   }
 
diff --git a/gcc/testsuite/gfortran.dg/transfer_array_subref.f90 b/gcc/testsuite/gfortran.dg/transfer_array_subref.f90
new file mode 100644
index 000..b480dffd00b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/transfer_array_subref.f90
@@ -0,0 +1,48 @@
+! { dg-do run }
+! { dg-additional-options "-O2 -fdump-tree-optimized" }
+!
+! PR fortran/102891 - passing of inquiry ref of complex array to TRANSFER
+
+program main
+  implicit none
+  integer, parameter :: dp = 8
+
+  type complex_wrap1
+ complex(dp) :: z(2)
+  end type complex_wrap1
+
+  type complex_wrap2
+ complex(dp), dimension(:), allocatable :: z
+  end type complex_wrap2
+
+  type(complex_wrap1) :: x = complex_wrap1([ (1, 2), (3, 4) ])
+  type(complex_wrap2) :: w
+
+  w%z = x%z
+
+  ! The following statements should get optimized away...
+  if (size (transfer ( x%z%re ,[1.0_dp])) /= 2) error stop 1
+  if (size (transfer ((x%z%re),[1.0_dp])) /= 2) error stop 2
+  if (size (transfer ([x%z%re],[1.0_dp])) /= 2) error stop 3
+  if (size (transfer ( x%z%im ,[1.0_dp])) /= 2) error stop 4
+  if (size (transfer ((x%z%im),[1.0_dp])) /= 2) error stop 5
+  if (size (transfer ([x%z%im],[1.0_dp])) /= 2) error stop 6
+
+  ! ... while the following may not:
+  if (any  (transfer ( x%z%re ,[1.0_dp])  /= x%z%re)) stop 7
+  if (any  (transfer ( x%z%im ,[1.0_dp])  /= x%z%im)) stop 8
+
+  if (size (transfer ( w%z%re ,[1.0_dp])) /= 2) stop 11
+  if (size (transfer ((w%z%re),[1.0_dp])) /= 2) stop 12
+  if (size (transfer ([w%z%re],[1.0_dp])) /= 2) stop 13
+  if (size (transfer ( w%z%im ,[1.0_dp])) /= 2) stop 14
+  if (size (transfer ((w%z%im),[1.0_dp])) /= 2) stop 15
+  if (size (transfer ([w%z%im],[1.0_dp])) /= 2) stop 16
+
+  if (any  (transfer ( w%z%re ,[1.0_dp])  /= x%z%re)) stop 17
+  if (any  (transfer ( w%z%im ,[1.0_dp])  /= x%z%im)) stop 18
+
+  deallocate (w%z)
+end program main
+
+! { dg-final { scan-tree-dump-not "_gfortran_error_stop_numeric" "optimized" } }
-- 
2.43.0

Re: [patch, fortram] Bug 120049 - ICE when using IS_C_ASSOCIATED ()

[Jerry D]

On 5/6/25 10:59 AM, Steve Kargl wrote:

On Tue, May 06, 2025 at 07:43:41PM +0200, Harald Anlauf wrote:


the new logic misses the following bad code:

   print *, c_associated(c_loc(val), 42)

This now ICEs here.

I suggest to not 'return true' too early before all arguments
have been checked.



Good catch, Harald.  We probably need to check c_ptr_2 first
if it is present.



As I began to explore this I had just backported our original patch to 
15 branch to do more testing. On 15 I do not get the ICE Harald is 
seeing but it is on 16.


Here I see on 16:

$ gfc test.f90
test.f90:5:21:

5 | program tests_gtk_sup
  | ^
Error: mismatching comparison operand types
void *
integer(kind=4)
_2 = D.4648 == 42;
test.f90:5:21: internal compiler error: ‘verify_gimple’ failed
0x22ba961 internal_error(char const*, ...)
../../trunk/gcc/diagnostic-global-context.cc:517
0xe4833e verify_gimple_in_seq(gimple*, bool)
../../trunk/gcc/tree-cfg.cc:5345
0xac296a gimplify_body(tree_node*, bool)
../../trunk/gcc/gimplify.cc:20916
0xac2ba5 gimplify_function_tree(tree_node*)
../../trunk/gcc/gimplify.cc:21042
0x8d12c7 cgraph_node::analyze()
../../trunk/gcc/cgraphunit.cc:689
0x8d3e47 analyze_functions
../../trunk/gcc/cgraphunit.cc:1265
0x8d4dfd symbol_table::finalize_compilation_unit()
../../trunk/gcc/cgraphunit.cc:2574
Please submit a full bug report, with preprocessed source (by using 
-freport-bug).

Please include the complete backtrace with any bug report.
See  for instructions.

The ICE is somewhere else.

Harald's test case works fine on 15 with the original patch.

$ ls
gtk_sup.f90  gtk_sup.mod  gtk_sup.o  test.f90
$ rm gtk_sup.mod
$ rm gtk_sup.o
$ gfc15 -c gtk_sup.f90
$ gfc15 test.f90
$ ./a.out
 F