Repository navigation
--cpu 68040 --no-fpu: substitute to 68ec040 instead of doing nothing - #168
Merged
Merged
Conversation
Fixes the 68040 half of issue #167. The m68k crate's own fpu_present doc says plainly it's a no-op for CpuType::M68040 ("the 040 models FPU absence through its EC/LC types instead") -- so --no-fpu combined with --cpu 68040 never actually disabled the FPU, while AttnFlags' AFF_FPU40 bit was (incorrectly) gated on the fpu flag anyway, telling the guest there was no FPU while FPU instructions kept working regardless. A real 68040 always has its on-die FPU, so there is no "68040, no FPU" machine to model honestly here. - attn_flags_for: AFF_FPU40 is now unconditional for M68040/M68060 (both always have an on-die FPU on real hardware), matching what the CPU core already does; --fpu no longer gates it for them. - check_cpu_fpu_combination (resolve, now Result<Options, String>): refuses --cpu 68040 combined with an *explicit* --no-fpu/FPU=false, pointing at --cpu 68ec040/68lc040 (the models that really have no FPU). Plain --cpu 68040 (fpu defaults to false) and --cpu 68040 --fpu (a no-op) are both still accepted. The --cpu 68060 --no-fpu crash (issue #167's other half) is a separate, deeper bug in how volamos routes the 68060's internal FPU-disabled exception, and is not touched by this change. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Godbk9UDW5obqYsdkXkyRR
Design change following review feedback: an explicit --no-fpu combined with --cpu 68040 is an unambiguous request once you know real hardware has a name for it (68ec040/68lc040), so automatically running as --cpu 68ec040 is more useful than a hard rejection -- with a one-line stderr note for visibility, since silently reinterpreting what --cpu named would be its own kind of surprise. - resolve_cpu_type_for_fpu (replaces check_cpu_fpu_combination): returns (CpuType, bool) instead of Result<(), String> -- substitutes cpu_type to M68EC040 when cpu_type == M68040 && fpu == Some(false) (explicit only, same as before), otherwise a no-op. - Options gains cpu_fpu_substituted; resolve() goes back to returning plain Options (no longer fallible -- there's nothing left to reject). run() prints the note when the flag is set. - --fpu/--no-fpu CLI help text and userdocs/CLI-Reference.md updated to describe the substitution instead of a refusal. 68060 --no-fpu is untouched, same as before this change. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Godbk9UDW5obqYsdkXkyRR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the
68040half of issue #167 (reported by @codewiz). Two real bugs, one root cause:--cpu 68040 --no-fpuhas no effect at all: them68kcrate's own doc onfpu_presentsays "[t]he 040 models FPU absence through its EC/LC types instead, so this stays true for it" -- the CPU core keeps executing every FPU instruction regardless of the flag.attn_flags_forwas (incorrectly) gatingAttnFlags'AFF_FPU40bit on the--fpuCLI flag for the fullM68040/M68060-- so a plain--cpu 68040(no--fpu/--no-fpuat all;fpudefaults tofalse) told the guest there was no FPU viaAttnFlags, while FPU instructions kept working anyway. A real 68040 always has its on-die FPU; there's no "68040, no FPU" machine on real hardware the way there is for 68020/68030 (plug-in 68881/68882).Fix
attn_flags_for:AFF_FPU40is now unconditional forCpuType::M68040/M68060-- matches what the CPU core already does, independent of--fpu/--no-fpu.resolve_cpu_type_for_fpu, wired intoresolve: an explicit--cpu 68040 --no-fpu(or a config file'sFPU=false) is substituted toCpuType::M68EC040-- the model them68kcrate itself treats as having no on-die FPU at all -- rather than refused. Checked against the pre-defaultOption<bool>specifically so a plain--cpu 68040(no flag,fpulater defaults tofalse) is not mistaken for an explicit ask -- that keeps matching real hardware (FPU present) exactly as it does today, just with theAttnFlagslie now fixed too.--cpu 68040 --fpustays accepted as a harmless no-op, unaffected.Options::cpu_fpu_substituted, checked inrun) for visibility -- silently reinterpreting what--cpunamed would be its own kind of surprise, even though the substitution itself is the more useful behavior than a hard rejection.--fpu/--no-fpuCLI help text anduserdocs/CLI-Reference.mdupdated to describe the substitution and note.Deliberately not touched:
--cpu 68060 --no-fpu, issue #167's other half. Them68kcrate can model an FPU-less 68060 (fpu_present = falsesetsPCR.DFP, handled entirely inside the crate'sdecode.rsviatake_fp_disabled_060), but that path bypasses volamos's owntake_hardware_exception(and its "vector handler == 0, decline cleanly" safety check) entirely -- unlike the 68020/68030/68040 FPU-absence paths, which are host-interceptable viaStepResult::FlineTrap. There is also no separateCpuTypeto substitute to for the 68060 case even if there were a reason to. That's a real routing/architecture gap (possibly worth reporting upstream against them68kcrate too), separate from this PR's scope.Test plan
attn_flags_68040_always_sets_fpu40_regardless_of_the_fpu_arg,attn_flags_68060_always_sets_fpu40_regardless_of_the_fpu_arg,cpu_68040_with_explicit_no_fpu_substitutes_68ec040,cpu_68040_with_explicit_fpu_is_accepted_as_a_no_op,cpu_68040_with_neither_fpu_flag_is_accepted,cpu_68ec040_with_no_fpu_is_accepted_without_substitution,cpu_68060_with_no_fpu_is_not_substituted,resolve_cpu_type_for_fpu_is_a_no_op_for_every_other_combination.fixtures/hello:--cpu 68040 --no-fpunow prints the note and runs successfully as 68ec040;--cpu 68040 --fpu, plain--cpu 68040, and--cpu 68ec040 --no-fpuall still run cleanly with no note.cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test --workspace(1109 tests) all clean.mkdocs build --strictclean.🤖 Generated with Claude Code
https://claude.ai/code/session_01Godbk9UDW5obqYsdkXkyRR