Bug 185 - Set all optimisation flags to off by default, and other enhcanements
Summary: Set all optimisation flags to off by default, and other enhcanements
Status: RESOLVED FIXED
Alias: None
Product: RMAC
Classification: Unclassified
Component: Core (show other bugs)
Version: unspecified
Hardware: PC All
: Normal enhancement
Assignee: Shamus Hammons
URL:
Depends on:
Blocks:
 
Reported: 2021-07-15 11:05 CDT by ggn
Modified: 2022-05-30 13:18 CDT (History)
2 users (show)

See Also:


Attachments
Patch (932 bytes, patch)
2021-07-15 11:08 CDT, ggn
Details
The patches! (extra!) (16.05 KB, patch)
2021-07-16 07:11 CDT, ggn
Details
Updating doc (995 bytes, patch)
2022-04-17 02:31 CDT, ggn
Details
More doc changes reflecting all optimzations disabled by default. (925 bytes, text/plain)
2022-04-17 02:47 CDT, James Jones
Details
Small fix and doc synchronisation (6.58 KB, patch)
2022-04-17 03:45 CDT, ggn
Details

Note You need to log in before you can comment on or make changes to this bug.
Description ggn 2021-07-15 11:05:19 CDT
This keeps tripping many people up (including myself!).

At least ST programmers do want to presume automatic optimisations are off by default because many times they write cycle exact code. And especially beginners do not exactly read the manual to know this, or the -s flag, which usually ends up having to ask me many questions and me having to repeat the same answers again :).

So really, let's get rid of O0, O1 and O2 on by default, shall we? :)
Comment 1 ggn 2021-07-15 11:08:52 CDT
Created attachment 160 [details]
Patch
Comment 2 ggn 2021-07-15 11:34:58 CDT
As a footnote... the regression tester will need "+o0 +o1 +o2" in a few places as a consequence to this patch, but I'm sure that's the last we'll hear from it :)
Comment 3 ggn 2021-07-16 07:11:58 CDT
Created attachment 162 [details]
The patches! (extra!)

So as I was reviewing the code for optimisations in general I did a few things more:

1. Added a couple of missing 56001 switches that were on by default. Keeping with the ticket's philosophy I made them all off by default, so again the regression tester will complain that you need to turn them on. I think this isn't too disruptive as I doubt people are using the 56k mode yet!
2. I added a prefix to all optimisations so now it is clear which flag caused the optimisation to happen. This way people can turn them on and off much easier.
3. Since flag "Op", or o11 should always be after all the flags that are affected by +Oall and ~Oall (because I doubt people really want to turn this on unless they really need it), and because of (1), I moved o11 to o30, so that will give us some leeway for adding more switches in the future. Hopefully not too bad either :).

Hopefully you agree with all these changes - I did jump the gun a bit and went and did them without discussing them with anyone!
Comment 4 Shamus Hammons 2021-08-20 10:59:16 CDT
Thanks for the patch!  :-)
Comment 5 James Jones 2022-04-17 01:46:18 CDT
FYI, this tripped me up. I updated rmac and my size-optimized (though not fully apparently) binary got about 32 bytes larger because these optimizations were gone. Not a big deal, I'll just turn them back on, but I wanted to point out the manual is now incorrect:

http://rmac.is-slick.com/manual/rmac.html#optimizations-and-translations

"Optimisation switches 0, 1 and 2 are turned on by default for legacy reasons. All other levels are off by default. (refer to section The Command Line for a description of all the switches)."

I assume "legacy reasons" means to match MADMAC behavior?
Comment 6 ggn 2022-04-17 02:31:30 CDT
Created attachment 173 [details]
Updating doc

First of all, sorry for the confusion.

I tried to remember to update everything. I changed the command prompt in rmac itself, I made an announcement on the website, but unfortunately I totally forgot to update the doc. Sorry. Sometimes it all just gets too much to remember :/ (add to that the actual stress of making and testing the release binaries in various OSes and you've got yourself a major headache in your hands).

> "I assume "legacy reasons" means to match MADMAC behavior?"

Yes, indeed. This was the default behaviour ever since the beginning of MadMac, I think. But recently I've had way too many questions from ST users that adopted rmac. Many of them want to produce cycle exact code that 100% matches what they type. So me and Shamus agreed that an optimising assembler by default is a "not that great of a thing" (TM) and turned the optimisations off.

Shamus: I've re-opened the ticket and added a patch to update the docs. Feel free to apply.
Comment 7 James Jones 2022-04-17 02:47:42 CDT
Created attachment 174 [details]
More doc changes reflecting all optimzations disabled by default.

I think you need this patch as well. Additionally, just below this hunk it states:

"In DSP56001 mode size optimisations are on by default. Currently there is no way to disable this behaviour."

I don't know if that is accurate or not. I haven't messed with DSP56001 mode.

The docs on http://rmac.is-slick.com/manual/rmac.html also appear out of sync with what is in git somehow. I see these on the website for possible +/~o values:

10: 56001 Use short format for immediate values if possible
11: 56001 Auto convert short addressing mode to long (default: on)

But not in rmac.rst. Not clear how/if that interacts with the statement noted above either, but they look suspiciously related.

Need to get some sleep now, but I'll try to test your other patches tomorrow. Happy Easter to you as well now that the clock's rolled over here too.
Comment 8 ggn 2022-04-17 03:45:02 CDT
Created attachment 175 [details]
Small fix and doc synchronisation

What you wrote is mostly true and thanks so much for peer reviewing!

This triggered me to re-evaluate all the changes and compare to the doc, I discovered one small issue and fixed it.

In general: the website's manual is correct about switches. o11 has to be on by default to keep compatibility with Mot's 56k assembler. I believe this new patch reflects the truth in the docs, so I've obsoleted yours.

Hopefully we'll get it right this time!
Comment 9 James Jones 2022-04-17 23:40:20 CDT
Sorry, too tired last night. What I proposed removed too much obviously. Your latest version LGTM. Thanks!
Comment 10 Shamus Hammons 2022-05-30 13:18:34 CDT
Changes have been added; thanks for the patch!  :-)