Bug 193 - .equr re-engineering
Summary: .equr re-engineering
Status: RESOLVED FIXED
Alias: None
Product: RMAC
Classification: Unclassified
Component: Core (show other bugs)
Version: unspecified
Hardware: All All
: Normal enhancement
Assignee: Shamus Hammons
URL:
Depends on:
Blocks:
 
Reported: 2022-03-07 13:46 CST by ggn
Modified: 2022-05-30 14:37 CDT (History)
2 users (show)

See Also:


Attachments
The multi-patches! (40.81 KB, application/octet-stream)
2022-03-07 13:46 CST, ggn
Details
Fix for disabled code (926 bytes, patch)
2022-04-17 01:58 CDT, ggn
Details

Note You need to log in before you can comment on or make changes to this bug.
Description ggn 2022-03-07 13:46:50 CST
Created attachment 167 [details]
The multi-patches!

Continuing a bit from #148:

Since people have requested for .equr to work everywhere and since I personally thought the way .equr is implemented was too hacky, here's a less hacky version!

Things of notice:

- Should work on all architectures (well, obviously!)
- Instead of relying on expr() to apply the .equr during parse time (and only for riscasm.c), the registers are replaced during tokenising. This allows for less testing during the token stream processing. Also, no need to add any additional code in any other architecture (yay!)
- Directives .regbank0/1 are now deprecated. As well as all the BANK_0/1 attributes and checks. In any case the checks were incomplete (for example, one could .equreg any register in any bank during the fixup phase), so they more gave people a false sense of security than anything else
- As a consequence, expr() became slightly less dirty
- WARNING WARNING WARNING: some regression tests WILL fail! This is expected as the symbol names for the .equreg defines have changed in value and we actually export those (the question here of course is: why? :).
- In any case, please double check that only the .equr symbols change, I might have broken something else!

Patch attached. It should be a 7-parter, as I didn't want to do this in a single commit. Let me know if something went wrong (I literally copy/pasted some commands from loloverflow!)
Comment 1 James Jones 2022-04-17 00:35:21 CDT
This breaks compilation of the Atari 3D engine code somehow, a copy of which is in my SDK here:

https://github.com/cubanismo/jaguar-sdk/tree/master/jaguar/3d

Error:

$ rmac -fb -rd  wfrend.s
loadxpt.inc 28: Error: cannot use reserved keyword as label name or .equ
loadxpt.inc 29: Error: cannot use reserved keyword as label name or .equ

To get that far you have to also apply the fix from issue #159 on top of this series, but I verified it is this  series causing the problem by applying my previous hacky fix on top of this series (which compiles the 3D demo fine on its own) and I got the same failure.
Comment 2 ggn 2022-04-17 00:54:25 CDT
Hi there, thanks for the report.

This is actually very awkward as those statements are inside an ".if 0" block, which means that they shouldn't be processed in the first place. When those lines are commented out, the source seems to assemble fine.

So I'll take a look to see why both things happen (the ".if 0" and those specific symbol names being rejected) and will report back soon.
Comment 3 ggn 2022-04-17 01:58:39 CDT
Created attachment 172 [details]
Fix for disabled code

Ok, that was an interesting edge case.

The tokeniser was trying to be a bit too clever by replacing an already equr'd symbol with a register inside a disabled code block, which then the line parser rejected as an invalid line. We probably shouldn't tokenise anything inside an '.if 0' block except scan for an '.endif' but that's for another issue as this one has done quite a few changes so far.

Bottom line: try the new attached patch. It should fix your reported problem.
Comment 4 James Jones 2022-04-17 23:50:50 CDT
Yeah, I saw the failure, looked up where it was, and just thought "Yup, I'm not touching that." and gave up immediately. Interesting case indeed.

I verified the fix works. Thanks!
Comment 5 James Jones 2022-04-17 23:55:48 CDT
Suggestion I just ran into: You probably want to add 56kregs.h, 6502regs.h, riscregs.h, and unarytab.h to .gitignore, and if existing generated files were removed, remove them there as well. Nothing worse than a perpetually dirty tree.
Comment 6 James Jones 2022-04-17 23:56:58 CDT
Argh, maybe that suggestion applies to issue #159 instead. Sorry for the noise if so.
Comment 7 Shamus Hammons 2022-05-30 14:37:31 CDT
Well, the patches applied cleanly and the regression suite passed, so I'm happy with it.  :-)