Stop the linter recommending a change that a faster helper would break
SplitLint knew that CALL restores A, B and Data Pointers 0 to 2, so a pointer set before a call is still set after it. That is true, and it made the tool give advice that was correct today and unsafe to take. Of the 178 redundant SETDs it found across the corpus, 122 were redundant ONLY because of that restore - the shape is everywhere, because it is how a helper is given its arguments: SETD.0 SbfsBlock SETD.2 SbfsFileStart CALL sbfsSetWord SETD.0 SbfsBlock <- flagged Removing that last line is right until sbfsSetWord is reached with RCAL, which restores nothing - and that is not hypothetical, it is what RCAL was added to this machine for, measured at close to halving the assembler's memory traffic. The failure would also be silent from the linter's side: it forgets everything across an RCAL, so it would stop reporting while the removals stayed removed. So a claim now ends at any call, for pointers and for registers, the way a claim about carry already did. 257 warnings become 127, and the redundant SETDs 178 become 54 - which is exactly the number an independent count of "no CALL in between" had arrived at separately. The fixture gained a SETD and an INIA repeated across a CALL, which must stay quiet, and the harness fails with the old behaviour put back. Two mistakes worth recording: the new expectations first pointed at the LABEL above the repeats rather than the repeats, which passes for free because nothing ever warns about a label; and the block landed in the middle of another check's comment, leaving that comment describing the code below it instead of its own.
This commit is contained in:
@@ -186,8 +186,19 @@ instruction, instructions with no ordinary fallthrough path, and one-byte Data P
|
||||
changes that can use `INCD` or `DECD`, including direct branches whose target is already
|
||||
the next labeled address. It also tracks symbolic Data Pointer bases and known offsets to
|
||||
find a `SETD` that reloads an address the pointer already holds. Device input and stack
|
||||
pops are excluded from dead-assignment checks because consuming their input is itself an effect. SplitLint's control-flow
|
||||
knowledge is deliberately local: labels begin reachable regions, while an unparsed
|
||||
pops are excluded from dead-assignment checks because consuming their input is itself an effect.
|
||||
|
||||
**Nothing it knows survives a call.** `CALL` really does restore A, B and Data Pointers 0
|
||||
to 2, so a pointer set before one is genuinely still set after it - and saying so produced
|
||||
advice that was correct today and unsafe to take. 122 of the 178 redundant `SETD`s it first
|
||||
found were redundant *only* because of that restore, and removing them would have become a
|
||||
wrong-pointer bug the moment the callee was reached with `RCAL` instead, which is what
|
||||
`RCAL` was added to this machine for. Worse, the linter would have gone quiet rather than
|
||||
complained, because it forgets everything across an `RCAL`. Fifty four recommendations that
|
||||
stay true are worth more than a hundred and seventy eight that are conditional on a change
|
||||
the project intends to make.
|
||||
|
||||
SplitLint's control-flow knowledge is otherwise deliberately local: labels begin reachable regions, while an unparsed
|
||||
directive or data item ends the current claim. It does not yet expand includes or build a
|
||||
complete control-flow graph. Warnings normally leave a successful exit status, while
|
||||
`--fatal-warnings` makes any warning fail the command for use in automated checks.
|
||||
|
||||
Reference in New Issue
Block a user