Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OP Code Constants, Freeze Warning, Branch Note. #129

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nacnud-sco
Copy link

Just messing with your code base a wee bit.

  1. Added constants for OP codes
  2. Used those OP code constants
  3. Create a wee function that shows if we took a "Branch"
  4. Added a warning about "Freeze if inactive" - as that's a real gotcha.

All just scratching my itch. Nothing too serious. I think that I'd re-write the whole GUI in a way that is not WIN32 madness. :D

I'm minded to also try and put some kind of debug server into it and it'd be nice if the "Debug Show" of instructions would work out if it "would branch" to save me just showing the results if it actually did.

@chrisn
Copy link
Collaborator

chrisn commented May 1, 2024

I like the idea of improving the debugger, but less keen on the constants. I wonder if the Freeze when inactive option is still useful, maybe could be removed altogether.

@nacnud-sco
Copy link
Author

I like the idea of improving the debugger, but less keen on the constants. I wonder if the Freeze when inactive option is still useful, maybe could be removed altogether.

Well, I know you've been around long enough that I don't have to 101 you on the need to not use magic numbers in code to make it more self documenting. So, I'll conclude, your code base, your ways :)

However, I do find the code much more readable if one doesn't know all the OP code names to hex values. I'll keep it in my personal stash.

The debugger is, for sure, a place that I'd like to see improvements too. I think I'd really see that as kind of a client server model. So that we could put a modern style GUI over it. Sure, we could hook it up to a GDB server, and I have toyed with that idea in the past. Likely that's a sensible way to go as it gives us access to all sort of GUI options.

However, in my mind just now, I'd be really interested in a memory access "heat map". I'd also like to see a real time ~250MB in memory execution buffer. Maybe we can even reverse it if needed.

There's lots that could be done.

I do like your idea of getting rid of the "freeze" business, as it seems kind of pointless, especially if you're in the debugger and in a paused state.

@chrisn
Copy link
Collaborator

chrisn commented May 1, 2024

Rather than remove it, in case someone needs it, for now I've changed BeebEm so it automatically switches Freeze when inactive off when you open the debugger.

@nacnud-sco
Copy link
Author

Rather than remove it, in case someone needs it, for now I've changed BeebEm so it automatically switches Freeze when inactive off when you open the debugger.

Yep. That was going to get my vote. I will look at how that affects the debugger and the "seeming constant message pump" to that debug window. I just get slightly confused by the "show this disasm" of the "current" instruction, but only execute it when "next" is called.

I mean, I get it, but I think it'd be nice if the side effects of the OP were known before hand. Like, would the branch actually take place? I guess that could be dependent on memory state and if someone, while the debugger was paused, changed the memory location that a compare was done against, then the "would jump" or "would set carry" or whatever, could change, but I think it'd still be nice to know.

Or show the "destination if it was taken" and "if it's not taken". Or some kind of easy context.

There's loads that could be done. I do remember looking at creating a GDB server, and and dumping that into the code. I may look at that again. Just for the fun of it.

Thanks for changing the freeze thing, that's a good quality of life improvement. And I can put that round the warning I typed now too...."if freeze if active, then warn". Kind of thing. Though it is almost a non-issue now, as I can't imagine anyone turning the feature on.

When is it supposed to be used?

@chrisn
Copy link
Collaborator

chrisn commented May 4, 2024

My guess is it's mostly legacy, from a time when PCs were much less powerful and so you might not want to have an app consuming CPU while in the background.

@chrisn
Copy link
Collaborator

chrisn commented May 4, 2024

I just get slightly confused by the "show this disasm" of the "current" instruction, but only execute it when "next" is called.

I know what you mean, but it makes sense to me. The debugger shows the current CPU state and the instruction that will be executed when you do "next". I was thinking about your branch flag feature, and might it be more useful to indicate "will branch" rather than "did branch"?

@nacnud-sco
Copy link
Author

I just get slightly confused by the "show this disasm" of the "current" instruction, but only execute it when "next" is called.

I know what you mean, but it makes sense to me. The debugger shows the current CPU state and the instruction that will be executed when you do "next". I was thinking about your branch flag feature, and might it be more useful to indicate "will branch" rather than "did branch"?

I agree with you that a "would do" is better than "did do" indication. That would mean "executing" the instruction though. Or, if it's a branch thing or a compare thing, just doing that op and having some kind of indication. It is backwards as to how the code works just now.

I'll see what comes out of this gdb investigation.

@chrisn
Copy link
Collaborator

chrisn commented May 6, 2024

The branch instructions use the state of the CPU flags, so for example if the next instruction is BEQ and the current CPU state has the Z flag set, you know the branch will be taken.

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.

2 participants