Skip to content

fix(xlaqp2rk): do not modify [in] argument KMAX; introduce KBOUND#1202

Open
nakatamaho wants to merge 1 commit intoReference-LAPACK:masterfrom
nakatamaho:xLAQP2RK
Open

fix(xlaqp2rk): do not modify [in] argument KMAX; introduce KBOUND#1202
nakatamaho wants to merge 1 commit intoReference-LAPACK:masterfrom
nakatamaho:xLAQP2RK

Conversation

@nakatamaho
Copy link
Contributor

Summary

In SLAQP2RK, DLAQP2RK, CLAQP2RK, and ZLAQP2RK, the argument KMAX is documented as [in], but the implementation overwrites it:

KMAX = MIN( KMAX, MINMNFACT )

This is a contradiction between the documentation and the implementation.

Root cause

Fortran callers are insulated from this side effect because DGEQP3RK (and its variants) pass a temporary expression JMAX - J + 1 rather than a named variable, so the write to KMAX inside xLAQP2RK is silently discarded. However:

  • The [in] annotation is violated.
  • C and C++ translations of these routines fail to compile, since an rvalue cannot be bound to a non-const reference.
  • Any caller that passes a named variable directly (e.g., in testing or alternative driver routines) will have that variable silently corrupted.

Fix

Introduce a local variable KBOUND to hold the clamped value. KMAX is no longer written to.

-      KMAX = MIN( KMAX, MINMNFACT )
-      DO KK = 1, KMAX
+      KBOUND = MIN( KMAX, MINMNFACT )
+      DO KK = 1, KBOUND
 ...
-      K = KMAX
+      K = KBOUND

Files changed

  • SRC/slaqp2rk.f
  • SRC/dlaqp2rk.f
  • SRC/claqp2rk.f
  • SRC/zlaqp2rk.f

No behavior change

The computed result is identical for all existing callers. This is a correctness fix for the argument intent annotation and a robustness fix for non-Fortran consumers of these routines.

In all four variants (SLAQP2RK, DLAQP2RK, CLAQP2RK, ZLAQP2RK),
KMAX was declared as [in] in the documentation but was overwritten
internally via:

    KMAX = MIN( KMAX, MINMNFACT )

This is a bug in the reference LAPACK implementation: the Fortran
calling convention silently tolerates this when the caller passes
a temporary expression, but the intent annotation is violated and
C/C++ translations fail to compile (cannot bind rvalue to non-const
reference).

Introduce local variable KBOUND to hold the clamped value:

    KBOUND = MIN( KMAX, MINMNFACT )

and replace all subsequent uses of KMAX in the executable section
(loop bound and final assignment K = KBOUND) with KBOUND.
KMAX itself is no longer modified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant