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

Add Crystal support #2732

Merged
merged 10 commits into from Jul 7, 2021
Merged

Add Crystal support #2732

merged 10 commits into from Jul 7, 2021

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Jun 23, 2021

Resolves #673. Includes compiler versions from 1.0 down to 0.29.

The Monaco language is written from scratch and supports only basic highlighting; Crystal's syntax is borrowed from Ruby, but there are too many dissimilarities between the two and I'd prefer lack of e.g. indenting rules over incorrect ones. It is subject to future improvements.

Even an empty file produces 190k lines of assembly code, unless --prelude=empty is provided (which more or less excludes the entire runtime and all libraries). If this is unacceptable then arrangements would have to be made on the language side.

Related: compiler-explorer/infra#533

Copy link
Contributor

@partouf partouf left a comment

Choose a reason for hiding this comment

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

Thanks so much for doing this! Looks good

static/modes/crystal-mode.js Outdated Show resolved Hide resolved
lib/compilers/crystal.js Outdated Show resolved Hide resolved
@partouf
Copy link
Contributor

partouf commented Jun 23, 2021

Some issues that need to be resolved before this:
When I install one of the versions (0.29.0 for example) and try to run a simple example, I get these errors:

partouf@ceub20:/tmp$ /opt/compiler-explorer/crystal-0.29.0/bin/crystal build -o output.s --emit asm hello.cr
/usr/bin/ld: cannot find -lpcre (this usually means you need to install the development package for libpcre)
/usr/bin/ld: cannot find -levent (this usually means you need to install the development package for libevent)
collect2: error: ld returned 1 exit status
Error: execution of command failed with code: 1: `cc "${@}" -o '/tmp/output.s'  -rdynamic  -lpcre /opt/compiler-explorer/crystal-0.29.0/bin/../lib/crystal/lib/libgc.a -lpthread /opt/compiler-explorer/crystal-0.29.0/share/crystal/src/ext/libcrystal.a -levent -lrt -ldl -L/opt/compiler-explorer/crystal-0.29.0/bin/../lib/crystal/lib -L/usr/lib -L/usr/local/lib`

When running 1.0.0 I get less errors, but errors nonetheless:

partouf@ceub20:/tmp$ /opt/compiler-explorer/crystal-1.0.0/bin/crystal build -o output.s --emit asm hello.cr
/usr/bin/ld: cannot find -lpcre (this usually means you need to install the development package for libpcre)
collect2: error: ld returned 1 exit status
Error: execution of command failed with code: 1: `cc "${@}" -o /tmp/output.s  -rdynamic -L/opt/compiler-explorer/crystal-1.0.0/bin/../lib/crystal/lib -lpcre -lm -lgc -lpthread /opt/compiler-explorer/crystal-1.0.0/share/crystal/src/ext/libcrystal.a -levent -lrt -ldl`

