search

MEGA65 / mega65-libc

Simple C library for the MEGA65
Other
25 stars 20 forks source link

Remove invalid asm clobbers (fixes compilation errors with llvm-mos main branch) #55

Closed zeldin closed 4 months ago

zeldin commented 4 months ago

An asm clobber is not allowed to specify a register used as an input or output. In the case of the debug asms which use "a" for input, the accumulator is never actually clobbered by the asm body, so just remove it from the clobber list. In the case of fc_nyblswap, the accumulator is used as both input and output, so declare it as such instead of as a clobber.

Checklist

Thanks a lot for your contribution! Please tick off the following:

mlund commented 4 months ago

Thanks!

mlund commented 4 months ago

@zeldin would you mind taking a look at #53 which is a duplicate that I completely forgot about. I removed a but added v for reasons I don't recall.

zeldin commented 4 months ago

Right. It is true that the debug functions clobber v (or more generally p). The fc_nyblswap function also clobbers c, as you correctly declare. I kind of wonder if this is necessary though -- wouldn't you expect flags to always be clobbered? Otherwise shouldn't z and n also be declared as clobbered (by almost any code)?

Anyway, I can try it out later tonight.

zeldin commented 4 months ago

So, this is what I found out so far: mos-clang does not recognize z, n, d or i in the clobber list. But it does recognize c and v as well as p and the generic cc (which I assume is equivalent to p). As for d and i, they are probably out of scope (meaning that the compiler does not track their contents), but n and z are actually treated as bits of a special two-bit nz register, which itself is a subregister of p. However, nz is also not recognized in the clobber list. I think a safe approach would be to declare cc clobber in any asm that touches any flags at all. I've seen people claim that a cc clobber does nothing on x86 because the flags are implicitly treated as clobbered anyway. It would make sense if this was also the case for 6502, but I have found no documentation to that effect. At any rate cc is always supposed to be valid in the clobber list, whether needed or not.