Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating past GCC 6.3? #103

Open
asiekierka opened this issue May 16, 2022 · 22 comments
Open

Updating past GCC 6.3? #103

asiekierka opened this issue May 16, 2022 · 22 comments

Comments

@asiekierka
Copy link

asiekierka commented May 16, 2022

I'm curious if there's a specific reason as to which the repository is frozen at GCC 6.3.0. I can see that the Git history would take substantial effort to integrate, but is there a reason to stick with 6.3.0 - or 6.x in general?

(FWIW, the same question applies to binutils, which I wouldn't mind tackling next.)

To find out how much effort it would take to port gcc-ia16 patches to GCC 12.1.x, I've decided to track the releases/gcc-12 branch on another repo and port over the changes:

https://github.com/WonderfulToolchain/gcc-ia16/tree/gcc-ia16-12
https://github.com/asiekierka/build-ia16/tree/toolchain-update-2022

I got stuck on a C++ error in calls.cc, and I have to get some sleep so I can't debug it further today... There's some caveats, too:

  • the big set of testsuite patches (commit ID 9ac0ad6) was not merged, as GCC itself made many patches to better support 16-bit targets in its testsuite since then.
  • "IA-16: fix: internal compiler error on _fmemset-like loop on -O3 " (commit ID 97ecb12) is also not entirely adapted, as I wasn't sure how to do it yet.
  • The HOST_WIDE_INT -> poly_int64 migration is not handled very well. I've mostly tried to get it to compile. It might be possible to use the features to clean up some target code going forward, see https://gcc.gnu.org/onlinedocs/gccint/Overview-of-poly_005fint.html and the following added protos:
extern poly_int64       ia16_push_rounding (poly_int64);
extern bool             ia16_exit_ignore_stack (void);
extern poly_int64       ia16_regmode_natural_size (machine_mode);
extern bool             ia16_regno_nregs_has_padding (unsigned int, machine_mode);
extern unsigned int     ia16_regno_nregs_with_padding (unsigned int, machine_mode);

Either way, I think the repo represents a good start/first look at the feasibility of upgrading the toolchain.

@LowLevelMahn
Copy link

great you try to do it

-is a jump from 6.3 to 12 not a bit too much/far? so many changes over the years
-did you see my post about rebasing the current repo (which is based on the old git gcc-mirror)
to the new official gcc git repo?

#46 (comment)

@asiekierka
Copy link
Author

Is a jump from 6.3 to 12 not a bit too much/far? so many changes over the years

I'm trying to go straight to 12 and seeing what happens. If it doesn't work, I can try porting to an earlier version first. I checked with friends, and all the major deprecations between 6.x and 12.x seem to not affect the ia16 target's code, so there was no reason to try.

Did you see my post about rebasing the current repo (which is based on the old git gcc-mirror)
to the new official gcc git repo?

I did not, but it seems gcc-mirror/git's newer branches (like releases/gcc-12) actually track GCC's official commits, instead of having their own distinct commit history. Therefore, I might have accidentally taken care of that too.

@asiekierka
Copy link
Author

asiekierka commented May 17, 2022

I've fixed the above-mentioned C++ error (it was the poly_int64 migration), and fixed almost all errors in non-target and target code. I'll look at it again some time later, probably later today or tomorrow.

@asiekierka
Copy link
Author

asiekierka commented May 17, 2022

Target: ia16-elf
Thread model: single
Supported LTO compression algorithms: zlib zstd
gcc version 12.1.1 20220516 (GCC)

It compiles! But it doesn't compile much of anything. But it's a start.

The problem seems to be that:

  • (a) GCC 12 makes supporting debug output mandatory, and we have nothing for that, so any call with -g will currently ICE;
  • (b) anything above -O1 will ICE for a separate reason.

I managed to compile the C examples for the Wonderful toolchain I develop with -O0, and they work fine, so it's a good start. It seems that the compiler has gotten slightly better at register allocation and not emitting unnecessary instructions even without optimization passes enabled.

@asiekierka
Copy link
Author

asiekierka commented May 17, 2022

Reverting 6a33438 's override of ia16_can_eliminate seems to fix compilation on -O1 and -O2, at least on my own (simple, C) code. But it's probably not the correct solution, and I'm not sure how much I should spend on it from here, so...

@tkchia , would you like to take this from here? I'm not sure what to do from here (work on it more, move on to another one of my bucket list projects, etc.) without further coordination.

@tkchia
Copy link
Owner

tkchia commented May 17, 2022

Hello @asiekierka,

I am kind of occupied with working on #104 — but I hope to get around to what you are doing.

For now, are you able to start running the regression tests (./build.sh test) on your end? If you can, then this will at least check that the compiler does not silently generate incorrect code, which is the huge no-no.

Thank you!

@asiekierka
Copy link
Author

asiekierka commented May 17, 2022

I cannot run ./build.sh test as it complains about missing the runtest program.

The GCC testsuite will need work - I did not merge reenigne's big testsuite patch commit as a lot of it was focused on fixes for platforms which do not assume a 32-bit int - but fixes for the same issue were done independently and in a different way by GCC's developers sometime between 6.3.0 and 12.1.0, leading to a very conflict-heavy situation.

I'll need to brush up on a lot of GCC theory to figure out what the correct patches should be for anything. Right now, it fails at building libgcc. :D

Thank you, and I hope we can get gcc-ia16 up to date soon!

EDIT: I fixed compiling with -g and the libgcc makefile, but now I'm going to be falling into libgcc ICEs :(

@tkchia
Copy link
Owner

tkchia commented May 17, 2022

Hello @asiekierka,

The runtest program comes from the DejaGnu test framework, which you will need to install (e.g. sudo apt-get install dejagnu). Thank you!

@asiekierka
Copy link
Author

asiekierka commented May 17, 2022

Given that ./build.sh test expects a stage2 compiler, and I'm crashing at libgcc, let alone newlib, running the full testsuite seems unlikely yet. For version updates, it might be good to add a stage1 test variant.

However, running make check-gcc RUNTESTFLAGS="--target_board=86sim" in the stage 1 compiler is a good start.

		=== gcc Summary ===

# of expected passes		18826
# of unexpected failures	6819
# of unexpected successes	3
# of expected failures		137
# of unresolved testcases	5214
# of unsupported tests		1053

NOTE: This is with ia16_can_eliminate no-oped out. Otherwise it crashes on any program in -O1/-O2/-Os/etc..

It's just a "barely got it to compile" test, so it's not looking great. Many of these failures are because of an ICE in libgcc. Many others are because of the same few issues (new unimplemented insns, etc). Some tests need to be re-adapted for ia16 (see original post for details).

@turol
Copy link

turol commented May 18, 2022

There is a tool called git-imerge which can do merges and rebases "commit by commit" to handle complex conflicts. It is however pretty slow and for large histories doing all the manual merges can take quite a while. Also you mentioned something about histories, if the GCC 6 history for this project and GCC 12 history haven't been made with the same tool you're probably in for some interesting times.

@LowLevelMahn
Copy link

LowLevelMahn commented May 18, 2022

It is however pretty slow and for large histories doing all the manual merges can take quite a while.

there a least thousands of commit between :)

