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

Added x64 assembly compilation #940

Merged
merged 6 commits into from
Nov 28, 2021

Conversation

Amaras
Copy link
Member

@Amaras Amaras commented Nov 22, 2021

And here we go again with X64 Assembly.

There was a little annoyance this time: the way we programmed the assembly required the compilation to use the link flag -no-pie. As such, to prevent environment duplication, I elected to add the link flag to all the executables.

Apart from this, compilation was not affected, and all executables compile successfully on my Linux machine.

We really should set up a Windows environment to check whether it works as intended there, though.

SConstruct Outdated Show resolved Hide resolved
@ShadowMitia
Copy link
Contributor

ShadowMitia commented Nov 22, 2021

I have executables that segfault on my machine, the ones that do run:

  • cooley_tukey
  • monte_carlo_integration
  • verlet_integration

Let me know if you want me to open an issue, or if you want more info.

@Amaras
Copy link
Member Author

Amaras commented Nov 22, 2021

The only segfault I could reproduce on my machine is for forward_euler_method, which does so in __printf, probably because of a bad formatting in results_loop.

I will need more detail for the others, and it is probably not my responsibility. I had already noticed the segfault in some of the x64 implementations, but did not take the time to report them, I think.

Copy link
Contributor

@ShadowMitia ShadowMitia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might've found a way to pass it only to specific languages.
I found the information here: https://scons.org/doc/production/HTML/scons-man.html#f-ParseFlags, basically you can use -Wl, to ass stuff to the linker.
It might not be very cross-platform though... Should support gcc/clang at least.

contents/cooley_tukey/code/asm-x64/SConscript Outdated Show resolved Hide resolved
contents/forward_euler_method/code/asm-x64/SConscript Outdated Show resolved Hide resolved
sconscripts/asm-x64_SConscript Outdated Show resolved Hide resolved
@ShadowMitia
Copy link
Contributor

The only segfault I could reproduce on my machine is for forward_euler_method, which does so in __printf, probably because of a bad formatting in results_loop.

I will need more detail for the others, and it is probably not my responsibility. I had already noticed the segfault in some of the x64 implementations, but did not take the time to report them, I think.

I've run some quick tests, it does seem to also be coming from printf formatting as well on my end, but it happens on more examples. I'll open an issue for it.

Amaras and others added 2 commits November 23, 2021 00:12
Co-authored-by: Dimitri Belopopsky <ShadowMitia@users.noreply.github.com>
@Amaras
Copy link
Member Author

Amaras commented Nov 22, 2021

It might not be very cross-platform though... Should support gcc/clang at least.

We basically don't matter care if it's cross platform at this point, do we? We already forced the use of gcc, gnulink, next will be g++ and the GNU X86_64 assembler.

@ShadowMitia
Copy link
Contributor

For now I think you're right

1. Using the `LINKFLAGS=''` instead of the `parse_flags` argument helped with a
`-lgcc_s not found` error that prevented linking.

2. The simplification was to only use the necessary tools (not the default toolset).
This should hopefully be clearer than the previous iterations.

All the executables compile correctly in the container.

TODO: check that compilation works on Windows.
@Amaras
Copy link
Member Author

Amaras commented Nov 27, 2021

Alright, @PeanutbutterWarrior and I decided on Discord here to give up on trying to build x86-64 assembly on Windows.
The linker seemed to outright ignore the -no-pie option, as well as --no-pie, -static and --static.

Commit 12350c5 fixed a bug in @ShadowMitia's proposal that made it impossible to link correctly. However, the expected behaviour was achieved by using the LINKFLAGS argument in the env.X64 calls. (The commit also simplifies the SConstruct so that's always a plus).

As such, I think this PR is ready to merge, just possibly waiting for the approval of another maintainer.

Copy link
Member

@leios leios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Thanks for the work here!

@Amaras Amaras merged commit 9d9e1cb into algorithm-archivists:master Nov 28, 2021
@Amaras Amaras deleted the asm-x64_compilation branch November 28, 2021 14:35
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.

3 participants