Adding Oric support to the Oscar64 C compiler...

Since we do not have native C compilers on the Oric, this forum will be mostly be used by people using CC65 or the OSDK. But any general C related post will be welcome !
User avatar
ibisum
Wing Commander
Posts: 1912
Joined: Fri Apr 03, 2009 8:56 am
Location: Vienna, Austria
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by ibisum »

There are many "standards", gnu just happens to be one of them, you use ".indent" while most of the people I know instead of "editor config" files.
I was only trying to use gnu-indent in order to match drmortalwombats's style, but it didn't work out.

I always try to match the style of any project - but it turns out that it wasn't that simple to get gnu-indent configured for his preferences - and, it has to be said, his style is not consistent throughout the project, anyway, so it wouldn't have worked.
if some modifications are required to support other operating systems or tool chains, that should be a separate commit by itself, something separate from the actual features.
Well, if done properly, adding another systems' support files shouldn't impact anything that was pre-existing. This is the case now with my oric-support branch as it stands right now - and drmortalwombat has already merged my CMake changes, since it immediately results in a working MacOS/Linux build without much fuss.

But this is moot, since oric-support will be rebase'd before I do the PR, and hopefully there won't be any further style issues.
So looking at your link I can immediately see things that would be rejected as a review where I work because they are changes that were not necessary:
- The line with "target_link_libraries(${PROJECT_NAME}" has a modified carriage return or white space at the end
Oh, what a boring, pedantic whine. This isn't a branch that is submitted as a PR yet, so stop evaluating it as if it were.

This is work in progress, so please don't bother commenting on such administrivia until there is actually a PR ready.
- The "if (mTargetMachine == TMACH_ATARI)" is now a "else if" and Oric has become the primary if instead
Oh my god, did you even read the code?
Regarding the actual change, I think the WriteTapFile should probably be renamed to something the clearly indicate it's for oric, because in the same way as the "DSK" format is not only used on Oric, so is the .TAP extension (or it could have a parameter to say it's an actual "oric tape").
Well, this follows the existing naming conventions, which also uses .PRG (used on multiple platforms as well), and .. this is one of a list of things I'll ask drmortalwombat what he thinks about refactoring anyway, when there is actually some valuable Oric support.

(The project doesn't use if {}'s properly, fwiw, and there are a few other code smells that would be nice to have fixed after Oric support is added...)
Regarding fopen_s, I never used it, but as far as I can see it returns an errno, are you sure that file would be set to zero if the open fails?
Why don't you code-review the rest of the project and see if drmortalwombat wants your PR with fixes to his use of fopen_s.

This is a first pass to get Oric support. There are a hundred things I'd fix about drmortalwombats code (WriteTapFile is based on WritePrgFile). The point is to maintain consistency with the existing code - once Oric support is basically working, and drmortalwombat accepts the PR for Oric support, I'll think about making some other suggestions to his code base. His use of fopen_s is a different issue than Oric support ..

Anyway, back to adding Oric CRT and conio functions ..
User avatar
ibisum
Wing Commander
Posts: 1912
Joined: Fri Apr 03, 2009 8:56 am
Location: Vienna, Austria
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by ibisum »

Okay, conio functions added.

Getting a Segmentation fault, but verified the Oric paths are taken. Will dig into the Segmentation fault tomorrow - its related to the calculation of the sections derived from BC_REG_ values, which are probably still not quite right, but its something for tomorrows Oric hacking session ..

Code diff here:

https://github.com/drmortalwombat/oscar ... ic-support
User avatar
xahmol
Squad Leader
Posts: 611
Joined: Sun Jun 28, 2020 7:32 pm
Location: Utrecht, The Netherlands
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by xahmol »

Dbug wrote: Sat Jan 25, 2025 10:51 pm Regarding fopen_s, I never used it, but as far as I can see it returns an errno, are you sure that file would be set to zero if the open fails?
I can confirm that Oscar64 does this completely opposite to CC65: all file functions return 1 on success and 0 on fault, with errnumber going to a variable.
This is very unhandy porting code from CC65, and cause for MANY bugs in my code to kink all the forgotten reworks of the return code out.

Told Drmortalwombat that, but he sticks to this method, also as changing this would break all existing Oscar64 code.
So guess we have to live with that.
And also I often thought, ok, would have done this differently. But ok, let’s stick to consistency.

And @ibisum: realise my comments in your PR on Locitest probably are read much more critical then I intended it. I agree actually with your point, just did not want to prio it as I first wanted to finish File Manager instead of fixing my build environment to work with your PR.
Mentioned it here to support that probably it is better to separate build chain PRs from Oric targer PRs to have a higher chance of if being accepted,
Was certainly not meant as criticism on your code quality, and would hate it to discourage what you are trying to achieve.
User avatar
ibisum
Wing Commander
Posts: 1912
Joined: Fri Apr 03, 2009 8:56 am
Location: Vienna, Austria
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by ibisum »

But ok, let’s stick to consistency.
There are many things I do not like about the Oscar64 code-base already - the lack of braces for if statements is a bit of a dreadful situation, for example - but for this contribution I choose to match the style and be consistent. I also tend to comment my code, as I often write the comments first, but have had to strip comments in order to match the existing code style.

So, fopen_s, for me, is non-sequitur - especially when the rant started off with 'oh, you broke the formatting, try to be consistent' .. the code-format change only happened because I was attempting to find a way to conform with drmortalwombat's style, in the first place. Anyway, I backed that out, and the currrent state of the contribution is aligned, both style-wise and code-smells too. Please factor that in if/when you evaluate this in the following days.
fixing my build environment to work with your PR.
Thanks for understanding that, xahmol. It was not my intention to break your flow or build technique - I just failed to inform you that my fix to your projects' build system was intended to make it easy for those of us who use package-manager provided CC65, while also allowing you to continue with your custom CC65 build. If I'd made it a bit clearer that you could still operate the way you wanted by setting an environment variable (that is all it takes) to point to your customer CC65 builds, perhaps it wouldn't have been a frustrating PR for you. Sorry about that - I'll try to be a bit more verbose in future submissions.
Mentioned it here to support that probably it is better to separate build chain PRs from Oric targer PRs to have a higher chance of if being accepted,
Its a dead horse now but this is still also not applicable in this case - I already got my PR for Oscar64 to build on MacOS and Linux merged by drmortalwombat, and his prior build rig still works also, so I don't know where this is coming from. I think you didn't read the diff that closely, or maybe the problem is you've only read the diff, and not the repo log, so far ..


Anyway I'm sitting down for some more hacking on this today, and hope to have a first-pass producing Oric .TAP files in the afternoon. It'd be grand if some of you folks could dig into the Oric-specific changes I'm contributing, and leave the meta- discussion for when there is, actually, a PR ready for submission that adds Oric support to Oscar64. It'll be fun to have a new compiler for Oric, no?
User avatar
ibisum
Wing Commander
Posts: 1912
Joined: Fri Apr 03, 2009 8:56 am
Location: Vienna, Austria
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by ibisum »

More bread crumb notes:

I don't have the BC_REG_* assignments working properly for Oric yet, resulting in a segfault when compiling a test program.

Code: Select all

               BC_REG_ACCU = 0x08; // use existing Oric indices for FACC
               BC_REG_WORK_Y = 0x48; // Oric free work area (1 byte)
               BC_REG_WORK = BC_REG_WORK_Y + 0x01;          // 0x48 + 0x01 = 0x49

               BC_REG_TMP = BC_REG_WORK + 0x0A;             // 0x49 + 0x0A = 0x53
               BC_REG_TMP_SAVED = BC_REG_TMP + 0x10;        // 0x53 + 0x10 = 0x63
               BC_REG_LOCALS = BC_REG_TMP_SAVED + 0x10;     // 0x63 + 0x10 = 0x73

               BC_REG_FPARAMS = BC_REG_LOCALS + 0x02;       // 0x73 + 0x02 = 0x75
               BC_REG_FPARAMS_END = BC_REG_FPARAMS + 0x0A;  // 0x75 + 0x0A = 0x7F

               BC_REG_IP = BC_REG_FPARAMS_END;              // 0x7F
               BC_REG_ADDR = BC_REG_IP + 0x02;              // 0x7F + 0x02 = 0x81
               BC_REG_STACK = BC_REG_ADDR + 0x04;           // 0x81 + 0x04 = 0x85
               
Once the morning fog clears, I will map this out a bit more rigorously - if anyone sees anything obvious meanwhile, it'd be great to hear your thoughts.
User avatar
xahmol
Squad Leader
Posts: 611
Joined: Sun Jun 28, 2020 7:32 pm
Location: Utrecht, The Netherlands
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by xahmol »

ibisum wrote: Sun Jan 26, 2025 12:51 am Okay, conio functions added.
Have some trouble following this, the addresses used of ROM functions all do not seem correct if I look in the Oric Advanced User Guide ROM mapping.
Also do not understand the storing in $FF01 as this is a ROM area unless overlay RAM is active, but then calling ROM functions would not be possible.

So are you sure you got the correct Oric addresses?
And if there is no direct equivalent to Commodore ROM function, maybe it is best to just add own code that does the same.
User avatar
ibisum
Wing Commander
Posts: 1912
Joined: Fri Apr 03, 2009 8:56 am
Location: Vienna, Austria
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by ibisum »

You're right! I totally screwed those up. Was a bit brain-fry'ed last night during the code formatting fixes, I suppose, and I managed to use the wrong addresses from my notes. Fix in progress.

BTW, I'm targetting the Oric-1 ROM for now - once this works, I'll see about adding oric/atmos flags.

The conio stubs are simple and we should use ROM functions for now, as they are all available in Oric - its simple "putchar/getchar" and "place cursor at x/y" functions, which are all relatively simple. The thornier issue is, what to do about the CRT, as this code is very difficult to read to my eyes. I will format it today, make any needed changes, and back-port the changes to the non-formatted original code. Once Oric support is working, I may take another pass at refactoring this for drmortalwombat, as I think this layout is detrimental to adding further platform support..
User avatar
Dbug
Site Admin
Posts: 4935
Joined: Fri Jan 06, 2006 10:00 pm
Location: Oslo, Norway
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by Dbug »

ibisum wrote: Sat Jan 25, 2025 11:25 pm
So looking at your link I can immediately see things that would be rejected as a review where I work because they are changes that were not necessary:
- The line with "target_link_libraries(${PROJECT_NAME}" has a modified carriage return or white space at the end
Oh, what a boring, pedantic whine. This isn't a branch that is submitted as a PR yet, so stop evaluating it as if it were.
This is work in progress, so please don't bother commenting on such administrivia until there is actually a PR ready.
It's not pedantic, it's practical: I don't see the advantage of waiting for you to spend days working on something containing changes that make the work difficult in a team environment with multiple people potentially changing the code in the same files.
ibisum wrote: Sat Jan 25, 2025 11:25 pm
- The "if (mTargetMachine == TMACH_ATARI)" is now a "else if" and Oric has become the primary if instead
Oh my god, did you even read the code?
I just looked at the diff URL you linked; if it was the github diff showing incorrect things, then sure.
All I saw is that there was a "-" on the "if Atari" section, and a "+" showing a "if Oric" and a "else if Atari" section of code.
User avatar
xahmol
Squad Leader
Posts: 611
Joined: Sun Jun 28, 2020 7:32 pm
Location: Utrecht, The Netherlands
Contact:

Re: Adding Oric support to the Oscar64 C compiler

Post by xahmol »

As far as I could grasp from the crt.c code is that it prepares the C runtime. And safeguards everything that overwrites in the first few pages of memory, to restore that on exit.
So it is especially that safeguarding that needs to be Oric specific, maybe some initialising as well.

But yeah, this one is a challenge to follow with all the if clauses and not always knowing what the addresses do on the original targets.
User avatar
ibisum
Wing Commander
Posts: 1912
Joined: Fri Apr 03, 2009 8:56 am
Location: Vienna, Austria
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by ibisum »

@Dbug: Please, just read the code carefully. Literally none of your concerns are warranted. The cross-platform build contribution has already been merged upstream, the formatting matches Oscar64's existing code, and I'm consistent with adhering to the very same code smells that exist throughout the entire codebase. I use CMakeListst just fine to produce working Oscar64 bins for MacOS and Linux - drmortalwombat still has his Visual Studio rig, which still works.

There are definitely bugs in my code - but there is also literally nothing in my contribution which prevents people from building Oscar64 from my branch, and running it, either. Nor will there be, when I rebase in preparation for submitting a PR for drmortalwombat's careful review. He and I already have rapport on things so far, I'm not going to mess with that.

@xahmol: Per drmortalwombat, the initialization depends heavily on the registers being properly set, which I already see I have not done correctly. The work so far has been more to get the 'scaffolding' in place without making impacts, which is now mostly ready - today I will drill into the details with a fine comb. Yes, the CRT code is thorny, and I'm learning far more about the C64 and PET than I really wanted to, but .. someone said this compiler is good .. so that's what brought me here. :)\\ :roll: It has to be said: I wanted to do the grunt work to see if I can quickly add Oric support to Oscar64, but I also kind of think it'd be cool if you boffins could know a bit about the effort too (thus the bread-crumb thread), since .. eventually .. one or two of you might end up actually using Oscar64 for Oric development, if things go well .. I'm doing the grunt work. Give me some drops of your Oric expertise though, guys.
Last edited by ibisum on Sun Jan 26, 2025 10:58 am, edited 2 times in total.
User avatar
ibisum
Wing Commander
Posts: 1912
Joined: Fri Apr 03, 2009 8:56 am
Location: Vienna, Austria
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by ibisum »

Re: conio stubs - Any objections to using some of the OSDK's code for this?

The code in OSDK's gpchar.s, for example, could easily be used to finish Oscar64's conio stubs.
User avatar
Dbug
Site Admin
Posts: 4935
Joined: Fri Jan 06, 2006 10:00 pm
Location: Oslo, Norway
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by Dbug »

ibisum wrote: Sun Jan 26, 2025 10:54 am Re: conio stubs - Any objections to using some of the OSDK's code for this?
The code in OSDK's gpchar.s, for example, could easily be used to finish Oscar64's conio stubs.
No objections, but about 95% of the OSDK library code dates from the earlier SDK from 1996 and only runs on Oric Atmos due to the directl ROM calls.
ibisum wrote: Sun Jan 26, 2025 10:54 am @Dbug: Please, just read the code carefully. Literally none of your concerns are warranted.
I was just explaining what I saw from when you posted your first link days ago, it's what I saw on the diff, nothing more.
User avatar
ibisum
Wing Commander
Posts: 1912
Joined: Fri Apr 03, 2009 8:56 am
Location: Vienna, Austria
Contact:

Re: Adding Oric support to the Oscar64 C compiler...

Post by ibisum »

I was just explaining what I saw from when you posted your first link days ago, it's what I saw on the diff, nothing more.
I understand, but it was not helpful and that is disappointing. Work with me here, DBug. This is my fork - the time to worry about code format issues and other smells, is after I've done the rebase (when things are actually working), and am preparing an actual PR. That's not where we are at, at the moment - maybe by the end of the day, however. It'd be awesome if you could find the time to clone my fork, do your own build and report any build-related issues you encounter. My first contribution to Oscar64 - the CMakeLists.txt file which made Oscar a multi-platform binary - has already been merged upstream. My next PR to drmortalwombat will be once we have all confirmed - and maybe even properly tested - Oric architecture support.
No objections, but about 95% of the OSDK library code dates from the earlier SDK from 1996 and only runs on Oric Atmos due to the directl ROM calls.
Yes, understood (been code-reading meanwhile), so I think that means that I will make Atmos the first target, actually and then worry about Oric-1 ROM support when things are smoother.
Post Reply