Arakula / A09

A09 6800/6801/6809/6309/68HC11 Assembler
GNU General Public License v2.0
40 stars 8 forks source link

Unary ! and ~ operators #9

Closed raybellis closed 4 years ago

raybellis commented 4 years ago

Hi Hermann - sorry, I found another couple of issues, both relating to the unary NOT operator.

Firstly, ! is acting as a not equal to zero operator, instead of a bitwise negation. This is a regression from the documented RELASMB behaviour. If given an expression that evaluate to $00 it emits $01. Any non-zero expression evaluates to $00.

The ~ operator (which RELASMB doesn't have) does act as a bitwise negation, but isn't recognised as valid in instructions such as OIM #~$FF,$40. (Curiously the ! operator is accepted in that instruction, albeit still with the incorrect non-bitwise behaviour).

The fix for the first issue is trivial (i.e. making the case '!': and case '~': blocks in scanfactor both use the ~ code) but I can't figure out how to fix the second. In fact making that change prevents the ! operator from working in OIM too.

Here's a test case. The actual use case is being able to write code like:

OIM # bit_mask, IO_register    ; set register bit
AIM #!bit_mask, IO_register    ; reset register bit
Arakula commented 4 years ago

Mhm. The ASMB documentation is a bit misleading, as it says ! is a "Logical NOT operator". The paragraph below that, however, suggests that this "logical not" is performed for each bit separately (which somebody with 40 years of C/C++ practice would label a binary NOT operator). I'll look into it.

raybellis commented 4 years ago

Yup, I'd call that a "bitwise NOT" myself, but I do think the intent in the RELAMB manual is clear. The various 6x0x documentation talks about "logical AND", "logical OR", etc, which does muddy the waters a bit.

Arakula commented 4 years ago

OK ... at least I know where the expression error in your example happens:

void bitdirect(int co)
{
struct relocrecord p = {0};
unsigned short dir;

skipspace();
if (*srcptr++ != '#')
  error |= ERR_EXPR;
if (!(dwOptions & OPTION_TSC))
  skipspace();
dir = (unsigned short)scanexpr(0, &p);   <--- # ~$00 returns -1
if (dir & 0xff00)                        <-- so this here cries "error!"
  error |= ERR_EXPR;
...

... I'm just not sure how to handle that. Presumably it would be sufficient to change the instruction to

dir = (unsigned char)scanexpr(0, &p);

and be done with it. If only I could remember why I implemented it this way (sigh)...

Arakula commented 4 years ago

The ! and ~ syntax has already been in Lennart Benschop's original A09, by the way. Blame him 8-)

raybellis commented 4 years ago

It looks like you were just trying to range check the left hand operand, but didn't take "negative" 8 bit values into account. I think (as you propose) that just truncating it to 8 bits should suffice. If I simply remove the range check (combined with the previous ~ / ! equivalence change) then my project (and the test cases) produce the expected result.

Arakula commented 4 years ago

What would you think is better? Simply remove the range check (less code, but unintentional overflow will go unnoticed), or change it to your suggestion (overflow flagged as error)?

Arakula commented 4 years ago

Anyway, if I leave it in, it would have to be if (dir > 0xff && dir < 0xff00) since ~$FF, for example, produces 0xff00 - that would be an error with your "check for signed negatives" above.

raybellis commented 4 years ago

Yes, that's the conclusion I had also just reached - my version currently reads:

if (dir >= 0x0100 && dir < 0xff00)

I had already deleted the incorrect "check for signed negatives" a while back when I realised it was wrong :p

raybellis commented 4 years ago

The ! and ~ syntax has already been in Lennart Benschop's original A09, by the way. Blame him 8-)

Weird - I can't think of any valid use case for a "C" style ! operator in assembler.

Arakula commented 4 years ago

I'll leave the check in, just with a comment ... next version will go up in a minute or so. It contains some changes in the macro / TEXT area, so you might have to redo some macros if you got any that work with quoted text (i.e., text with ' or " around it) as parameter or TEXTs.

Arakula commented 4 years ago

Weird - I can't think of any valid use case for a "C" style ! operator in assembler.

Maybe not in the code directly, but in constructs like IF !&H63 it can make perfect sense. (Although, on second thought, ~ would do the same here ...).

raybellis commented 4 years ago

I see you've left !val having the same "logical" rather than "bitwise" behaviour? Notwithstanding this seems to be the original A09 behaviour it's still a diversion from RELASMB's documentation.

raybellis commented 4 years ago

It rather depends whether setting an OPT sets its value to $FF (such that ~OPT and !OPT resolve to the same $00 value) or to $01 (where ~OPT == $FE and !OPT = $00)

It looks to me like they are set to "1" per setoptiontexts() ? Changing that would break anything that does IFC &H63,1 :(

Arakula commented 4 years ago

I see you've left !val having the same "logical" rather than "bitwise" behaviour? Notwithstanding this seems to be the original A09 behaviour it's still a diversion from RELASMB's documentation.

Yes. I'm thinking about it.

It's funny ... I checked some others. The Motorola Freeware Assembler (as9) can't do binary negation at all. AS uses ~. The GNU Assembler uses ~ (OK, no wonder here, it's leaning towards C syntax anyway).

I might make the behaviour TSC-specific - like, "If OPT TSC is set, use ! like ~". But that might easily lead to quite a lot of "WTF?" situations.

So ... right now, I'm asking myself whether it wouldn't be enough to document the deviation.

raybellis commented 4 years ago

So ... right now, I'm asking myself whether it wouldn't be enough to document the deviation

That would be fine with me. I'm never going to use as9 or relasmb.

Arakula commented 4 years ago

Fine with me, too. I've uploaded a new version that documents the ! and ~ in the "Things that are and aren't" section ... and the overflow check generates a specific warning now instead of an error. That's better, I think.

Arakula commented 4 years ago

I consider this issue closed, then; feel free to reopen it if you have a different view.