RPGHacker / asar

(Now) official repository of the SNES assembler Asar, originally created by Alcaro
Other
199 stars 42 forks source link

FEATURE REQUEST: Option to lock freespace to an even address #143

Open VitorVilela7 opened 5 years ago

VitorVilela7 commented 5 years ago

Recent researchment on the SA-1 chip has shown that some of its features requires to the input address to be an even address, in particular the DMA and Variable Bit Length Reader.

This is because the ROM chip data bus on SA-1 ROMs is 16-bit wide and some special treatment should be done for reading odd values. While the SA-1 chip take cares for that when handling opcodes (it adds one cycle penalty if the jump destination is an odd address), the DMA and VBR circuits completely ignores the odd address and this can lead to potential undefined behavior or data corruption. In essence, these circuits discards the very last bit of the ROM address (address &= ~1).

While a user could add an assert on the label for checking if it's even or odd and move it around or fill with NOPs to align properly the label, that can't be controlled on freespace assignment and depending if the freespace is odd or even it could misalign everything on end-user's code.

I propose adding "freespace/freecode/freedata even" or "freespace/freecode/freedata odd" for specifying if the freespace should stay in an even or odd address.

Consider it important, probably all SA-1 dynamic sprites could be breaking on real hardware right now because it uses SA-1 DMA from ROM to BW-RAM.

RPGHacker commented 5 years ago

Sounds reasonable. Though instead of adding variations of freespace, my proposed solution would be to just change freespace behavior for SA-1 in general and make it always return aligned memory. I can't think of a situation where one would actually want an unaligned address, and if a situation like that exists, you could probably just use a NOP or a skip to get there.

VitorVilela7 commented 5 years ago

I personally dunno. Is it too complicate to add another option? Making it default for all SA-1 ROMs could generate a lot of freespace fragmentation if Asar sequentially write multiple freespace blocks with an odd size and always having to skip one byte between each patch.

Not every patch will mess with SA-1 registers, hence I believe it's better for having an option pretty much like freedata.

Alcaro commented 5 years ago

I'd prefer aligning by default, to avoid having to update every single sprite. The RATS tag wastes eight bytes already, a ninth (half of the time) is no big deal.

e: I wouldn't mind explicit options to choose odd, even or either alignment. I just wouldn't recommend it as a default.

VitorVilela7 commented 5 years ago

Making it aligned by default would be a reasonable option if we were aware that all SMW sprites that uses SA-1 DMA had their labels aligned by default, but we don't know... That will require someone taking a look on them first. But it's likely to be 50% chance for all of them.

VitorVilela7 commented 5 years ago

But then doing it by default is interesting since we won't need to update the Sprite Tools... I'm convinced now.

RPGHacker commented 5 years ago

I personally dunno. Is it too complicate to add another option?

Not necessarily complicated, but the way commands in Asar are implemented is pretty quick and dirty, they're pretty much just a gigantic if/else chain, and Asar already has quite a lot of commands with optional arguments and stuff. This is pretty messy in the code, and of course every added command is bound to make it messier, so when new commands are suggested, I always first try to make sure whether adding that command is the best solution or whether there might be another solution that is just as good or even better.

Making it default for all SA-1 ROMs could generate a lot of freespace fragmentation if Asar sequentially write multiple freespace blocks with an odd size and always having to skip one byte between each patch.

Well, as Alcaro already started, it's unlikely to really add up to a significant amount. I mean, how many hacks are there that use more than a couple of patches/sprites? Very few, I'd assume. Even if a hack used, let's say, a hundred of them, that would still at most be a hundred bytes wasted, which is unlikely to matter in the grand scheme of things. In fact, on average, it would probably be only half of that, since every freespace address technically already has a 50% chance of being properly aligned.

Making it aligned by default would be a reasonable option if we were aware that all SMW sprites that uses SA-1 DMA had their labels aligned by default, but we don't know...

I'm not quite sure if I understand what you mean by that. This alignment only really matters when using freespace commands, right? And there previously wasn't any way to assure this alignment when using freespace, correct? This means that if any sprite already exists that uses freespace and SA-1 DMA, it is likely already broken in its current form, and if sprites exist that use SA-1 DMA without using freespace, they shouldn't be affected by any changes made to the freespace command in the first place. Am I misunderstanding?

VitorVilela7 commented 5 years ago

I'm not quite sure if I understand what you mean by that. This alignment only really matters when using >freespace commands, right? And there previously wasn't any way to assure this alignment when using >freespace, correct? This means that if any sprite already exists that uses freespace and SA-1 DMA, it is >likely already broken in its current form, and if sprites exist that use SA-1 DMA without using freespace, >they shouldn't be affected by any changes made to the freespace command in the first place. Am I >misunderstanding?

This is correct. Without making sure the freespace is aligned, you can't really make sure that a certain label that pointers to sprite graphics, for example, will be aligned or not because its position is relative to the whole memory map and not to the freespace address.

Alcaro commented 5 years ago

since we won't need to update the Sprite Tools

oh right, sprites don't set their own freespace, the tool does it. Updating tool is way easier than updating all sprites, though still nonzero (and getting people to upgrade is tough no matter what you do).

But either way, I'd recommend analysing all sprites; it wouldn't surprise me if one or a few have an odd-sized code block before a DMAed incbin.

every freespace address technically already has a 50% chance of being properly aligned

Actually, slightly more than 50%. Anything starting on a new bank never needs padding.

And uncompressed graphics are even-sized, so two consecutive uncompressed graphics never pads the second. If sa1rom likes to segregate code and data (like lorom), two consecutive graphics is quite likely.

But that's a pointless overanalysis. 50% is good enough for all practical purposes.

KungFuFurby commented 4 years ago

Maybe related to #138? I recall requesting an align feature that may be at least partially resolved by what I was specifying over there. It could even be combined with what you specified: in my case, the align 2 could work on ending on bytes always divisible by 2 (and effectively acts the same as an even). You have a case where the align command does not cover, where it is on an odd byte (or perhaps at an offset after aligning).

randomdude999 commented 4 years ago

so i've implemented #138 and i think it's sufficient for all use cases mentioned here - you can just put a fill align 2 after the freespace command. I actually think this solves the issue better than just adding an option for even-aligned freespaces, since you can add the align command before each routine or data block that needs to be aligned properly.