-
Notifications
You must be signed in to change notification settings - Fork 2
Remove C-wrapper for MPI_Waitall #98
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
Conversation
src/mpi.f90
Outdated
! Allocate temporary arrays for the C representations. | ||
type(c_ptr), allocatable :: c_requests(:) | ||
type(c_ptr), allocatable :: c_statuses(:) | ||
allocate(c_requests(count)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lfortran treats arrays of type(c_ptr) as scalar
No specific test file given. Will compile/run all .f90 tests...
semantic error: Array reference is not allowed on scalar variable
--> ../src/mpi.f90:688:13
|
688 | c_requests(i) = c_mpi_request_f2c(array_of_requests(i))
| ^^^^^^^^^^^^^
Note: Please report unclear, confusing or incorrect messages as bugs at
https://github.com/lfortran/lfortran/issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can just make it a non-allocatable by doing type(c_ptr) :: c_requests(count)
, would that be ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope it too gives error
aditya-trivedi tests wait_2 ≢ ~1 gfortran -c test.f90
aditya-trivedi tests wait_2 ≢ ?1 ~1 lfortran -c test.f90
semantic error: Array reference is not allowed on scalar variable
--> test.f90:6:5
|
6 | a(1) = c_loc(i)
| ^^^^
Note: Please report unclear, confusing or incorrect messages as bugs at
https://github.com/lfortran/lfortran/issues.
aditya-trivedi tests wait_2 ≢ ?1 ~1 cat test.f90
program main
use iso_c_binding, only: c_ptr, c_loc
type(c_ptr) :: a(10)
integer ,target:: i=1
! allocate(a(10))
a(1) = c_loc(i)
end program
Here is small mre which i tried
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. We've an error with GFortran + MPICH as well for both standalone test and POT3D as well, so there is something wrong with the Fortran code either in mpi.f90 or mpi_c_bindings.f90, I think we should try to fix that first.
Then we would've a better understanding of whether something is actually needed to be fixed in LFortran or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem we discovered in LFortran, probably is fixed with the PR: lfortran/lfortran#6839.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though, I still don't know exactly why this PR (to remove the C wrapper) doesn't work with MPICH but works with Open MPI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please fix LFortran for all bugs. That's why we are doing it also. :)
That PR is merged, so this might fix it --- can you try locally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still get an error, I'm extracting the MRE for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some other error albeit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is an LFortran error, then extract it and fix it. That's a very valuable thing to do.
Now there is new llvm error declare void @_lpython_free_argv()
asr_to_llvm: module failed verification. Error:
Call parameter type does not match function signature!
%25 = getelementptr inbounds i32, ptr %array_of_requests, i32 %23
i32 %26 = call i32 @MPI_Request_f2c(ptr %25)
Call parameter type does not match function signature!
%49 = getelementptr inbounds i32, ptr %4, i32 %47
i32 %50 = call i32 @MPI_Request_c2f(ptr %49) I'm extracting MRE for it |
opened in here lfortran/lfortran#6951 |
We've an approval from @certik , I'll merge this PR. And instead of necessarily needing to fix the issue: lfortran/lfortran#6951, we've used a simple workaround for now. I'm merging this PR now. |
I merged it!! |
No description provided.