[cfe-dev] Merging MinGW toolchain patches to 3.7

[Richard Smith]

On Thu, Jul 23, 2015 at 2:56 PM, Yaron Keren <yaron.keren at gmail.com> wrote:

> Hi Richard,
>
> r242660 + r242666 + r242667 are the current mingw toolchain, should be
> tested together.
> The code test covarage is not complete:
>
> 1) To make the toolchain work automatically, the first stage is detection
> of the mingw installation dir in MinGW::findGccLibDir(). This part is not
> covered by any tests as it probes several file system directories outside
> the LLVM tree using llvm::sys::fs::directory_iterator, we can't control
> that using test infrastructure.
>

Please take a look at how other targets handle this (see the various
sysroot trees under test/Driver/Inputs). If the same approach can't work
here, we should figure out how to fix that. The MinGW driver support should
be testable without having a MinGW toolchain installed, otherwise the cost
imposed on people maintaining the driver and Clang in general is too high.

The most reliable way to test this toolchain would be to have a mingw
> toolchain installed on various bots and run tests using it. I'm open to
> suggestions how to test MinGW::findGccLibDir() otherwise.
>
> 2) The second stage generates the C and C++ include directories based
> on GccLibDir found in stage one. This *is* covered by the tests introduced
> in r242666 by injecting GccLibDir with --sysroot argument (instead of
> probing the file system) for both Windows and Linux.
>
> MinGW::Arch has a trainling slash as a small optimization so Arch could be
> concatenated into a path instead of using sys::path::append. If you find
> that confusing I can remove the trailing slash and use sys::path::append
> calls instead, it would not matter much.
>

There should at least be a comment explaining this in the class definition.
But generally, using the higher-level facilities such as sys::path::append
should be preferred over manually inserting separators into paths, so I'd
prefer you did that. Performance microoptimizations are not especially
relevant for the driver, since it's such a tiny proportion of the cost of a
compilation.


> Yaron
>
>
>
>
> 2015-07-24 0:09 GMT+03:00 Richard Smith <richard at metafoo.co.uk>:
>
>> On Thu, Jul 23, 2015 at 12:30 PM, Richard Smith <metafoo at gmail.com>
>> wrote:
>>
>>> Yes, we should get these changes into 3.7. I'll look over the patches
>>> later today.
>>>
>> I'm not very happy about applying patches with no tests to the branch,
>> but, reluctantly, approved.
>>
>> Yaron: please add some tests for r242660 and r242667 to trunk. (I also
>> find it very weird that the MinGW::Arch member has a trailing slash. Is
>> there a good reason for that?)
>>
>>> On 23 Jul 2015 9:56 am, "Hans Wennborg" <hans at chromium.org> wrote:
>>>
>>>> +cfe-dev to keep it on the list, and Richard
>>>>
>>>> This sounds like a reasonable thing to merge.
>>>>
>>>> Richard, what's your opinion as an owner?
>>>>
>>>>
>>>> On Thu, Jul 23, 2015 at 9:48 AM, Martell Malone <
>>>> martellmalone at gmail.com> wrote:
>>>> > I almost forgot
>>>> >
>>>> > Yaron also fixed all the different combination of linux host/distros
>>>> > targetting mingw with theses 3.
>>>> > I have a feeling we will have people hitting the mailing list about
>>>> come the
>>>> > 3.7 release if their distro isn't working.
>>>> >
>>>> > clang
>>>> >
>>>> > r242660  Support mingw toolchain include and lib directories on Arch
>>>> Linux.
>>>> > r242667  Remove erroneous space in "lib64" string constant.
>>>> > r242766  Fix mingw toolchain to honor sysroot on Linux and add tests.
>>>> >
>>>> > Again as i said I've no idea what the process is or what we can do
>>>> here.
>>>> > Hoping you can fill me in on the details
>>>> >
>>>> > Kind Regards
>>>> > Martell
>>>> >
>>>> >
>>>> >
>>>> > On Thu, Jul 23, 2015 at 5:44 PM, Martell Malone <
>>>> martellmalone at gmail.com>
>>>>
>>>> > wrote:
>>>> >>
>>>> >> Hi Hans :)
>>>> >>
>>>> >> rnk and I were talking to me about the possibility of getting the
>>>> mingw
>>>> >> driver work back ported into the 3.7 branch.
>>>> >>
>>>> >> I read the thread where you branched the 3.7 release you said that if
>>>> >> anyone has a request to have patches merged they should contact you
>>>> directly
>>>> >>
>>>> >> When the branch was made we were in the middle of crushing the final
>>>> bugs
>>>> >> in the MINGW Driver.
>>>> >> I had a total of about 9 patches merged but 2 missed the window
>>>> >> unfortunately.
>>>> >> 1 was for compiler-rt and 1 was for clang
>>>> >>
>>>> >> clang
>>>> >> r242905  Add support for -rtlib option and -stdlib option to the
>>>> mingw
>>>> >> driver
>>>> >>
>>>> >> compiler-rt
>>>> >> r242539  compiler-rt: add support for mingw-w64 in builtins
>>>> >> r242540  Add missing chkstk.S files from r242539
>>>> >>
>>>> >> (r242540 is here because only half the patch was committed initially)
>>>> >>
>>>> >> These 2(now 3) patches complete my series and fixes everything to
>>>> have a
>>>> >> clang toolchain with mingw-w64 without needing gcc.
>>>> >>
>>>> >> I don't know what the process is for back porting or how you feel
>>>> about
>>>> >> these patches but I said I would email you about it an see where we
>>>> can go
>>>> >> from here :)
>>>> >>
>>>> >> Kind Regards
>>>> >> Martell
>>>> >
>>>> >
>>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150723/e4c81cd8/attachment.html>