freemint / m68k-atari-mint-gcc

Fork of GNU's gcc with support for the m68k-atari-mint target
https://github.com/freemint/m68k-atari-mint-gcc/wiki
Other
26 stars 7 forks source link

Support for PRG+ELF for older compilers #26

Closed th-otto closed 10 months ago

th-otto commented 1 year ago

Some issues may still need to be resolved, but i think its about time to talk about adding the same support also to gcc 4.6.4 (used by emutos) and gcc 7.5.0 (used here for most projects) atleast.

I've started on this already in my own forks, but i think same should be done here, too. There should be some final decisions been made however, to not end up with incompatible toolchains. The underscore setting is now the same, but some other differences remain:

So question is whether we need to include that file at all, just to redefine the settings in mint.h again, or add the things that are missing directly to mint.h. In my forks, i used the latter.

Generally, supporting to build both a.out and elf toolchains will need a few more adjustments, both to mint.h and some other config files.

Note that i'm not speaking about adding a.out support back to binutils, or stabs to gcc. That will mean you will still need binutils-2.30 (last official one with a.out support) for a.out toolchains, and binutils >=2.41 for elf toolchains.

Another thing: after vincents initial commit to support PRG/ELF, there have been some more refinements. Should they be squashed into a single commit when backported, or be applied one after the other, like done in gcc-13-mintelf?

Edit: there is another thing i was thinking about, when preparing the patches for emutos. The LTO support in gcc 4.6.4 does not support attribute((optimize)) on a per-function level. In EmuTOS that can easily be handled by adding -D__LTO__ to the LTOCFLAGS. But it might make sense to automatically define that symbol in the compiler when using -flto.

vinriviere commented 1 year ago

Some issues may still need to be resolved,

Yes. Each remaining issue has to be addressed separately, until we consider we reach something stable.

but i think its about time to talk about adding the same support also to gcc 4.6.4 (used by emutos) and gcc 7.5.0 (used here for most projects) atleast.

Regarding to my own binaries, I'm not interested in old GCC versions. In 2007, my initial goal with the cross-tools was to provide up-to-date GCC/binutils toolchain for C++ projects. My binaries/patches were quickly used by the FreeMiNT team (I wasn't aware it was still alive) and by EmuTOS. I updated GCC up to version 4.6.4, then I stopped for several reasons. Lack of time, of course. But also because during some time the FreeMiNT sources weren't compilable with GCC 4.7. And finally because new gcc produced bigger and slower code than before. I don't really mind, but that's a problem for EmuTOS.

At some point, I was late to update my GCC 4.6.4 toolchain (MiNTLib, PML instead of fdlibm...) and that caused trouble to new FreeMiNT autobuilds. So I proposed that FreeMiNT uses dedicated binaries, not directly my own binaries. So I can hibernate during years without causing trouble to the FreeMiNT team. So FreeMiNT ugraded to its own GCC 7.5.0, and it was fine like that.

EmuTOS is a specific case, as it is self-contained. Newer GCC won't bring anything good, except (a bit) bigger size and (a bit) slower execution. Only exception is LTO available with GCC/ELF, which mitigates (actually improves) the size issue. I'm sure that the situation will improve, as we regularly find issues in current GCC and report them to the GCC team. But currently, it's like that.

That being said:

Sorry for this long monologue, at least you all know that I have in mind.

So @th-otto, feel free to upgrade older GCC to PRG/ELF, and provide your binaries. If this can be useful for anyone (starting from FreeMiNT), then go for it. I just say that I won't build such binaries myself, that's out of my scope.

I've started on this already in my own forks, but i think same should be done here, too. There should be some final decisions been made however, to not end up with incompatible toolchains. The underscore setting is now the same, but some other differences remain:

Yes. This has to be decided, for good reasons. I'm still in favor to BITFIELD_TYPE_MATTERS as it allows odd-sized structs, like all other contemporary compilers. But we must really find where and why it actually causes trouble in the real world. After that, we will have all the information to decide objectively.

  • use of A0/A1 for M68_STRUCT_VALUE_REGNUM/STATIC_CHAIN_REGNUM. This is pulled in from m68kelf.h, but a.out compiler used the opposite setting. As there is no advantage of using one or the other, it should stay the same as before.

That's the other topic. Why are there different settings, as that doesn't bring any advantage? I guess there was a register conflict is some operating system? Which one? Does it also affect the Atari targets? Once again, before making a choice, we have to know why, and not dumbly do because it was done that way before. We have an unique opportunity to do better. Personally, I have no preference. But I would be annoyed if the Atari target use different settings than other contemporary targets for no known reason.

So question is whether we need to include that file at all, just to redefine the settings in mint.h again, or add the things that are missing directly to mint.h. In my forks, i used the latter.

For the experimental toolchain, my concern was to create minimal patches, and be as close as possible to m68k-elf for testing. We know that m68k-elf has some settings not suitable for us, so we must change them. Indeed, that's a good question: include or override? Key is the maintainability. When upgrading from version to version, we don't want surprises. Generally, the best solution is to keep minimal patches, so include+override is best (what I tried to use). But if we override everything, that doesn't make sense at all.

So 2 solutions :

I haven't looked closely at those files recently. I may prefer the second solution (put everything into mint.h), even if it wasn't used in my initial patch. Key point is to ease future maintainability. We should also avoid solutions involving much code duplication (because if the source changes, we have to also report the changes to mint.h).

Anyway, this isn't critical, as it is just a matter of source reorganisation. Key point is to decide of an ABI, and stick to it. Whatever how the includes are organized.

From what I remember, there is no obvious choice to completely avoid code duplication. We will have to find an acceptable compromise, regarding to maintainability.

Also, it would be very helpful to have something like a testsuite, somewhere. To check that GCC has been correctly configured, regarding to our standards. That could be scripts testing the GCC defines, and checking assembler output.

Generally, supporting to build both a.out and elf toolchains will need a few more adjustments, both to mint.h and some other config files.

As said previously, I'm not personally interested in supporting a.out toolchains. But if some adjustments can help you @th-otto to do that, and they don't cause trouble to me for ELF-only toolchain with the latest GCC, then that's fine.

Note that i'm not speaking about adding a.out support back to binutils, or stabs to gcc. That will mean you will still need binutils-2.30 (last official one with a.out support) for a.out toolchains, and binutils >=2.41 for elf toolchains.

Fine. That's sensible.

Another thing: after vincents initial commit to support PRG/ELF, there have been some more refinements. Should they be squashed into a single commit when backported, or be applied one after the other, like done in gcc-13-mintelf?

This is a good question. I wondered how to do when I rebuilt all the mint-gcc history when I initially setup the gcc repository on GitHub. I chose to keep the initial patches in original branches, then to only use a single squashed commit for each new major new version (actually, I mainly did that for binutils). But doing so, we lose the commit history and useful information on the new branch. Thanks to GitHub, it can still be retrieved from the original branch, buts it's not easy to find.

An example showing that a single squashed commit is not really the ultimate solution is the math-68881.h. We keep it our mint branches, because we need it, but it isn't strictly related to the mint target. It's something missing from the official C sources, which should ideally be pushed upstream.

I think that the ideal solution (for browsing the history) is to rework the patches when switching to a new version, to keep a minimal set of patches. I.e. "add support for mintelf target", "fix math-68881.h", etc.

But I see that I'm slightly out of topic. My answer was about moving the patches onto a new branches, and never look back (what I did initially in GCC 4.x branches). If the goal is to keep several branches alive, then we have no other choice than adding the new fixes on top of each branch. And maybe we could allow rebasing the history for old branches, if that makes sense. Well, the ideal solution doesn't exist.

Keeping history tidy is important. For the current development branch, it's certainly better to keep patches as they arrive, sequentially for each change. For secondary branches, like old GCC versions still in use, it might be better to rework the history with a few well-structured patches. I don't know.

FYI, inside my Ubuntu or Cygwin packages, I always use a squashed patch applied to the official sources. So whatever solution is used, that won't affect my current process.

mikrosk commented 1 year ago

My POV is quite close to Vincent's: I don't want to maintain/provide gcc-7 nor gcc-4 mintelf builds, it's just more work and confusion. As soon as gcc-13-mintelf is done, I don't plan to look back either. Heck even now I'm already compiling everything with gcc-13, I have gcc-7 just a fallback for some performance comparisons.

Having gcc-4/7 as t he "old a.out compiler" works nicely for us: you don't like to rework your asm sources? Fine, just keep using gcc-4/7, no problem with that. You'd like to explore new gcc-13+ options? Perhaps even the debugger? Fine, it requires some effort on your side. Both sides should be happy, nobody loses anything.

Having said that, I have noticed that some people do appreciate Thorsten's gcc-13 a.out builds for example so I can image that for some people will welcome if Thorsten backports those mintelf changes into his gcc-4/7 builds. I don't know, personally I wouldn't bother.

As for squashing: when everything is done, I've planned to ask you for your permission to squash everything and force-push, I'm pretty sure our current efforts will lead to many random commits.

th-otto commented 1 year ago

I'm sure that the situation will improve, as we regularly find issues in current GCC and report them to the GCC team. But currently, it's like that.

I'm quite sure the situation won't improve much in the next ten years or so. There are still some issues that even effect other targets, but noting has happened for several years.

plan to keep my GCC 4.6.4 a.out binaries as-is, without any upgrade.