but this presentation of imerge is really interesting: https://www.youtube.com/watch?v=FMZ2_-Ny_zc
especially the find the next conflicting commit feature

at ~18:05 he is talking about how each merge step can be tested in between if the merge result runs fine
https://youtu.be/FMZ2_-Ny_zc?t=1082

could take some time but maybe its a good solution
(maybe the author is interested in guiding the process)

funny that there is no git intergrated feature like that

@asiekierka
Copy link
Author

I've already done the "commit by commit" process by hand. I skipped two commits because:

  • the first one (testsuite fixups) was incompatibly replicated by GCC developers due work on the inclusion of the MSP430 backend, and besides it's always a good idea to re-review your testsuite patches;
  • the second one (-O3 ICE patch) is particularly distinct and can be reintroduced later, when things are more reliable in other places.

@turol
Copy link

turol commented May 18, 2022

imerge might also allow you to find the problematic commits on GCC side so you can see exactly what broke ours. Though that might require an ability (and willingess) to run the whole GCC build and test process on every commit. I'm not sure that's implemented yet and and suspect it would take an insanely long time.

@asiekierka
Copy link
Author

asiekierka commented May 18, 2022

Nono. I merged the ia16 port commits to the GCC 12 tree, not the GCC 12 commits to the ia16 tree. The latter would be nearly impossible, involve literally many thousands of commits, and two repos with incompatible root commits and history trees.

So no, this approach won't help. I firmly believe it's better to figure it out this way - and try to keep more up to speed on GCC updates in the future :)

@tkchia
Copy link
Owner

tkchia commented May 18, 2022

Hello @asiekierka,

  • (a) GCC 12 makes supporting debug output mandatory, and we have nothing for that, so any call with -g will currently ICE;

Urgh. The GNU toolchain generally knows how to output debug information (e.g. DWARF) in ELF files. The problem is that I have not quite figured out how to propagate this debug information to the output executables (which are normally not ELF), and it is probably not good to erroneously pretend that there is debug output. But it looks like we will need to undo this tricky knot somehow or other. 😐

Given that ./build.sh test expects a stage2 compiler, and I'm crashing at libgcc, let alone newlib, running the full testsuite seems unlikely yet. For version updates, it might be good to add a stage1 test variant.

That will be kind of hard. Many of the test cases expect a toolchain that can go the full nine yards and output an executable program. This can only be done if there is an actual C library, in this case Newlib.

I guess it is a good idea for now to look further into the libgcc crashes first. I figure that you have some idea of where in the gcc-ia16 code the crashes are happening? If it helps, you can try adding calls to debug_tree (·) or debug_rtx (·) in the GCC code, to dump out the contents of any relevant tree structures and/or rtx structures respectively.

Thank you!

@asiekierka
Copy link
Author

asiekierka commented May 18, 2022

I figure that you have some idea of where in the gcc-ia16 code the crashes are happening?

Yes, it's emitting ICEs. However, I'm not sure when I'll be able to look into them particularly deeply - I'll try to do so as soon as possible.

In the meantime, the Binutils 2.34 -> 2.38 update I made seems to be more reliable (due to fewer breaking changes) and actually removes one of the hacks made by the ia16 (segelf) patches, so if you want to, you can look into that too.

PS. I managed to undo the debug output requirement. So that's not a worry for now.

@asiekierka
Copy link
Author

asiekierka commented May 18, 2022

Two ICE patches made so far:

  • alias.cc:init_alias_analysis () hardcodes a %bp -> %sp elimination. Do not allow it for now.
  • Renaming the "simple_return" insn and expand to "ia16_return"; I'm not sure if this is correct, but it fixes an JUMP_LABEL (insn) == x assertion in jump.cc:mark_jump_label_1 (...); otherwise, x is often (return), but JUMP_LABEL (insn) is (simple_return).

Results look a little more promising (I'm running the tests multithreaded):

# of expected passes		20571
# of unexpected failures	196
# of unexpected successes	1
# of expected failures		88
# of unresolved testcases	159
# of unsupported tests		578

# of expected passes		14013
# of unexpected failures	146
# of expected failures		70
# of unresolved testcases	162
# of unsupported tests		797

# of expected passes		16420
# of unexpected failures	336
# of unexpected successes	3
# of expected failures		167
# of unresolved testcases	264
# of unsupported tests		504

# of expected passes		7437
# of unexpected failures	974
# of expected failures		26
# of unresolved testcases	188
# of unsupported tests		485

# of expected passes		9858
# of unexpected failures	982
# of unexpected successes	3
# of expected failures		69
# of unresolved testcases	11
# of unsupported tests		440

# of expected passes		16798
# of unexpected failures	462
# of expected failures		85
# of unresolved testcases	317
# of unsupported tests		674

# of expected passes		19836
# of unexpected failures	711
# of expected failures		141
# of unresolved testcases	326
# of unsupported tests		645

# of expected passes		18956
# of unexpected failures	298
# of expected failures		388
# of unresolved testcases	192
# of unsupported tests		929

Just to keep notes, the next issue is:

Unknown cost for rtx with code vec_select and outer code set:
(vec_select:QI (mem:V2QI (reg/f:HI 406 [ _2005 ]) [1 MEM <vector(2) unsigned char> [(struct insn_t *)_668]+0 S2 A16])
    (parallel [
            (const_int 1 [0x1])
        ]))
Unknown cost for rtx with code parallel and outer code vec_select:
(parallel [
        (const_int 1 [0x1])
    ])

on gcc.c-torture/compile/920625-1.c with -O2.

@asiekierka
Copy link
Author

asiekierka commented May 19, 2022

I fixed the above issue, as well as two ICEs that manifested in the test suite. Further comments available in commits, but here's a summary:

  • The vec_select and parallel fix was just adding more return false; cases to ia16_rtx_costs.
  • There was an ICE in record_common_node caused by turning va_list into an union on the ia16 target - it wasn't expecting an union to be fed to it.
  • There was an ICE in gen_reg_rtx when compiled with -mno-assume-ss-data - this is because while GCC 6 called ia16_lra_p in the main lra () function, GCC 12 instead caches the result in init_lra_once () - but this is called before init_emit (), causing crtl->emit.regno_pointer_align_length to be uninitialized. The tentative fix is to restore GCC 6's behaviour, as it seems to be meant as an optimization and lower performance is preferable to ICE - but a better one should be implemented eventually.

Unfortunately, there are a fair few execution errors, and I'm not yet sure how to debug these - and I'd like to spend some time working on the library part of my WonderSwan toolchain too, so I'm not sure how much time I'll be able to devote to the GCC 12 update...

Either way, libgcc compiles fully now, and Newlib gets pretty far before hitting a different ICE.

I'd say it would be good to think about deciding when a GCC 12 branch could be adopted in place of the GCC 6.3 branch for new development.

Thank you!

@asiekierka
Copy link
Author

Given the lack of time and expertise to work on a larger update, I decided to try something with a smaller surface first:

  • (1) rebase the GCC 6.3 branch on gcc-mirror/gcc, see here,
  • (2) update the GCC 6.3 branch to at least GCC 6.5, see here.

Due to the smaller change surface, this one naturally fares much better, only ICEing at the ./build.sh libi86 step of the build-ia16 compilation process (using my Binutils 2.39 fork and my GCC 6.5.0 fork) for any optimization value >= -O1:

$ ia16-elf-gcc  -mregparmcall  -I. -I../../../libi86/host-gcc -I../../../libi86/host-gcc/../common -U_BORLANDC_SOURCE -O2 -mseparate-code-segment -Wall -Werror=strict-prototypes -Werror=missing-prototypes -Werror=unused -Werror=deprecated -Werror=uninitialized -Werror=maybe-uninitialized  -mregparmcall  -c -o dos/dos-creatnew.o ../../../libi86/host-gcc/dos/dos-creatnew.c
../../../libi86/host-gcc/dos/dos-creatnew.c: In function ‘_dos_creatnew’:
../../../libi86/host-gcc/dos/dos-creatnew.c:51:1: internal compiler error: Max. number of generated reload insns per insn is achieved (90)

 }

No further updates for now, unfortunately.

@Serentty
Copy link

Any updates on this? In particular, if it were possible to use GCC-IA16 with LIBGCCJIT, it would open up a lot of possibilities, such as the Rust GCC backend, and I am fairly sure that requires a newer version of GCC.

@lpsantil
Copy link

Is there a reason you need a 16-bit Rust? DJGPP (32-bit GCC) is available for 12.1. I documented the process for building on Fedora 36 here (andrewwutw/build-djgpp#45).

@Serentty
Copy link

I have gotten Rust to target a 386 before, but yeah, I recently acquired an IBM 5155, which has an 8088, and developing for it is something I’m interested in these days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants