Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding a proposal to allow complex pointers to real arrays and vice-versa #325

Merged
merged 25 commits into from
Jun 30, 2024

Conversation

PierUgit
Copy link
Contributor

@PierUgit PierUgit commented Jan 3, 2024

Following the discussion in #323

Comments about the split-complex layout
@certik
Copy link
Member

certik commented Jan 4, 2024

I think this is a good proposal now. You can ask at the Fortran Discourse to get more feedback.

@everythingfunctional
Copy link
Member

IMO, the additional intrinsics and/or new syntax are superfluous. The storage sequence is already sufficiently defined (or is nearly trivial to add clarification in case it's not) that all that's needed is to say that a complex pointer may be associated with a real target of the same kind and vice-versa. All the other existing constraints and semantics should take care of everything else, barring the edge case of ensuring that the size of the real target must be even.

@PierUgit
Copy link
Contributor Author

PierUgit commented Jan 4, 2024

IMO, the additional intrinsics and/or new syntax are superfluous.

Maybe. I wanted such association to be explicit, in order to avoid possible mistakes, but it can also make sense to stick to the usual syntax.

@PierUgit
Copy link
Contributor Author

PierUgit commented Jan 5, 2024

Actually, for full consistency, I also think that associating a real actual argument to a complex dummy argument, and vice-versa, should be allowed, with the same rules and restrictions as for the pointer association.

EDIT: I am actually wondering if the proposal should be rewritten in a more general way, allowing association between real and complex, be it through pointers or arguments (or equivalence, or common blocks, but I don't think it's worth elaboring more on these obsolescent features). Also, since there would not be any new syntax for the argument case, for consistency no new syntax should be created for the pointer case.

@ivan-pi
Copy link

ivan-pi commented Jan 5, 2024

Perhaps it's worth referencing the section 6.2.5, paragraph 17, page 39, from the draft C23 standard N3096 (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf),

Each complex type has the same representation and alignment requirements as an array type
containing exactly two elements of the corresponding real type; the first element is equal to the real part, and the second element to the imaginary part, of the complex number

This would imply that for the C-interoperable Fortran types complex(c_double) (or complex(c_double_complex)) and real(c_double), these two share the same representation and alignment requirements, meaning that pointer/storage association is possible.

@PierUgit
Copy link
Contributor Author

PierUgit commented Jan 5, 2024

@ivan-pi I actually think that this would be implied to make the association work as expected.

@everythingfunctional
Copy link
Member

I also think that associating a real actual argument to a complex dummy argument, and vice-versa, should be allowed, with the same rules and restrictions as for the pointer association.

Actually, you don't want to be able to do this, except perhaps for assumed size or explicit shape (which imply the actual must be contiguous), or explicitly contiguous. I.e. what happens in the following case?

r = [1., 2., 3., 4.]
call sub(r(1:3:2))
print *, r
contains
subroutine sub(c)
  complex, intent(inout) :: c(:)
  print *, c(1)%im, c(1)%re
  c(1)%im = 42.
end subroutine
end

or even crazier

r = [1., 2., 3., 4.]
call sub(r([4,1]))
print *, r
contains
subroutine sub(c)
  complex, intent(inout) :: c(:)
  print *, c(1)%im, c(1)%re
  c(1)%im = 42.
end subroutine
end

IMO, better not to get into all the complexities to define argument association, as you can still do it (albeit with more lines) with the pointer association.

@PierUgit
Copy link
Contributor Author

PierUgit commented Jan 5, 2024

@everythingfunctional It doesn't look so bad to me.

In your first example the compiler would detect a real non-contiguous actual argument vs a complex dummy argument, and it would apply the contiguity rule in this case: either it reports a compilation error, or it generates copy-in/copy-out. It's actually not very different from c => r(1:3:2) (not allowed because the RHS is non contiguous)

And the same for the second example (actually this one wouldn't be valid even with a real dummy argument, as an array-indexed actual argument can not be passed to an intent(out) dummy argument IIRC.

@PierUgit
Copy link
Contributor Author

PierUgit commented Jan 8, 2024

I meant that the compiler can check the contiguity (as well as other requirement) equally well for the argument association than for the pointer association.

@PierUgit
Copy link
Contributor Author

I have update the proposal with the optional extension to argument association.

Copy link
Member

@everythingfunctional everythingfunctional left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor grammatical corrections.

proposals/cmplx-real-pointers.txt Outdated Show resolved Hide resolved
proposals/cmplx-real-pointers.txt Outdated Show resolved Hide resolved
proposals/cmplx-real-pointers.txt Outdated Show resolved Hide resolved
@PierUgit
Copy link
Contributor Author

On the Fortran Discourse @everythingfunctional has raised a backward compatibility issue if argument association was allowed between real and complex types, so I think it's better to remove that part from the proposal and focus only on the pointer association.

@aradi
Copy link
Contributor

aradi commented Feb 14, 2024

It would be probably worth to consider to remove also the direct pointer association r => c and leave only the one achieved via the call of an intrinsic subroutine. This way, the changes to the standard would be very well localized. The "type breaking" would only occur at one place.

It would be probably also worth to make the subroutine name more generic (e.g. call associate_storage(r, c, [shape], [stat])), so that in the future it can also work for other type combinations, if they make sense (e.g. array of bytes/characters associated with integers, etc.) And I'd suggest to add a stat optional argument, which can report back, if the storage association was not successful (e.g. types are not compatible, size of the target is incommensurable with the pointer type, etc.)

@PierUgit
Copy link
Contributor Author

What's next with this proposal? What should I/we do now?

@certik
Copy link
Member

certik commented Apr 15, 2024

@PierUgit now you should attend a J3 meeting (at least remotely) and advocate for it. If you can't do that, find a committee member who can do that for you.

@everythingfunctional
Copy link
Member

It will go a long way if you commit to helping do the work on this all the way through the process. I.e.

  1. Use cases (which you've got a decent start on here)
  2. Requirements
  3. Specification
  4. Syntax
  5. Edits to the standard

I'm willing to upload papers and facilitate discussion in plenary on someone's behalf if you can't join the committee or attend the meetings, but I'm not going to advocate other members spend time on features I'm not passionate about. And I don't really have time to work on more than I am right now.

@PierUgit
Copy link
Contributor Author

@everythingfunctional Thank for the proposal. I've also sent a message on the J3 ML in case someone would want to push it. Let's see.

@certik
Copy link
Member

certik commented Apr 16, 2024

My suggestion as the first step would be to get the committee to agree that this is in scope and should move forward, i.e. that you get support for this in the appropriate subgroup at least. For that 1. is helpful. @PierUgit once you secure the preliminary approval, then it's time to do 2. - 5. And as @everythingfunctional said, it would help if you could agree that you will do it, if the committee gives a "preapproval".

@everythingfunctional
Copy link
Member

@certik , yes. We are currently at the phase of deciding what goes on the F202Y worklist. WG5 votes on the final list at the meeting in June. @PierUgit , you should view this paper as simply "I think the committee should work on this, and here's some initial ideas". It is basically pre step 1. But a tacit agreement that you will help with that work can make a difference in whether the committee will accept it.

@PierUgit
Copy link
Contributor Author

With some help I can definitely try.

@everythingfunctional
Copy link
Member

@PierUgit , did you still want me to submit this paper for the meeting in two weeks?

@PierUgit
Copy link
Contributor Author

@everythingfunctional Definitely, if you agree...

@everythingfunctional
Copy link
Member

Ok, I will submit it and let you know about follow-up if any is needed.

@everythingfunctional
Copy link
Member

It's submitted: https://j3-fortran.org/doc/year/24/24-129.txt

@PierUgit
Copy link
Contributor Author

Great, thanks!

@everythingfunctional
Copy link
Member

@PierUgit , you'll be happy to learn that J3 did find this idea interesting enough to pursue for F202Y, and WG5 is highly likely to put it on the work list this morning.

@PierUgit
Copy link
Contributor Author

That's good, and thanks for the help! BTW, now that the draft has been officially submitted to the meeting, the branch could be merged (I have updated the document by using the uploaded version).

@certik certik merged commit 608c3c2 into j3-fortran:master Jun 30, 2024
@certik
Copy link
Member

certik commented Jun 30, 2024

@PierUgit thanks for the proposal. I merged it, as requested. You can open a new PR for a subsequent paper(s).

@everythingfunctional thanks for representing the paper at the committee. This is a great example how the committee can involve the very large community out there to help. It works.

@PierUgit PierUgit deleted the real_complex_proposal branch August 19, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants