Adding Oric support to the Oscar64 C compiler...

[ibisum]

Thought I'd start a thread documenting the effort to add Oric/Atmos support to the Oscar64 C Compiler...

Repo: https://github.com/drmortalwombat/oscar64.git

Notes:

drmortalwombat (oscar64 author) gave me the advice (in this FR: https://github.com/drmortalwombat/oscar64/issues/183) to follow the TMACH_ATARI and TMACH_X16 machine-type enums, in order to get a feeling for the additions needed to add Oric support.

Here, a bread-crumb trail and notes I've made so far regarding what changes are needed to add Oric support - I'm hoping some of the Oric deep crew will pitch in with their ideas/corrections to my ideas, as I don't have a lot of confidence on any of this yet - it *seems as though it would be quite simple to add Oric support*, with the two most difficult things being to ensure the Zero Page System Variable indexes are correct, that the various Memory regions are correctly defined, and that an approprite writeTapFile() function be written, which could possibly be sourced from/compared with that of OSDK's existing functions:


Zero Page System Variable Indexes:

Code: Select all

//Compiler.cpp:55
bool Compiler::ParseSource(void)
{
	if (mTargetMachine == TMACH_ATARI)
	{
		BC_REG_WORK_Y = 0x80;
		BC_REG_WORK = 0x81;
		BC_REG_FPARAMS = 0x8b;
		BC_REG_FPARAMS_END = 0x97;

		BC_REG_IP = 0x97;
		BC_REG_ACCU = 0x99;
		BC_REG_ADDR = 0x9d;
		BC_REG_STACK = 0xa1;
		BC_REG_LOCALS = 0xa3;

		BC_REG_TMP = 0xa5;
		BC_REG_TMP_SAVED = 0xc5;
	}

Note: These are addresses in zero-page for specific architecture features. Per my reckoning, these could be values for the Oric target:

Code: Select all

	if (mTargetMachine == TMACH_ORIC) {
		BC_REG_WORK	= $00;
		BC_REG_WORK_Y = $02;
		BC_REG_FPARAMS = $20;
		BC_REG_FPARAMS_END = $2F;
		BC_REG_IP = $30;
		BC_REG_ACCU = $34;
		BC_REG_ADDR = $36; 		// .. $37;
		BC_REG_STACK = $40; 	// ..$5F;
		BC_REG_LOCALS = $60; 	// ..-$7F;
		BC_REG_TMP = $04;
		BC_REG_TMP_SAVED = $08;
	}

Zero-Page Compiler Variable space:

Code: Select all

Compiler.cpp:504
// bool Compiler::GenerateCode(void) ... 

	LinkerRegion* regionZeroPage = mLinker->FindRegion(identZeroPage);
	if (!regionZeroPage)
	{
		if (mTargetMachine == TMACH_ATARI)
			regionZeroPage = mLinker->AddRegion(identZeroPage, 0x00e0, 0x00ff);
		else if (mCompilerOptions & (COPT_EXTENDED_ZERO_PAGE | COPT_TARGET_NES))
			regionZeroPage = mLinker->AddRegion(identZeroPage, 0x0080, 0x00ff);
		else
			regionZeroPage = mLinker->AddRegion(identZeroPage, 0x00f7, 0x00ff);
	}

Note: by my reckoning, at first glance we should support Oric ROM co-existence, so this would be an appropriate setting - but probably would need further tweaking to support Oric Bare-metal:

Code: Select all

		if (mTargetMachine == TMACH_ORIC) {
			regionZeroPage = mLinker->AddRegion(identZeroPage, 0x0080, 0x00FF);
		}

.TAP file writing routine:

Code: Select all

Compiler.cpp:1310
		if (mTargetMachine == TMACH_ATARI)
		{
			strcat_s(prgPath, "xex");
			if (mCompilerOptions & COPT_VERBOSE)
				printf("Writing <%s>\n", prgPath);
			mLinker->WriteXexFile(prgPath);
		}

Note: This is a little trickier - I guess a WriteTapFile() function would have to be written, possibly parts of this could come from the existing OSDK compiler. TODO: Further investigate the requirements.


Set the BASIC Start region for Oric:

Code: Select all

oscar64.c00:447
		else if (!strcmp(targetMachine, "x16"))
		{
			strcpy_s(basicStart, "0x0801");
			compiler->mTargetMachine = TMACH_X16;
			compiler->AddDefine(Ident::Unique("__X16__"), "1");
		}

This seems pretty obvious, not sure we need to AddDefine() though, but obviously the basicStart address would be good to know - I think its a matter of s/0x0801/0x0501/, thus:

Code: Select all

oscar64.c00:447
		else if (!strcmp(targetMachine, "oric"))
		{
			strcpy_s(basicStart, "0x0501");
			compiler->mTargetMachine = TMACH_ORIC;
		}

Okay, that is my first (albeit quite glib) pass at breadcrumb trailing the tweaks needed to add Oric support to Oscar64 - would be great to hear some feedback from those of you more knowledgeable, and maybe we can see about adding Oric support in the Oscar64 compiler, relatively quickly ..

[xahmol]

Super!
Again, limited in time, so afraid I can not contribute much now.

But I guess also some work, would be needed to adapt the screen output functions (like conio.c) to the Oric.

And I would assume there is need to adapt crt.c to the Oric envirionmemt.
https://github.com/drmortalwombat/oscar ... lude/crt.c

In that BASIC start: question is if we would need actually a BASiC header. A self starting .TAP image does not need one.
Advantage in that case is that you can start at $400 instead of $501, and do not need space for the basic header,
Disadvantage is that you can not exit and start again, but have to clean/reboot to properly exit.

[ibisum]

Duly noted, and yes there is probably some work to be done on the libraries once the compiler is producing .TAP files ..

[xahmol]

Duly noted, and yes there is probably some work to be done on the libraries once the compiler is producing .TAP files ..

For sure, but primarily conio.c and and of the related screen output ones.
Those functions now use Commodore kernal calls.

Obviously also any file IO, but think file IO is only practically implementable for Loci as target.
And I like the C64 charwin lob, ported that one myself to C128 VDC for my C128 projects.

And my only issue with Oscar64 is that the creator obviously never designed it to be multi target: all the if clauses in a.o. the crt.c get very confusing with even more targets. CC65 solved that one better by having different crt files separately for every target.

[xahmol]

Oh, and the AddDefine() is needed as input for those if statements in a.o. crt.c, so yes, we need it.

Adding a tape header to a binary should be trivial, that is just adding a few bytes.

In CC65 it is this:
https://github.com/cc65/cc65/blob/maste ... /tapehdr.s

With this as BASIC header:
https://github.com/cc65/cc65/blob/maste ... s/bashdr.s

And modified it myself to:
https://github.com/xahmol/locifilemanag ... /tapehdr.s
And I ditched the BASIC header in that project.

[Dbug]

In that BASIC start: question is if we would need actually a BASiC header. A self starting .TAP image does not need one.
Advantage in that case is that you can start at $400 instead of $501, and do not need space for the basic header,
Disadvantage is that you can not exit and start again, but have to clean/reboot to properly exit.

Another issue with starting in $400 is that this will not work with Sedoric for example because that's where the ROM/DOS switching code is located.

Imo two setups are needed: One for "oric system compatible" programs which need to be respectful of the content of page 0, 2 and 4... and one for the "I have full control of the machine" in which case you can use all that the way you want

[xahmol]

Another issue with starting in $400 is that this will not work with Sedoric for example because that's where the ROM/DOS switching code is located.

Imo two setups are needed: One for "oric system compatible" programs which need to be respectful of the content of page 0, 2 and 4... and one for the "I have full control of the machine" in which case you can use all that the way you want

Good one, totally forgot about that, did not code for SEDORIC for a while….
Am using $400 since you were stating somewhere that you did, and that works great with my Loci targeted projects (logical, as that does not use SEDORIC). Indeed my SEDORIC based projects use the basic header from $501, but that was more because it is the CC65 default and did not learn from you yet that starting at $400 was possible…

By the way: the CC65 crt0 for the Oric target never worked completely for me on exit, it somehow does not return to a state that you can actually restart the program with run. Never had the patience to find out why, probably something in page 0 or 2 not restored correctly.
But if we are doing the Oscar64 crt.c from scratch, we could just as well make sure that if you use the basic header, then at least at exit it exits to a state that you actually can rerun that basic header.
And if you have to start at at least $500 anyway, then I would just as well start at $501 with basic header as that only takes something like 25 bytes or so, while making exit and restart much easier for the user.
But yes, two targets would be nice then. Will complicate stuff even more though in all those if target then clauses. So would prioritise the CC65 standard $501 with basic header.

[ibisum]

To be fair, the precedent for having two targets has already been set in Oscar64 - my desire to simplify this is just to reduce the complexity of the work to add Oric support, but very strong arguments have been made for there to be two paths, based on configuration.

I will spend some time re-tracing the breadcrumb trail over the weekend with a view to making a first, somewhat naive code contribution. I'm going to need to lean heavily on you folks for testing and debugging purposes, however. I already see that we will need some test programs to verify things ..

[ibisum]

Just a note: For anyone following along, I've converted the Visual Studio .sln/.vcxproj files to CMakeList.txt, so Oscar64 can be built on MacOS/Darwin and Linux.

https://github.com/seclorum/oscar64.ori ... eLists.txt

I've successfully built it with this CMakeLists.txt, built all the samples, run a couple of them .. and am now proceeding to work on adding Oric support.

I'll be working on my fork (oscar64.oric in URL above) and submitting a PR from there ..

[ibisum]

A little progress report, I've gotten as far as having the basic Oric zero page description, and writing .TAP files, in place:


https://github.com/seclorum/oscar64.ori ... ain...main

All untested so far - now going to move towards the crt problem ..

[Dbug]

It looks like the entire codebase has gone through a massive reformatting, makes it quite hard to see what the actual changes are.

[xahmol]

I am indeed afraid that creating a PR this way will not be accepted as it is impossible to review.
Guess this is because all the ‘make it work on a Mac’ that creates all those diffs also on what you did not change?

No git pro, and never worked with a Mac, but there is no way to keep all the reformatting and make it work on Mac stuff apart from the PR you want to make?

I know your PR to make Locitest work on Mac also did break my build chain, so had to reject.

[ibisum]

Yes, I accidentally pushed a re-indented version, my CLion configuration is set up for auto-indent'ing and I mistakenly pushed the reformatted branch. Will fix it, stay tuned ..

[ibisum]

Okay, sorry about that folks ..

I've now fixed the indent issues and got things a bit more clinical on my side of the fork, here's the diff:

https://github.com/drmortalwombat/oscar ... ic-support

* Added TMACH_ORIC support for the project, AddDefine("__ORIC__") for crt code
* BC_REG_ setup for Oric ZP
* WriteTapFile() function

(The issue with indent was because I wanted to try to conform to drmortalwombats' preferences, but I don't quite have the gindent settings for that yet .. I'll just try to follow his tabs/spaces policies manually for now.. @xahmol: I don't remember the PR for Locifiletest, but its often necessary when people start contributing, to align such policies .. also build environments .. I have a preference for vim as an IDE, CLion if its a modern code-base, CMake-based projects, and a standard set of gnu indent preferences set in a project .indent.pro file - but some folks prefer Visual Studio/C++ and other dark and arcane things ..)

[Dbug]

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.

Imo the most important is that contributors should minimize their impact on other people's codebases, and 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.

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
- The "if (mTargetMachine == TMACH_ATARI)" is now a "else if" and Oric has become the primary if instead

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").

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?