lfortran icon indicating copy to clipboard operation
lfortran copied to clipboard

Fix: deflake OpenMP integration tests

Open krystophny opened this issue 2 months ago • 3 comments

Summary

Deflake gfortran OpenMP integration tests by removing data races on shared variables.

Scope

  • integration_tests/openmp_31.f90
  • integration_tests/openmp_58.f90

Verification

  • gfortran stress run (openmp_58 critical): /tmp/lfortran-dev/9027/gfortran_openmp_58_stress_critical_2025-12-12.txt
  • gfortran stress run (openmp_31+58 atomic version earlier): /tmp/lfortran-dev/9027/gfortran_openmp_31_58_stress_2025-12-12.txt

Fixes: #9027 Fixes: #8613 Fixes: #8660 Fixes: #8770

krystophny avatar Dec 12 '25 16:12 krystophny

@certik This is now deflaked for gfortran: openmp_31 uses atomic update for phi(1), openmp_58 uses a critical section for total/index. Fixes #9027/#8613/#8660/#8770.

krystophny avatar Dec 12 '25 17:12 krystophny

semantic error: The clause update is not supported for parallel sections
  --> /home/runner/work/lfortran/lfortran/integration_tests/openmp_31.f90:12:9
   |
12 |         !$omp atomic update
   |         ^^^^^^^^^^^^^^^^^^^ 


Note: Please report unclear, confusing or incorrect messages as bugs at
https://github.com/lfortran/lfortran/issues.
[ 55%] Linking Fortran executable openmp_26

Is there another way to fix this? If not, then change the above error to a warning and say the clause is ignored. That should make it work.

certik avatar Dec 12 '25 17:12 certik

semantic error: The clause update is not supported for parallel sections
  --> /home/runner/work/lfortran/lfortran/integration_tests/openmp_31.f90:12:9
   |
12 |         !$omp atomic update
   |         ^^^^^^^^^^^^^^^^^^^ 


Note: Please report unclear, confusing or incorrect messages as bugs at
https://github.com/lfortran/lfortran/issues.
[ 55%] Linking Fortran executable openmp_26

Is there another way to fix this? If not, then change the above error to a warning and say the clause is ignored. That should make it work.

I think the "update" i not needed at all here and I removed it. Let's see what CI says now.

krystophny avatar Dec 12 '25 18:12 krystophny