This is an archive of the discontinued LLVM Phabricator instance.

Differential D14290

compiler-rt: add make solution to bootstrap for mingw-w64
ClosedPublic

Authored by martell on Nov 3 2015, 9:23 AM.

Details

Reviewers
compnerd
Commits
rG5b7658c1a4d7: compiler-rt: add make solution to bootstrap mingw-w64
rCRT252976: compiler-rt: add make solution to bootstrap mingw-w64
rL252976: compiler-rt: add make solution to bootstrap mingw-w64
Summary

I know we will be moving forward with the cmake fixes but I wanted to submit something useful for users in the mean time.
From what I can gather this doesn't use autotool and just invokes make directly so it might be something to have until cmake is ready to bootstrap.

The cmake work around I am currently using is not suitable for upstreaming but this should be
but for some reason I can't currently get it to compile chkstk or the recently added chkstk2?

I intend to update this to support 3 targets x86_64 i686 and armv7 (aka thumb2 win NT)

Diff Detail

Repository
rL LLVM

Event Timeline

martell updated this revision to Diff 39078.Nov 3 2015, 9:23 AM
martell retitled this revision from to compiler-rt: add make solution to bootstrap for mingw-w64.
martell updated this object.
martell set the repository for this revision to rL LLVM.
martell added a subscriber: llvm-commits.
Herald added a subscriber: aemerson. · View Herald TranscriptNov 3 2015, 9:23 AM
martell updated this object.Nov 3 2015, 9:39 AM
martell added a reviewer: beanz.
martell added a subscriber: rnk.
martell updated this revision to Diff 39084.EditedNov 3 2015, 10:18 AM
martell edited reviewers, added: compnerd; removed: beanz.
martell removed rL LLVM as the repository for this revision.
martell added a subscriber: beanz.

Updated to support arm and i386
If the end user has i686 they will just have to rename when they are copying as i386 is the default in the makefiles.
Same applied for arm to armv7 under winNT
Also remove -Werror until the armv7 ifdef's out pcs for _WIN32

having make find chkstk still remains the issue.

martell updated this revision to Diff 39086.Nov 3 2015, 10:26 AM

updated i686 to use -m32

samsonov added a subscriber: samsonov.Nov 3 2015, 10:46 AM

Hm, I understand why this patch can be useful, but not entirely sure we want to upstream it and therefore let other people use it, given the effort to switch to CMake entirely ASAP - it means more people will have to change their workflows. Chris, WDYT?

martell added a comment.Nov 3 2015, 11:04 AM
In D14290#280264, @samsonov wrote:

Hm, I understand why this patch can be useful, but not entirely sure we want to upstream it and therefore let other people use it, given the effort to switch to CMake entirely ASAP - it means more people will have to change their workflows. Chris, WDYT?

On this my reason for the upstream is because it is an immediate solution.
I only intend to add support for builtins to this and nothing more.
It looks like it might be some time for the bootstrapping via cmake is ready.
At that point I assume we would dump this along with the rest of the other make targets.

We currently have OSX IOS and Linux buildable in this mannor so I don't see the harm in adding mingw until cmake is ready

compnerd added inline comments.Nov 3 2015, 8:36 PM
make/platform/clang_mingw.mk
6 ↗(On Diff #39086)

What happens when llvm-ar is not found? I don't think using llvm-ar since we can't bootstrap that. It should use ar.

23 ↗(On Diff #39086)

The -rtlib flags here are irrelevant. The builtins are static libraries, no link will be performed. -m32 on ARM is incorrect. There is no 32-bit or 64-bit there. 64-bit ARM is armv8, not armv7.

33 ↗(On Diff #39086)

This isn't Linux :-).

36 ↗(On Diff #39086)

This shouldn't be needed since no shared libraries will be created.

samsonov added a comment.Nov 4 2015, 10:54 AM

Not that I see harm in it, we just don't plan to support it. We still have Makefile on Mac for historic/legacy reasons, as there are known clients/systems that used to depend on this build process. Anyway, I have no strong opinion here, and will happily delegate Windows-related stuff to Saleem :)

martell updated this revision to Diff 39783.Nov 9 2015, 10:15 PM

Address Saleem's comments :)

compnerd accepted this revision.Nov 12 2015, 12:05 PM
compnerd edited edge metadata.

At the very least, please remove the -m32 and -m64. This is fine for the time being, but I believe that at some point this is going to go away with the rest of the make based build system, so you may have to move to cmake in the near future.

make/platform/clang_mingw.mk
5 ↗(On Diff #39783)

This really is better as cc.

22 ↗(On Diff #39783)

Actually, why do you need the ABI specification? The target architecture already does the right thing. That is -m64 and -m32 are superfluous.

This revision is now accepted and ready to land.Nov 12 2015, 12:05 PM
martell updated this revision to Diff 40093.Nov 12 2015, 2:55 PM
martell edited edge metadata.
Closed by commit rL252976: compiler-rt: add make solution to bootstrap mingw-w64 (authored by martell). · Explain WhyNov 12 2015, 4:24 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
compiler-rt/
trunk/
make/
platform/
30 lines

Diff 40098

compiler-rt/trunk/make/platform/clang_mingw.mk

Description := Static runtime libraries for mingw-w64
###
CC ?= cc
AR ?= ar
Arch := unknown
Configs :=
SupportedArches := x86_64 i386 arm
Configs += builtins-x86_64 builtins-i386 builtins-arm
Arch.builtins-x86_64 := x86_64
Arch.builtins-i386 := i386
Arch.builtins-arm := arm
###
CFLAGS := -Wall -O3 -fomit-frame-pointer
CFLAGS.builtins-x86_64 := -target x86_64-windows-gnu $(CFLAGS)
CFLAGS.builtins-i386 := -target i686-windows-gnu $(CFLAGS)
CFLAGS.builtins-arm := -target armv7-windows-gnu $(CFLAGS)
FUNCTIONS.builtins-x86_64 := $(CommonFunctions) $(ArchFunctions.x86_64)
FUNCTIONS.builtins-i386 := $(CommonFunctions) $(ArchFunctions.i386)
FUNCTIONS.builtins-arm := $(CommonFunctions) $(ArchFunctions.arm)
# Always use optimized variants.
OPTIMIZED := 1