Ok, so maybe we can skip 4.6.4 for now (although it won't be much more work, because i have most of the patches ready). But maybe for freemint, we decide one day to switch to gcc 7.5.0 mintelf (that would still require some more changes than just the compiler, mainly settings in Makefiles, adjusting where to fetch prebuilt binaries etc).

Why are there different settings, as that doesn't bring any advantage? I guess there was a register conflict is some operating system? Which one?

The situation is that

I think that choice was made because of the SVR4 ABI, where functions return pointer values both in D0 and A0, and therefor A0 cannot also be used to return structure values.

Does it also affect the Atari targets?

Yes, it does. At least when using shared libs that were compiled by one compiler, and an application that was compiled by another. That alone makes it difficult to change it. Another example would be assembler sources that are ported to the elf toolchain, there is no way currently to know from the compiler which setting was used. And gdb also "knows" where struct values are returned.

But I would be annoyed if the Atari target use different settings than other contemporary targets for no known reason.

See notes above. We are already using the same setting as linux/bsd, so there is absolutely no good reason to change that.

For the experimental toolchain, my concern was to create minimal patches, and be as close as possible to m68k-elf for testing.

Yes, i understand that. But m68k-elf isn't a real target that is used anywhere.

Key is the maintainability.

Yes, i agree. But for me it seems to be simpler and safer to define the settings we want, rather than being surprised by a new settings that suddenly pops up in m68kelf.h and we forgot to override it.

When upgrading from version to version, we don't want surprises. Generally, the best solution is to keep minimal patches, so include+override is best (what I tried to use). But if we override everything, that doesn't make sense at all.

However, we still have to monitor the changes on other m68k files, because if a new parameter appears, we might miss it.

Yes, that's the most difficult part sometimes, atleast for new major versions.

An example showing that a single squashed commit is not really the ultimate solution is the math-68881.h. We keep it our mint branches, because we need it, but it isn't strictly related to the mint target. It's something missing from the official C sources, which should ideally be pushed upstream.

IIRC we already tried that, but they said that this include file should disappear. I personally have a different opinion about this, since that would mean a lot of simple operations will result in a library call rather then being inlined.

FYI, inside my Ubuntu or Cygwin packages, I always use a squashed patch applied to the official sources. So whatever solution is used, that won't affect my current process.

Yes, thats the same i do in my scripts. If someone is interested at the single commits, he has to look at the git history.

vinriviere commented 1 year ago

I think that choice was made because of the SVR4 ABI, where functions return pointer values both in D0 and A0, and therefor A0 cannot also be used to return structure values.

If a function returns a struct, it can't also return a pointer at the same time. So using the same register shouldn't conflict.

Anyway, as the SVR4 people did strange and unwanted things to return pointers, we could also consider they did equally strange and unwanted things to return structs.

So let's keep away from SVR4 strangeness, and keep standard m68k settings. As I said, I have no preference for this setting. I just want us to avoid making a bad choice. I used m68k-elf as reference for the experimental toolchain, finally it seems this wasn't a very wise choice.

Does it also affect the Atari targets?

Yes, it does. At least when using shared libs that were compiled by one compiler, and an application that was compiled by another. That alone makes it difficult to change it. Another example would be assembler sources that are ported to the elf toolchain, there is no way currently to know from the compiler which setting was used. And gdb also "knows" where struct values are returned.

Good interfaces must unambiguously specify the ABI. Did the slb spec define which register to use to return structs? I don't think so. So that's a grey area.

@th-otto You pointed that C++ returns structs everywhere, that's right. On the other hand, I don't know any C source returning structs by value. Even if this is valid, that's a rarely used practice. I would be very surprised if we could find any slb in the real world doing that.

There are examples of ABI changes:

As long as the ABI is well documented, there are ways to make adaptations so it can still work. Even if that's not always pretty. And again, I would be interested to see where there would be issues in the real world by changing the register used to return structs by value.

Anyway, that's unimportant, as we both agree to drop the strange SVR4 ABI to return pointers/structs.

But I would be annoyed if the Atari target use different settings than other contemporary targets for no known reason.

See notes above. We are already using the same setting as linux/bsd, so there is absolutely no good reason to change that.

If they are good settings, and widely used, there is no reason to use anything else. But with a new toolchain, except the special case of dynamic libraries, there is no reason to refrain from changing the ABI for a better choice. And if finally old settings were better for good reasons, then no problem, let's keep that.

Yes, i understand that. But m68k-elf isn't a real target that is used anywhere.

Really? I would have bet that it's the one used everywhere for embedded programming? Am-I mistaken? Really, I have no proof. I considered it as the more standard m68k target. Maybe I'm just wrong.

Yes, i agree. But for me it seems to be simpler and safer to define the settings we want, rather than being surprised by a new settings that suddenly pops up in m68kelf.h and we forgot to override it.

OK. As I said, I wasn't convinced by the override solution in the specific case of mint.h.

Yes, thats the same i do in my scripts. If someone is interested at the single commits, he has to look at the git history.

Good. So we must ensure that individual commits (or reworked individual commits) still exists somewhere. Then we can easily use squashed commits elsewhere.

th-otto commented 1 year ago

Did the slb spec define which register to use to return structs

Not really. But fact is that SLBs should be compiled by gcc. It would be difficult to compile them using other compilers like Pure-C (which also clobbers D2), and then call the functions from gcc which expects D2 to be preserved.

I don't know any C source returning structs by value. Even if this is valid, that's a rarely used practice. I would be very surprised if we could find any slb in the real world doing that.

IIRC, the png and lzma libraries do that. That's what makes it sometimes difficult to call such functions from other compilers, like Pure-C.

Anyway, that's unimportant, as we both agree to drop the strange SVR4 ABI to return pointers/structs.

Good ;)

Good. So we must ensure that individual commits (or reworked individual commits) still exists somewhere. Then we can easily use squashed commits elsewhere.

I'm only talking about the mint-4_6-mint and gcc-7-mint branches. Individual commits are still visible in the gcc-13-mintelf branch.

But if there is no interest in doing such things here, i will just do the backports in my own fork.

th-otto commented 1 year ago

FYI, inside my Ubuntu or Cygwin packages

@vinriviere : i fear you have to rebuilt them again. There was a bug in mintlib headers (missing extern "C" for statvfs()), which leads to mangled references to it for programs using std::filesystem. That also means you have to rebuilt & install a newer version of mintlib first.

vinriviere commented 1 year ago

Nothing to fear. I'm prepared to rebuild everything each time it's necessary. As long as all components of my toolchain are compatible with each other, that's fine for me. Corollary: I don't mind breaking compatibility with old binaries. Thanks @th-otto for that fix.

th-otto commented 1 year ago

I'm prepared to rebuild everything each time it's necessary.

Me2, but this was a really stupid bug, which now forces me to recompile all my toolchains. Several gcc major versions, several OS, and a.out+elf toolschains: last time i did that (not so long ago) it took almost two weeks just to compile everything...

vinriviere commented 11 months ago

Key is the maintainability.

Yes, i agree. But for me it seems to be simpler and safer to define the settings we want, rather than being surprised by a new settings that suddenly pops up in m68kelf.h and we forgot to override it.

When upgrading from version to version, we don't want surprises. Generally, the best solution is to keep minimal patches, so include+override is best (what I tried to use). But if we override everything, that doesn't make sense at all.

@th-otto: Here is a first try to get rid of m68kelf.h (work in progress, not to be pushed).

tmp-remove-m68kelf.patch.txt

Do you think we should go that way? Finally, our mintelf target is quite different of both m68k-linux and m68k-elf, so it makes sense to have a completely separate configuration file. I'm quite annoyed because this adds much more duplicated lines. On the other hand, it's more readable, as all the relevant settings are at the same place, without fear to bring some m68k-linux/m68k-elf oddity into mintelf. I have mixed opinions, with a small preference for this option.

If we agree to continue that way, I will finish that patch and carefully test it.

th-otto commented 11 months ago

Do you think we should go that way?

Yes, i think so. IMHO it does not make sense to have m68kelf.h override settings from m68k.h, just to override them again in mint.h (sometimes essentially reverting them to the m68k.h version).

Having them all in the same place seems to be easier to maintain. In some cases, comments may need to be added, to explain why we choose that settings, and where that setting originated from, but i think for most of them this is already the case.

Ideally, after cleanup, it would be nice if it is mostly identical to my own version (https://github.com/th-otto/m68k-atari-mint-gcc/blob/mint/gcc-13/gcc/config/m68k/mint.h), except for the a.out/dbx stuff of course.

mikrosk commented 11 months ago

our mintelf target is quite different of both m68k-linux and m68k-elf

I admit, this is pretty surprising for me. Who would have thought.

When upgrading from version to version, we don't want surprises

[...]

without fear to bring some m68k-linux/m68k-elf oddity into mintelf.

I just want to point out that a separate file is not a silver bullet -- there is still the possibility that GCC people update something important in m68k-linux/m68k-elf and since our port is off tree, they wont be aware of it. Typical example was that --with-cpu building in https://github.com/freemint/m68k-atari-mint-gcc/pull/8 which broke multilib setup differnet than default m68000.

I don't have a definite opinion on this either, I'll go with the crowd.

th-otto commented 11 months ago

Yes, on new releases (major versions atleast) it is always important to check whether any settings have been changed/added/deleted in m68k.h. But that can easily be done with git diff

vinriviere commented 11 months ago

our mintelf target is quite different of both m68k-linux and m68k-elf

I admit, this is pretty surprising for me. Who would have thought.

I initially made a mistake. I considered that m68k-elf was plain the vanilla ELF target, in other words: the normal thing. But it isn't. m68k-elf actually targets embedded systems without libc, and for an unknown reason it has adopted the strange SVR4 ABI. That's a nonsense for us, as we prefer standard things. On the other hand, m68k-linux has its own oddities, such as returning pointers into a0 (even worse: both a0 and d0 for compatibility). We don't want that either. So to be as standard as possible, mintelf has to be different from m68k-elf and m68k-linux. There is also the case of m68k-netbsd, but I didn't investigate it much. What else? Well, in older gcc, there was many other m68k targets. But now in gcc 13 I didn't find any other similar m68k targets. configure files are messy, I didn't find anywhere an exhaustive list of targets supported by binutils/gcc.

Yes, on new releases (major versions atleast) it is always important to check whether any settings have been changed/added/deleted in m68k.h. But that can easily be done with git diff

Yes. Even all config/m68k*.h should be monitored for changes. Then mint.h will have to be updated accordingly. Essential task, but not really complicated. Doing so systematically, we will be almost sure that everything stays fine. Furthermore, for each setting, I wrote very simple testcases in dedicated GitHub Issues to check the effects. So for any new release, it will be easy to check that the produced code stays as expected.

Do you think we should go that way?

Yes, i think so. IMHO it does not make sense to have m68kelf.h override settings from m68k.h, just to override them again in mint.h (sometimes essentially reverting them to the m68k.h version).

OK, so I'm going to continue that way. I will check that everything remains unchanged, then I will push the patch. I already verified the resulting defines with the nice -E -dM option, and that looks good.

Ideally, after cleanup, it would be nice if it is mostly identical to my own version (https://github.com/th-otto/m68k-atari-mint-gcc/blob/mint/gcc-13/gcc/config/m68k/mint.h), except for the a.out/dbx stuff of course.

Yes. I will progress step by step to avoid regressions, but finally we will tidy the contents of mint.h to ease diff/upgrades. Really, that mint.h is the major part of the gcc patch. By keeping it clean, tidy and documented, future upgrades and variants should be a piece of cake, without surprises.

vinriviere commented 11 months ago

OK, so I'm going to continue that way.

Done: https://github.com/freemint/m68k-atari-mint-gcc/commit/a1550270e2506c1462a57a7f48f832776f93a681 mint.h doesn't use or need m68kelf.h anymore.

Then I also started to reorder mint.h, keeping assembler stuff at top, then C stuff. https://github.com/freemint/m68k-atari-mint-gcc/commit/452473b84cb6b821579b44abf451d5dc39f98e5c

I carefully checked all the resulting defines with -E -dM. Functionally, these refactorings don't change anything. But now, mint.h is more readable and maintainable.

th-otto commented 11 months ago

Then I also started to reorder mint.h, keeping assembler stuff at top, then C stuff

Was that really neccessary? Yes, i agree it looks better, but there was a reason why i wanted the final version to look as close as possible than mine. With those changes, they are now impossible to compare. And i have to maintain 8 different major versions, not only one.

BTW, you also removed the definition of PCC_BITFIELD_TYPE_MATTERS, which is not defined elsewhere. Was that by purpose?

vinriviere commented 11 months ago

Then I also started to reorder mint.h, keeping assembler stuff at top, then C stuff

Was that really neccessary? Yes, i agree it looks better, but there was a reason why i wanted the final version to look as close as possible than mine. With those changes, they are now impossible to compare. And i have to maintain 8 different major versions, not only one.

This was an attempt to make mint.h more readable. If this doesn't fit your needs, feel free to reorder it. What matters for me is that the resulting settings stay the same.

Note that, except the PCC_BITFIELD_TYPE_MATTERS question which is still opened, I consider that the work in mint.h is almost finished. After long investigation and testcases, we understood almost everything. I don't plan to revolutionize mint.h again.

BTW, you also removed the definition of PCC_BITFIELD_TYPE_MATTERS, which is not defined elsewhere. Was that by purpose?

Yes, I removed PCC_BITFIELD_TYPE_MATTERS from mint.h because the default value was already the same. It is set to 1 in elfos.h: https://github.com/freemint/m68k-atari-mint-gcc/blob/gcc-13-mintelf/gcc/config/elfos.h#L61-L65 So this doesn't change the result.

Note: I remember well that we have an opened discussion about PCC_BITFIELD_TYPE_MATTERS in https://github.com/freemint/m68k-atari-mint-gcc/issues/21. I plan to do more investigation there, but that's another task. Here I just focus on mint.h refactorings, which should not affect the settings.

In my initial mintelf release, I had defined PCC_BITFIELD_TYPE_MATTERS to 1. As we haven't agreed (yet) to a definitive answer for that setting, I never changed it. As it is already defined to 1 in elfos.h, in my last commit I just removed it from mint.h. That's functionally equivalent. At least for my standard mintelf build.

@th-otto It's hard for me to see all the implications for your many custom builds. So if that doesn't fit your needs, feel free to reorganize mint.h for your needs, as long as it doesn't change the settings for the standard build.

mikrosk commented 10 months ago

Again, things mentioned in this task were either solved or moved into separate issues, so closing.