It seems to me crystal requires a C compiler in the path, that should be possible with the default Ubuntu 20.04 gcc compiler (though not sure if that's what we want always) - but the main problem lies in the non-standard prerequisite libraries (through the OS package system).

So my questions would be:

  • Is there a list of these requirements per versions somewhere?
  • Can we make it less dependent on cc and -L/usr/lib -L/usr/local/lib? - although I'm guessing maybe 1.0.0 has a different library resolving mechanism

@partouf
Copy link
Contributor

partouf commented Jun 23, 2021

It seems there's this list
https://github.com/crystal-lang/crystal/wiki/All-required-libraries

It also seems to require an LLVM to be installed?

And the latest documentation seems to refer to some environment variables for the build command:
https://crystal-lang.org/reference/using_the_compiler/index.html#environment-variables

But I don't know if these are also used in the earlier versions.

@partouf
Copy link
Contributor

partouf commented Jun 23, 2021

Note that the biggest issue here would probably requiring library installs on the OS level, as we would need to produce new images for our instances. We can probably solve the CC and LLD requirements some other way.

cc @mattgodbolt

@HertzDevil
Copy link
Contributor Author

cc can be overridden through the CC environment variable, and library search paths are prepended by CRYSTAL_LIBRARY_PATH. The required libraries have not been changed from that list you found for the Crystal versions added in this PR.

LLVM shouldn't be needed just for running Crystal. It definitely needs to be there when building Crystal from Crystal itself (something to consider for nightlies).

@straight-shoota
Copy link

It seems to me crystal requires a C compiler in the path

Crystal just requires a linker. It picks up what's configured as CC (and defaults to cc). We can use whichever fits for this purpose. The compiler also allows passing flags via --link-flags CLI option.

Is there a list of these requirements per versions somewhere?

So https://github.com/crystal-lang/crystal/wiki/All-required-libraries is a list of all libraries required for the entire standard library and compiler. If you include the XML module, you need libxml2 for example.
I assume it's probably best to have all stlib dependencies available. Not sure about the compiler dependencies (the difference is only LLVM, though).

It also seems to require an LLVM to be installed?

LLVM is linked statically into the compiler binary from the tarball. Unless building the compiler itself, there is no need to have LLVM installed.

And the latest documentation seems to refer to some environment variables for the build command:
crystal-lang.org/reference/using_the_compiler/index.html#environment-variables

But I don't know if these are also used in the earlier versions.

They should be available in earlier versions, but probably not all back in 0.29. I'm not sure if there's much point in going so far back, anyways. IMO it would be fine to focus on 1.0, at least for a first iteration.

@HertzDevil
Copy link
Contributor Author

HertzDevil commented Jun 23, 2021

The one I mentioned, CRYSTAL_LIBRARY_PATH, seems to have existed since 0.28.0.

Perhaps we could build the libraries locally with https://github.com/compiler-explorer/infra/blob/main/update_compilers/install_libraries.sh and symlink to them from crystal-{name}/lib/crystal/lib? Theoretically we could even offer the same libraries for C/C++.

Also added an assembly parser that handles Crystal's quoted labels. Filtering everything reduces the number of lines to roughly 13k (the Crystal prelude's own included files are counted as library code).

@HertzDevil
Copy link
Contributor Author

Will this work?

],

surroundingPairs: [
{ open: '{', close: '}' },

Choose a reason for hiding this comment

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

Suggested change
{ open: '{', close: '}' },
{ open: '{', close: '}' },
{ open: '{%', close: '%}' },

Not sure I'm reading this correctly, but I think macro block identifiers are missing.

Copy link
Member

@RubenRBS RubenRBS left a comment

Choose a reason for hiding this comment

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

Thank you so much for this!

Copy link
Member

@RubenRBS RubenRBS left a comment

Choose a reason for hiding this comment

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

Forgot to actually send the review

etc/config/crystal.amazon.properties Outdated Show resolved Hide resolved
@partouf
Copy link
Contributor

partouf commented Jun 27, 2021

Superweird asm emission, but c'est la vie.

The 1 thing I'm worried about is that the square function example is unused and is not emitted until you call the function.

@partouf
Copy link
Contributor

partouf commented Jun 29, 2021

Superweird asm emission, but c'est la vie.

The 1 thing I'm worried about is that the square function example is unused and is not emitted until you call the function.

Any thoughts about the last one? This is different behaviour compared to all the other languages and compilers.
There doesn't seem to be an extern/export kind of option (at least not what I can find in the language documentation)

Perhaps an option would be to get an input parameter from the command line and give that to the function, so that it's always used?

@straight-shoota
Copy link

Export function would be fun instead of def. But then it can only use primitive types as parameters. Int32 works, for example.

fun square(num : Int32) : Int32
  num * num
end

But I don't think that would be very helpful for general use cases.

The general approach to include a Crystal method in codegen is adding a call to it.

@partouf
Copy link
Contributor

partouf commented Jun 29, 2021

Export function would be fun instead of def. But then it can only use primitive types as parameters. Int32 works, for example.

fun square(num : Int32) : Int32
  num * num
end

But I don't think that would be very helpful for general use cases.

The general approach to include a Crystal method in codegen is adding a call to it.

Any idea on how to do that without specifying a specific number? I saw you can override the main() function, but doubt that's a recommended thing to do

@straight-shoota
Copy link

straight-shoota commented Jun 29, 2021

Sorry, I actually have no idea how compiler explorer works and what we're aiming for here. If the method is to be called without a baked-in value, the value needs to come from somewhere like a CLI arg, env or rand.

There shouldn't be a reason to override main, because everything in the top level scope is executed as main body.

@partouf
Copy link
Contributor

partouf commented Jun 29, 2021

Sorry, I actually have no idea how compiler explorer works and what we're aiming for here. If the method is to be called without a baked-in value, the value needs to come from somewhere like a CLI arg, env or rand.

There shouldn't be a reason to override main, because everything in the top level scope is executed as main body.

I didn't mean how to get the value from CE, that's already supported for cmdline args and stdin.

My question was about how to write such an example in Crystal that uses it.

@partouf
Copy link
Contributor

partouf commented Jul 2, 2021

Sorry, I actually have no idea how compiler explorer works and what we're aiming for here. If the method is to be called without a baked-in value, the value needs to come from somewhere like a CLI arg, env or rand.
There shouldn't be a reason to override main, because everything in the top level scope is executed as main body.

I didn't mean how to get the value from CE, that's already supported for cmdline args and stdin.

My question was about how to write such an example in Crystal that uses it.

@HertzDevil can you create such an example? (something that uses cmdline input and calls square with it)

@mattgodbolt
Copy link
Member

Catching up here, agree with everything @partouf has said. The intention of the default example is to show a little simple assembly, conventionally the "square a number" routine. It's not meant to be a language primer; is there a reason the fun isn't OK? We can have named examples (accessible from the load/save button) that show off the language more, if that's what you want.

@partouf
Copy link
Contributor

partouf commented Jul 6, 2021

Catching up here, agree with everything @partouf has said. The intention of the default example is to show a little simple assembly, conventionally the "square a number" routine. It's not meant to be a language primer; is there a reason the fun isn't OK? We can have named examples (accessible from the load/save button) that show off the language more, if that's what you want.

The problem is that the square fun without a call isn't emitting Any assembly, so there would be nothing but overhead showing.
If it would emit something that resembled c=a*b, then you would be absolutely right, but it doesn't.

So I think this is the way to go.

I have tested the rest, so this is good to merge and go live.

@straight-shoota
Copy link

The intention of the default example is to show a little simple assembly,

In that case, maybe it would be an option to run the example without stdlib. That would keep the generated assembly to a minimum and more strictly represent the language itself instead of the stdlib. @HertzDevil WDYT?
It might be difficult to achieve because this requires a different compiler arg (ˋ--prelude=emptyˋ) than building any "normal" Crystal code which would usually include the stdlib.

@HertzDevil
Copy link
Contributor Author

I think all funs will emit code even if they aren't "instantiated" like defs. I'll try to come up with an example a few hours later.

Is it possible to pre-populate the compiler args when the default snippet is loaded?

@partouf
Copy link
Contributor

partouf commented Jul 6, 2021

I think all funs will emit code even if they aren't "instantiated" like defs. I'll try to come up with an example a few hours later.

Ah, I think I missed the point of changing def to fun. I don't remember if I tested that.

Is it possible to pre-populate the compiler args when the default snippet is loaded?

Not afaik. We've talked about it in various circumstances, but it's just not worth the trouble imo.

@straight-shoota
Copy link

Yes, fun Methode are exported always. But with default compiler behaviour, there's still a lot of additional code for runtime initialization.

@HertzDevil
Copy link
Contributor Author

HertzDevil commented Jul 6, 2021

An absolute minimal example for that would be:

# compile with --prelude=empty

fun square(num : Int32) : Int32
  num &* num
end

In this case the ASM output would just be:

square:
        imull   %edi, %edi
        movl    %edi, %eax
        retq

".L'dd40a2442'":
        .long   1
        .long   9
        .long   9
        .asciz  "dd40a2442"
        .zero   2

; ...

Does this look fine?

@partouf
Copy link
Contributor

partouf commented Jul 6, 2021

An absolute minimal example for that would be:

# compile with --prelude=empty

fun square(num : Int32) : Int32
  num &* num
end

In this case the ASM output would just be:

square:
        imull   %edi, %edi
        movl    %edi, %eax
        retq

".L'dd40a2442'":
        .long   1
        .long   9
        .long   9
        .asciz  "dd40a2442"
        .zero   2

; ...

Does this look fine?

absolutely, that's fine

@HertzDevil HertzDevil requested a review from RubenRBS July 7, 2021 09:06
@partouf partouf merged commit 94ecb09 into compiler-explorer:main Jul 7, 2021
@HertzDevil HertzDevil deleted the crystal branch July 7, 2021 22:29
mattgodbolt pushed a commit that referenced this pull request Aug 2, 2021
* Add Crystal support

* Fix copyright

* Add ASM parser for Crystal

* Add supportsLibraryCodeFilter

* Update crystal-mode.js

* use baseName

* Force `square` call

* Update default snippet

Co-authored-by: Patrick Quist <partouf@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Crystal support
6 participants