rocBLAS: Fix Fortran sample HIP result names#7059
Conversation
Partial upstream split from ROCm/TheRock#3928. Changes: - Rename the local HIP C binding function result from c_int to hip_status in the rocBLAS Fortran samples. - Declare the renamed result explicitly as integer(c_int) under implicit none. Rationale: - The samples imported iso_c_binding::c_int and also used result(c_int), which makes the function result name collide with the imported kind name. - Flang diagnoses the collision and missing explicit result type; gfortran also accepts the explicit non-conflicting result name. Validation: - Applied on current rocm-libraries develop after git pull --ff-only origin develop. - gfortran -fsyntax-only on example_fortran_{axpy,gemv,scal}.f90 with temporary stub rocblas modules. - git diff --check. Generated with Codex
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (77.83%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #7059 +/- ##
========================================
Coverage 65.12% 65.12%
========================================
Files 2081 2081
Lines 321794 321794
Branches 42301 42301
========================================
Hits 209551 209551
Misses 94736 94736
Partials 17507 17507
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
| interface | ||
| function hipMalloc(ptr, size) & | ||
| result(c_int) & | ||
| result(hip_status) & |
There was a problem hiding this comment.
I do not see why the result(hip_status) is needed. A result is only needed if a piece of Fortran code wants to use a different variable name to return the function's result.
When doing BIND(C) function result cannot be renamed and is implicitly returned from the called C function. So, all instances of result(hip_status) should be removed and replaced by declaration like this:
integer(c_int) :: hipMalloc after the implicit none. Same for all other functions.
To further improve the code, one could consider having an alias for the type for HIP error codes, e.g. a integer, parameter :: hip_error_t = c_int and then use hip_error_t instead.
There was a problem hiding this comment.
FTR - I am about the furthest thing from a Fortran programmer as one can get. Haven't touched it in 25 years. So can I put this back on you: as written at head, this does not compile with flang: That's a problem. I can attempt to fat-finger the fixes but if you have the experience to do so correctly, it would be more efficient for you to just take over the patch and meet the goal: it should syntax check properly with flang and gfortran. Today, afaict, it only passes with gfortran.
There was a problem hiding this comment.
(this was blocking switching the overall build to work consistently with flang, which is how I came to be incidentally here)
There was a problem hiding this comment.
Done. Please merge #7251 with this branch to absorb the changes.
There was a problem hiding this comment.
Pull request overview
This PR fixes a Fortran name collision in rocBLAS client sample code by renaming the bind(C) HIP function result variable from c_int (which conflicts with iso_c_binding::c_int) to hip_status, and explicitly declaring the result type under implicit none for stricter compiler compatibility (e.g., Flang).
Changes:
- Rename
result(c_int)toresult(hip_status)for local HIP C-binding interfaces in Fortran samples. - Add explicit
integer(c_int) :: hip_statusresult declarations for those interfaces to satisfyimplicit none. - Apply the same fix consistently across AXPY, GEMV, and SCAL samples.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| projects/rocblas/clients/samples/example_fortran_axpy.f90 | Renames HIP interface function result to hip_status and declares it as integer(c_int) to avoid c_int collision. |
| projects/rocblas/clients/samples/example_fortran_gemv.f90 | Same HIP result-name fix and explicit result typing for Flang compatibility. |
| projects/rocblas/clients/samples/example_fortran_scal.f90 | Same HIP result-name fix and explicit result typing for Flang compatibility. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Partial upstream split from ROCm/TheRock#3928.
Changes:
Rename the local HIP C binding function result from c_int to hip_status in the rocBLAS Fortran samples.
Declare the renamed result explicitly as integer(c_int) under implicit none.
Rationale:
The samples imported iso_c_binding::c_int and also used result(c_int), which makes the function result name collide with the imported kind name.
Flang diagnoses the collision and missing explicit result type; gfortran also accepts the explicit non-conflicting result name.
Validation:
Applied on current rocm-libraries develop after git pull --ff-only origin develop.
gfortran -fsyntax-only on example_fortran_{axpy,gemv,scal}.f90 with temporary stub rocblas modules.
git diff --check.
Generated with Codex