pragma unroll warning in hip

[yhmtsai]

This PR removes the pragma unroll warning in hip.
In general, the warning in ginkgo is caused by using break in the loop.

for () {
  if (condition) break;
  // other commands
}

->

for () {
  if (!condition) {
    // other commands
  }
}

to make pragma unroll work. ( I am not sure whether it affect the performance.)

copy_matrix unroll problem:
When using clang and hip to compile, copy_matrix gives the wrong result on custom precision.
I need to use #pragma unroll 1 to disable it.
I add local one variable to store source_row value to make everything work as expected.
I only go through some different part of fix_13.isa and unfix_13.isa. The assembly looks similar except the number of VGPR.
For some reasons, hip use many VGPR (registers) in the original version (without using the local variable)
unfix_13.isa contains NumSpilledVGPRs and use full VGPR (256), but fix_13.isa does not need to use full VGPR.
unroll_isa.zip
If someone is interested in the assembly code, I use # main - start and # main - end around

const auto val = source_row[i * increment];
destination[get_row_major_index<mod>(idx, row_perm, stride)] = static_cast<ResultValueType>(val);

or destination[get_row_major_index<mod>(idx, row_perm, stride)] = static_cast<ResultValueType>(source_row[i * increment]);
This issue ROCm/aomp#24 also mentions some weird things about NumSpilledVGPRs although the reason might be different from this PR.

Note. The original version gives wrong answer when casting to custom precision format and the wrong result are stable.

[codecov]

Codecov Report

Merging #492 into develop will decrease coverage by 0.18%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #492      +/-   ##
===========================================
- Coverage    88.64%   88.45%   -0.19%     
===========================================
  Files          268      268              
  Lines        16946    16894      -52     
===========================================
- Hits         15021    14944      -77     
- Misses        1925     1950      +25     

Impacted Files	Coverage Δ
omp/components/format_conversion.hpp	40.00% <0.00%> (-60.00%)	⬇️
include/ginkgo/core/matrix/ell.hpp	68.75% <0.00%> (-31.25%)	⬇️
include/ginkgo/core/base/lin_op.hpp	58.33% <0.00%> (-11.91%)	⬇️
include/ginkgo/core/matrix/hybrid.hpp	73.83% <0.00%> (-10.38%)	⬇️
include/ginkgo/core/matrix/sellp.hpp	81.81% <0.00%> (-8.43%)	⬇️
include/ginkgo/core/base/name_demangling.hpp	85.71% <0.00%> (-3.18%)	⬇️
reference/components/format_conversion.hpp	100.00% <0.00%> (ø)
include/ginkgo/core/base/utils.hpp	87.23% <0.00%> (+1.81%)	⬆️
include/ginkgo/core/matrix/csr.hpp	70.70% <0.00%> (+2.07%)	⬆️
include/ginkgo/core/matrix/dense.hpp	97.67% <0.00%> (+2.32%)	⬆️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ec56dd...2ec6529. Read the comment docs.

[yhmtsai]

This group is larger than warp_size, so we can not unroll this loop in the compile time.

[thoasm]

Makes sense, thanks for clearing that up.

[thoasm]

I am perfectly fine with changing the breaks, especially if it improves the unrolling. However, I think there are some minor errors in the code currently.

[tcojean]

LGTM except for Thomas' __GNUC__ comment.

[yhmtsai]

__GNUC__, which stands for supporting GNU C, is also defined in clang compiler.
hcc/hipcc also define clang, so I can not use clang to identify clang.
does anyone have an idea for identify clang/gcc only?

[upsj]

This is only device code, right? Wouldn't it be sufficient to differentiate between nvcc and the ROCm compiler? Why do we need to check for gcc?

[yhmtsai]

Yes, it is only on device.
For some reasons, using gcc in hip works well but using clang gives wrong answer.
Thus, I also wonder whether there are some potential bug in the code.

[tcojean]

For gcc:

#define GCC_COMPILER (defined(__GNUC__) && !defined(__clang__) && !defined(__INTEL_COMPILER))

For clang (maybe?):

#define CLANG_COMPILER (defined(__clang__) && !defined(__HIPCC__))

I did not check it but probably something like this.

[upsj]

Is there any way to look at the intermediate representations generated by ROCm? I guess they separate their code just like nvcc does, right? Because they should hopefully give an insight into the differences between the compilers.

[thoasm]

LGTM!

[upsj]

LGTM!

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

No Coverage information
0.0% Duplication