Skip to content

Unused locals in m_riemann_solver_lf.fpp, one occupying a GPU private() slot #1715

Closed

Description

@sbryngelson

Summary

src/simulation/m_riemann_solver_lf.fpp declares two locals that are never read. One of them also occupies a slot in a GPU private() clause, so every device thread carries storage for a value nothing uses.

Detail

variable occurrences in the file what they are
H_avg 1 the declaration only
c_sum_Yi_Phi 2 the declaration, plus one private() clause entry
real(wp) :: Cp_avg, Cv_avg, T_avg, eps, c_sum_Yi_Phi          ! line 62
real(wp) :: H_avg                                             ! line 76
...
$:GPU_PARALLEL_LOOP(collapse=3, private='[..., eps, c_sum_Yi_Phi, Cp_L, Cp_R, ...]')

Neither is ever assigned or read.

Not a regression

Occurrence counts are identical on master, so both predate the current work. Found while reviewing #1714 (the eos_state refactor), which touches this file but did not introduce them — likely leftovers from when the Lax-Friedrichs solver was factored out of a solver that did compute a Roe-averaged enthalpy and a chemistry sound-speed term, both of which the LF path does not need.

For comparison, the sibling solvers do use these: m_riemann_solver_hll.fpp and m_riemann_solver_hllc.fpp both read H_avg and c_sum_Yi_Phi in their Roe-average paths.

Impact

Minor. No functional effect — an unused local is free on CPU. The private() entry is the part with any cost: it reserves per-thread storage on device for a value that is never touched.

Suggested fix

Delete both declarations and remove c_sum_Yi_Phi from the private() clause. Confirm nothing else in the file references them first (a whole-file grep is sufficient given the counts above), and check that no #:if variant reintroduces a use.

Worth checking the other Riemann solvers for the same pattern while in there.

Activity

  1. sbryngelson commented on Aug 28, 2026

    @sbryngelson
    MemberAuthor

    Still present, and there are more than two. Re-checked on fix/speed-of-sound-from-state:

    variable occurrences uses outside the declaration and the private() clause
    H_avg 1 0
    c_sum_Yi_Phi 2 0
    Cp_avg 2 0
    Cv_avg 2 0
    T_avg 2 0
    eps 2 0

    So it is six, not two - Cp_avg, Cv_avg, T_avg and eps are dead in exactly the same way, and five
    of the six occupy a private() slot.

    The reason is now unambiguous: m_riemann_solver_lf.fpp never calls s_compute_average_state,
    s_compute_chemistry_average_state or s_compute_speed_of_sound_avg - zero call sites. Those six locals
    exist only to mirror the Roe-average path that HLL and HLLC have and LF does not, which matches this
    issue's guess that they are leftovers from when LF was factored out.

    Found independently while auditing c_sum_Yi_Phi for #1774; cross-referencing rather than duplicating.

  2. sbryngelson commented on Sep 2, 2026

    @sbryngelson
    MemberAuthor

    Fixed by #1762, merged as c9c8b0f. This issue was listed in that PR's Closes line but GitHub only auto-closes the first reference in a comma-separated list, so it stayed open; closing manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions