Skip to content

Fix REAL*8 statement-function declarations and tab warnings in r10vol1.f / r10tap.f (no numerical change) - #18

Open
d-diaz wants to merge 1 commit into
FMSC-Measurements:masterfrom
d-diaz:upstream/fix-r10vol1-and-r10tap
Open

Fix REAL*8 statement-function declarations and tab warnings in r10vol1.f / r10tap.f (no numerical change)#18
d-diaz wants to merge 1 commit into
FMSC-Measurements:masterfrom
d-diaz:upstream/fix-r10vol1-and-r10tap

Conversation

@d-diaz

@d-diaz d-diaz commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Clears 102 gfortran -Wall warnings in two Region 10 files — r10vol1.f 78 → 2 and r10tap.f 26 → 0 — with no change to computed results. The compiler emits byte-identical machine code before and after, at both -O0 and -O2, so this is verifiably a diagnostics-only change.

Most of it comes from one line per file: seven statement functions are declared REAL*8 although their bodies are entirely single-precision, so every use of them warned about a narrowing that the compiler was already performing.

All line numbers below refer to the pre-change files, so they match what a reviewer sees on the left side of the diff. The two block-DO conversions shift everything after r10vol1.f:131 down by one and everything after :271 down by one more.

The warning

Verbatim, from gfortran -Wall -c r10vol1.f on gfortran 13.3.0:

r10vol1.f:644:20:

  644 |                BK = BB(D,H)
      |                    1
Warning: Possible change of value in conversion from REAL(8) to REAL(4) at (1) [-Wconversion]

Counts before this change:

Warning r10vol1.f r10tap.f
REAL(8)REAL(4) conversion [-Wconversion] 26 7
REAL(4)INTEGER(4) conversion [-Wconversion] 2 0
Nonconforming tab character [-Wtabs] 45 18
Label defined but not used [-Wunused-label] 1 1
Fortran 2018 deleted feature: DO termination 2 0
Unused dummy argument [-Wunused-dummy-argument] 2 0
Total 78 26

What triggers it

r10vol1.f:534 and r10tap.f:25 declare seven statement functions — the Region 10 taper and bark-thickness equations — as REAL*8:

      REAL LIMD,LOWD,LUPD,LMERCH,TAPER,HITOP,RH,DSX,BKWRC,DSI
      REAL HTTOT, HT1PRD, DBHOB, STUMP, TOP,RXL,DXL,HTEST
      REAL BKAYC,H,HEST,D,C,HTD,BK,RH32,RH40
      REAL*8 BKAC,DVA,BKWR,DVR,BB,DD2MI,DVREDA

Every dummy argument of those seven (RH, RH32, RH40, D, H, DBHOB) is declared default REAL on the three preceding lines, and every literal in every body is a default-real constant — there is no d0 anywhere in them. For example:

      BB(D,H) = (0.8467 + 0.0009144*D + 0.0003568*H)

A statement function's expression is evaluated in its own operand types, and the value is converted to the declared result type afterwards — the same rule that makes REAL*8 X; X = A*B evaluate A*B in single precision when A and B are REAL*4. So the REAL*8 declaration widened an already-rounded single-precision value to double, and every one of the 33 uses immediately narrowed it back at a REAL(4) assignment target:

      REAL BK
      ...
      BK = BB(D,H)

That round trip is what -Wconversion reports. The declaration is the actual defect; the warnings are its symptom.

The fix

Before (r10vol1.f:534, and identically r10tap.f:25):

      REAL*8 BKAC,DVA,BKWR,DVR,BB,DD2MI,DVREDA

After:

      REAL BKAC,DVA,BKWR,DVR,BB,DD2MI,DVREDA

This computes the same result because the expressions were always single-precision. The declaration only controlled the type of an intermediate that was immediately converted back — every one of the 33 call sites is a bare assignment into a REAL(4) target, with no site where the result feeds a wider expression. Removing the round trip removes nothing else.

Also in this change

Two implicit real→integer conversions in r10vol1.f made explicit, preserving the existing truncate-toward-zero behaviour:

-               NUMSEG = HT1PRD
+               NUMSEG = INT(HT1PRD)

-               itmp = anint((ht1prd-int(ht1prd))*10)
+               itmp = INT(ANINT((ht1prd-INT(ht1prd))*10))

Line 129, two statements away, already used the explicit form (NSEG16=INT(HT1PRD*2.0)) and produced no warning, so this simply makes the neighbours consistent. INT(ANINT(...)) is used rather than NINT(...) because it is the literal spelling of what the compiler already emits; the two are numerically equal, but NINT is a distinct intrinsic that can lower to a different instruction sequence.

Two DO loops terminated by an assignment rather than END DO or CONTINUE — a feature deleted in Fortran 2018 — converted to block DO. Labels 36 and 254 existed only as loop terminators; label 253, which sits on the second of those two DO statements, is a live GOTO target (branched to from two places) and was preserved in columns 1–5.

-                     DO 36 I=1,NSEG16
- 36                     PIELEN(I)=16.0
+                     DO I=1,NSEG16
+                        PIELEN(I)=16.0
+                     END DO

63 hard tab characters replaced with 6 spaces each. gfortran's fixed-form extension already advances a tab in columns 1–6 to column 7, so every effective column is unchanged. Two unused labels removed (r10vol1.f:898, r10tap.f:201), each on a statement that generates no code.

The two unused_dummy_argument warnings in r10vol1.f (MTOPS, SPFLG) are left alone deliberatelyR10VOL1's argument list is identical to R10VOL's, and dropping either would break that parity and any external caller's signature.

Test evidence

The strongest available check is not a numerical comparison but a direct one: compile the file before and after and diff the generated assembly.

gfortran -fPIC -cpp -O2 -S r10vol1.f -o pre.s     # before
gfortran -fPIC -cpp -O2 -S r10vol1.f -o post.s    # after
diff pre.s post.s
Check Result
Assembly, r10vol1.f, -O0 and -O2 byte-identical (only the .file directive differs, naming the temp copies)
Assembly, r10tap.f, -O0 and -O2 byte-identical
Double-precision arithmetic instructions in r10vol1.f 0 before, 0 after (605 single-precision ops, 0 cvtss2sd/cvtsd2ss, 37 powf, all unchanged)
Control perturbing one coefficient in its 6th significant digit makes the diff non-empty, confirming the comparison detects real changes
Regression suite (fork) 86/86 pass, unchanged

Compiled without -g so that debug line tables — which do shift, since the block-DO conversion adds two lines to r10vol1.f — do not enter the comparison.

A byte-identical result is stronger than a numerical test suite for a change of this kind: it says the compiler produced the same machine code, which holds for every possible input rather than for the inputs a test happens to sample. The instruction census makes the point plainly — the REAL*8 declaration was generating no double-precision code at all.

That evidence matters here for a second reason, disclosed below.

How to reproduce

With nothing but a stock gfortran:

gfortran -Wall -c r10vol1.f -o /tmp/r10vol1.o   # 78 warnings before, 2 after
gfortran -Wall -c r10tap.f  -o /tmp/r10tap.o    # 26 warnings before, 0 after

One thing to flag: r10vol1.f has no callers

In this repository R10VOL1 is never called — by anything. git log -S "R10VOL1(" shows it never has been, and nm over all objects reports r10vol1_, r10_hts_, r10gdib_, r10tc_ and r10mlen_ as defined and referenced nowhere. R10MLEN is not called even from within its own file, despite the comment at line 384 saying R10VOL1 calls it.

The live Region 10 path is volinit.f:468R10VOL (r10vol.f:3) → R10VOLO (r10volo.f:4), and R10VOL1's argument list is byte-identical to R10VOL's. R10TC (r10vol1.f:980) is a near-duplicate of R10TCO (r10volo.f:312), and R10_HTS (:505) mirrors R10HTS (profile.f:1643). It looks like a superseded implementation that was never removed.

That means no behavioural test can reach r10vol1.f, which is exactly why the evidence above is compile-output rather than numerical output. It also raises a question we are not positioned to answer: is r10vol1.f still wanted? You are far better placed to know whether FVS or a DLL consumer calls it — it is exported from the shared library and listed in vollib.vfproj. If it is obsolete, deleting it is a worthwhile change.

r10tap.f is genuinely live — reachable from R10VOL/R10VOLO — and now compiles clean.

Separate issue worth raising

In both files, ISP is left uninitialized for any species outside 042/242/098/351:

      IF(EQNUM(8:10).EQ.'042') ISP='AC'
      IF(EQNUM(8:10).EQ.'242') ISP='RC'
      IF(EQNUM(8:10).EQ.'098') ISP='SS'
      IF(EQNUM(8:10).EQ.'351') ISP='RA'

There is no ELSE and no default, and the species branch immediately below tests ISP against IAC/IRC/ISS/IRA. r10tap.f:93-96 is on the live path, and the Region 10 default tables include species 264 and 000, neither of which matches. Not touched here, since any default we chose would change dispatch — happy to open a separate issue if useful.

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