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.

Sign up for GitHub

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

Support windows linker #4491

Merged
merged 1 commit into from
Jun 3, 2017
Merged

Support windows linker #4491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/compiler/crystal/codegen/link.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,24 @@ module Crystal

class Program
def lib_flags
has_flag?("windows") ? lib_flags_windows : lib_flags_posix
end

private def lib_flags_windows
String.build do |flags|
link_attributes.reverse_each do |attr|
if ldflags = attr.ldflags
flags << " " << ldflags
end

if libname = attr.lib
flags << " " << libname << ".lib"
end
end
end
end

private def lib_flags_posix
library_path = ["/usr/lib", "/usr/local/lib"]
has_pkg_config = nil

Expand Down
43 changes: 32 additions & 11 deletions src/compiler/crystal/compiler.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ module Crystal
# optionally generates an executable.
class Compiler
CC = ENV["CC"]? || "cc"
CL = "cl"
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest renaming CC and CL to something like POSIX_LINKER and WINDOWS_LINKER

Copy link
Contributor

Choose a reason for hiding this comment

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

No, keeping the same name as ENV variables makes it easier to deal with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's a good idea. I want to work out an env var or commandline flag to set the linker, like CC on posix.

Copy link
Member

Choose a reason for hiding this comment

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

Why don't just use LINK https://msdn.microsoft.com/en-us/library/hx5b050y.aspx

Copy link
Contributor Author

@RX14 RX14 May 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The LINK tool prepends the options and arguments defined in the LINK environment variable and appends the options and arguments defined in the _LINK_ environment variable to the command line arguments before processing.

From this wording, it appears to me that the LINK env var is used to prepend options to the link command, not contain the path to the executable.

Copy link
Member

Choose a reason for hiding this comment

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

I mean the link command instead of cl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not use ld instead of cc? I don't know, but I think it'd beyond the scope of this PR. If we change to use the linker directly instead of the compiler directly we should do that on all platforms simultaneously.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bcardiff Because, just like on Unix, calling the linker directly would require you to manually pass the correct C/base libraries to link. On Windows, it's more complicated than Unix; in addition to kernel32.lib, you need to link in the proper C library using its full path, depending on the architecture and debug vs release mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kirbyfan64 you certainly don't need to pass the full path.


# A source to the compiler: it's filename and source code.
record Source,
Expand Down Expand Up @@ -225,9 +226,7 @@ module Crystal
bc_flags_changed
end

private def codegen(program : Program, node, sources, output_filename)
@link_flags = "#{@link_flags} -rdynamic"

private def codegen(program, node : ASTNode, sources, output_filename)
Copy link
Member

Choose a reason for hiding this comment

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

why program has lost it's type restriction : Program?

Copy link
Contributor Author

@RX14 RX14 May 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

It was superfluous and made the line longer. ASTNode is required to distinguish the override, Program is just repeating the information contained in the variable name program.

llvm_modules = @progress_tracker.stage("Codegen (crystal)") do
program.codegen node, debug: debug, single_module: @single_module || @release || @cross_compile || @emit, expose_crystal_main: false
end
Expand All @@ -247,20 +246,18 @@ module Crystal
CompilationUnit.new(self, type_name, llvm_mod, output_dir, bc_flags_changed)
end

lib_flags = program.lib_flags

if @cross_compile
cross_compile program, units, lib_flags, output_filename
cross_compile program, units, output_filename
else
result = codegen program, units, lib_flags, output_filename, output_dir
result = codegen program, units, output_filename, output_dir
end

CacheDir.instance.cleanup if @cleanup

result
end

private def cross_compile(program, units, lib_flags, output_filename)
private def cross_compile(program, units, output_filename)
llvm_mod = units.first.llvm_mod
object_name = "#{output_filename}.o"

Expand All @@ -269,10 +266,34 @@ module Crystal

target_machine.emit_obj_to_file llvm_mod, object_name

stdout.puts "#{CC} #{object_name} -o #{output_filename} #{@link_flags} #{lib_flags}"
stdout.puts linker_command(program, object_name, output_filename)
end

private def linker_command(program : Program, object_name, output_filename)
if program.has_flag? "windows"
if object_name
object_name = %("#{object_name}")
else
object_name = %(%*)
end

if (link_flags = @link_flags) && !link_flags.empty?
link_flags = "/link #{link_flags}"
end

%(#{CL} #{object_name} "/Fe#{output_filename}" #{program.lib_flags} #{link_flags})
else
if object_name
object_name = %('#{object_name}')
else
object_name = %("${@}")
end

%(#{CC} #{object_name} -o '#{output_filename}' #{@link_flags} -rdynamic #{program.lib_flags})
end
end

private def codegen(program, units : Array(CompilationUnit), lib_flags, output_filename, output_dir)
private def codegen(program, units : Array(CompilationUnit), output_filename, output_dir)
object_names = units.map &.object_filename

target_triple = target_machine.triple
Expand Down Expand Up @@ -303,7 +324,7 @@ module Crystal

@progress_tracker.stage("Codegen (linking)") do
Dir.cd(output_dir) do
system %(#{CC} -o "#{output_filename}" "${@}" #{@link_flags} #{lib_flags}), object_names
system(linker_command(program, nil, output_filename), object_names)
end
end

Expand Down