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

BUSY build of frontend on MSVC 2015, Clang 4 and GCC 12 #1027

Merged
merged 4 commits into from
May 5, 2024

Conversation

rochus-keller
Copy link
Contributor

It compiles and runs on

  • Windows 11 x64 with MSVC 2015 x86
  • Debian 12 x86 with GCC 12 x86, runs also on Ubuntu 14.4
  • Ubuntu 14.4 x86 with Clang 4.0.1 and libc++ (compile only)

I added two defines:

  • ORANGE_NO_MSIL: when defined there is no PeLib dependency and the functions return an error or do nothing
  • ORANGE_NO_INASM: when defined there is no oasm dependency and the functions return an error or do nothing
    If not defined, the code behaves as normal.

I used my LeanCreator to build on each platform, but also the command line version of BUSY will do (though only on one core).

Since I now have a working version on my old Linux development machine I can start to check out what the frontend can do and how the output looks.

The build actually only depends on occparse, ocpp, occopt and util (with a few headers from occ), but I think it's more beneficial to have the changes upstream if possible.

@rochus-keller rochus-keller changed the title changes to BUSY build frontend on MSVC 2015, Clang 4 and GCC 12 BUSY build of frontend on MSVC 2015, Clang 4 and GCC 12 May 3, 2024
@rochus-keller
Copy link
Contributor Author

rochus-keller commented May 3, 2024

Meanwhile I was also able to compile and run on Debian 11 x64 with GCC 10.

All built Linux executables fail in ToolChain::StandardToolStartup when calling SwitchParser.Parse, which returns false and causes the app to exit via ToolChain::Usage. I don't yet fully understand the logic, but in any case the behaviour differs between Linux and Windows.

On Windows I can compile with /Y Hello.c and get a Hello.icf and Hello.icd, the latter of which looks decently.

Unfortunately the binary compiled on Debian 12 x86 makes a segmentation fault on Ubuntu 14.4 as soon as I give it a file to compile (so it behaves differently from Debian 12). I was successfull with this approach with other applications I could not directly compile on my old Ubuntu 14.4, so I hoped it would also work with OrangeC. Interestingly, the crash does not materialize if I run the app with GDB.

This will keep me busy for some time.

@LADSoft
Copy link
Owner

LADSoft commented May 4, 2024

Certainly I have no problem merging such changes, as long as the compiler still works properly when I don't use those defines 😄 As far as the SwitchParser.Parse... there MAY be an involvement with windows variables called '__argc' and '__argv'.... which are the args given to main, only presented as global variables. That was another thing I hadn't figured out what to do with... I will review this after a while, have a couple of things in front of it...

@rochus-keller
Copy link
Contributor Author

as long as the compiler still works properly when I don't use those defines

Sure, this is asserted.

there MAY be an involvement with windows variables called '__argc' and '__argv'

I don't think so; so far rather looks like an assumption on how a file path should look, but I'm still researching.

I also didn't understand yet where the platform dependency comes into play for generating the IR; sure the pointer byte width (and some other byte widths) has to be considered; but is it really necessary to write something like configx86.cpp/h for each supported architecture if I just want to generate IR (which is described as a "generic assembly language" in the documentation)?

@LADSoft
Copy link
Owner

LADSoft commented May 4, 2024

well for the architecture files, you would really only need two or maybe more for what you are doing One for 32 bit, one for 64 bit, maybe 8 and 16 bits if you want to try to support those lol... then you could rewrite the initial parsing of the platform name to choose different config files based on your target or based on a 'bits' setting if you'd rather... as far as actually generating the IR I assume you'll be calling the same functions from all these configurations so that part of the configuration wouldn't have to change, you could put it in a header for example. Some parts of the configuration really wouldn't apply eiether, for example if you are going to use part of the optimizer (I'm not sure what you said on that) it would be silly to enable the register allocation code, as I'm sure the platform you are using takes care of such details...

Really all that configuration stuff SHOULD be done with an object instead of all these structures, and maybe separated out a little better so there could be more sharing and less copy and paste... this code predates the time I was compiling the compiler as C++ source code though sigh....

Oh i don't know if you noticed but we kinda collided on changes to the DotNetPELib stuff.... there is a collision in occparse.cpp and some new code I think in osutil.cpp that you may want to put an ifdef around... if you could make changes to address that I will accept it into the project...

@rochus-keller
Copy link
Contributor Author

Thanks for the clarifications.

I merged your most recent master branch which should have fixed the collision.

@rochus-keller
Copy link
Contributor Author

All built Linux executables fail in ToolChain::StandardToolStartup when calling SwitchParser.Parse

Meanwhile I was able to find out why SwitchParser.Parse returns false; the reason is quite simple; this function assumes that a command line option can start with "/"; but since argv[0] is the path to the executable, the function tries to interpret it as option and fails; so I have to ifdef the argv[0][0] == '/' || so it is only active on Windows and not use "/" as an option signal on Linux.

Unfortunately the binary compiled on Debian 12 x86 makes a segmentation fault on Ubuntu 14.4

I also found the reason for this behaviour. On Ubuntu 14.4 I run the frontend via command line and ./frontend, so it didn't trigger the aforementioned issue, but directly run into the missing SharedMemory.cpp implementation; SharedMemory::GetMapping returns zero which causes the segfault. I will implement it for Linux.

@LADSoft
Copy link
Owner

LADSoft commented May 5, 2024

thanks... i would of course be very interested in your shared memory implementation if you want to contribute it back 😄

@LADSoft LADSoft merged commit 72adf4c into LADSoft:master May 5, 2024
2 of 3 checks passed
@rochus-keller
Copy link
Contributor Author

Thanks for accepting the pull request.

Meanwhile I was able to create a working version which runs on all tested platforms without crash and generating the same icd file from the same Hello.c on all platforms (the icf only seems to differ in the source path name).

So it especially seems to work on my old Ubuntu x86 14.4 Linux, which was my primary goal; now I can start to compile different test C and C++ projects and see what IR is generated.

Here are the changes which were eventually necessary to make it work: rochus-keller@ac0d9a0

As you can see the SharedMemory is a trivial implementation at the moment which actually doesn't share anything. I will add a true shared memory implementation when I will integrate the optimizer; but first I have to check whether I understand your IR and can convert it to the Eigen IR with reasonable effort.

Note that this version seems to also work on Debian 11 x64, not only on x86. Are you interested in another pull request for this version?

@LADSoft
Copy link
Owner

LADSoft commented May 5, 2024

I compile with MINGW64 and then use that version of the compiler to compile itself with, so it isn't such a surprise that works on Debian 11 x64.

By the way don't worry about the size of the IL files I'm doing work to make them smaller... they were getting out of hand when compiling the compiler with itself.

yes i am interested in changes you make which would be of general interest! please make a new pull request :)

@rochus-keller
Copy link
Contributor Author

Great.

I wasn't concerned with the IL size so far, more that I understand what's inside.

The pull request it up.

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

Successfully merging this pull request may close these issues.

2 participants