colin-broderick / emu

0 stars 1 forks source link

Abstraction of addressing modes #20

Closed colin-broderick closed 3 years ago

colin-broderick commented 3 years ago

@Mark-Platts

We want to implement functions to abstract the behaviour of the different addressing modes wherever possible.

I think this is worth tackling before we implement too many more instructions, as it will simplify that task and reduce the amount of back work we have to do (converting all the old instructions).

Shall we split them up and try half each?

Mark-Platts commented 3 years ago

I think it's a good time to do this now that we have both had a chance to understand each address mode. It will make a lot of the future ops a lot quicker.

I was planning to start with the two indirects this evening if that sounds ok with you. I was going to do the indexed absolutes next. But feel free to take them if you fancied them. FYI I was planning to just do one indexed absolute that takes the initial address and then an augmenter as arguments. That way one function can be used for both absolute x and absolute y.

colin-broderick commented 3 years ago

Ok, I'll select a couple and have a crack some time this week. I'm currently compiling a list of cycle counts in a second sheet on the opcodes spreadsheet. Might be illuminating and make patterns more apparent.

colin-broderick commented 3 years ago

In case you don't know, a little check in the top right corner of a cell in Google Sheets indicates there's a comment on that cell. You can hover over to read the comments and add your own.

Right click a cell to add a comment.

colin-broderick commented 3 years ago

Looks like there are some easy ones and some that are a bit trickier but do seem to follow a pattern as far as cycle count goes, if we can discover it.

The implied address mode is all over the place so probably not worth trying to abstract that. Logically it could be seen as the lack of an addressing mode, so maybe that makes sense.

For some others, it could be sensible to declare a base number of cycles, have the abstraction method spend that, and spend extras manually where required. For example, zpg mode always uses three or five cycles, so three would be the base count and the extra two can be spent manually for instructions that need it.

Another option of course is to not bother managing cycle count directly in the function, and just always spend them manually.

STA appears to be special somehow. In a few modes it goes against the grain where everything else is consistent.

Let me know what you think. Certainly the easiest way is to spend all cycle counts manually. This is a finite problem so we don't really need to generalise too much.

Mark-Platts commented 3 years ago

Personally, I think the cycle counts shouldn't be in these functions. In my mind we have the actual op function encasing everything, and then inside we have a function that grabs the needed data/operand, and a function that checks page boundary crossing. It seems like it would be asking too much of those functions for them to also have to worry about cycle count. So I think manual is the way to go.

That said, I think there is a pattern that would mean the cycle counts stuff could be potentially cleaned up a little. I'm going to spend some time trying to understand it better.

On a cursory look, it seems that STA has maximal cycle counts, like it always assumes there is a page boundary cross. I wonder if the pattern has something to do with whether the CPU thinks it can get away with only using bytes instead of the full word. Because STA puts something in memory, it kind of has to always look at the full word, so there's no way to save a cycle by dropping the high byte of the address. Possibly :P

Mark-Platts commented 3 years ago

On a different note: If we do manual cycle counting, I think it would be best to abstract away the duplicates. So instead of sem.wait() sem.wait() sem.wait()

We'd have something like do_sem_wait(3) or the like.

colin-broderick commented 3 years ago

I agree. I thought it would be good to have the functions do cycle counts as well, but you're right, it's asking too much. If the patterns were simpler or even constant I think it would be fine, but there are far too many exceptions and special cases for it to be reasonable.

I don't know about a function for checking page boundary crossing though. It's always a change to some address value, so you have to store the original anyway, and then all you have to do is

if (page1 != page2)
{
    ...
}

Probably the best you could do would be

if (crosses_page_boundary(page1, page2))
{
   ...
}

which doesn't improve anything. Am I missing something?

colin-broderick commented 3 years ago

I would agree about sem.wait(3) but it's non-trivial and given we're throwing away the semaphore anyway probably not worth doing. We can think about doing something in the same spirit when we do the new timing, depending how we do that.

Mark-Platts commented 3 years ago

I think you're correct on both points there. I was thinking something more like check_boundary_cross_indirect_indexed(address, augment) which then checks for the specific address mode. Since for some 'target_address' will now be hidden away in the get_data function. I'm still not sure if it would save much hassle though.

Some good it could do is if it returned 1/0 instead of a bool and we could do sem_wait(3 + check_boundary_cross_indirect_indexed(address, augment)) which would take care of the whole cycle count logic in one line. That all depends on how we decide to do timing though.

Mark-Platts commented 3 years ago

I'd like your opinion: Should the get_data functions contain IP++'s, or should they only read the data?

colin-broderick commented 3 years ago

I would only include it if the address mode affects IP in a consistent way. I think they probably will. They will all increase IP by zero, one, or two, except for the instructions which affect IP directly but they will presumably always come after the get so that shouldn't matter.

There are details though. Like immediate addressing will sometimes fetch one byte and sometimes two, so that will need to be considered.

As a guiding principle, if you find yourself adding lots of exceptions or special cases in a function it probably needs further abstraction or not to be a function it all.

Mark-Platts commented 3 years ago

I've learned a little about the circuitry related to the +1 for page boundary crossing.

Apparently, whenever the CPU adds to an address, it will try to do it with only the low byte first and will look for a carry flag, which would indicate that the address went to a new page number. If carry, the CPU will then devote an extra cycle to also doing an addition to the high byte of the address. Since it's an 8-bit CPU it needs 2 cycles to add words, so this saves cycles.

For STA, it doesn't do this on purpose, to guarantee it always get the correct address.

With this in mind, it might be worth having a add_cycle_if_necessary() function that checks for crossing and then internally adds the cycle. Then the whole if (crosses_page_boundary(page1, page2)) { ... } Would be contained in a function.

colin-broderick commented 3 years ago

That's cool. I had read that it was to do without having to do two sums due to carry but hadn't read all that detail.

We can add that function if you like. I don't feel strongly either way. If I really had to choose I'd probably choose the if (page1 != page2) version because that is semantically very meaningful, and it's immediately obvious why the cycle is added. But it's really a 51/49 split for me so honestly go for the function if you prefer it :).

colin-broderick commented 3 years ago

Actually one thing to think about is the fact that "if necessary" might mean different things for different ops. For example, on the branching ops, cycles are added if

which is different to the other ops. So you'd need two or three separate functions or a pretty clever single function.

Mark-Platts commented 3 years ago

You are correct, I was just using that function name as a quick example.

I'm also not too fussed whether or not we hide things with these functions. I just thought it might make the code cleaner. But for now shall we just carry on as we have been? If we find a better reason later we can consider things again.

colin-broderick commented 3 years ago

Closing this now as I think we're happy with what we have. The only thing we might still want to implement is putting the IP++ into the get methods, but I consider it pretty low priority; now that all the instructions are implemented it's not going to save us any work.