[PATCH] Enable building LTO on WIN32

Wed Oct 2 08:20:52 PDT 2013

> The build now works, but the test was failing, so I disabled it again.

Looks like the test changed in the meantime since I generated the patch 
yesterday!  It was working then I promise! :-)

I have a theory on what this failure is, as it looks very very similar to 
something I fixed in our very outdated local branch, which I was also 
planning to send upstream if it was still applicable.  It all comes down 
to the fact that Windows makes a distinction between files written in text 
mode and binary mode.

My local fix was to make the following change in LTOCodeGenerator.cpp 
where it does "generate object file":

-   tool_output_file objFile(uniqueObjPath.c_str(), errMsg);

+   tool_output_file objFile(uniqueObjPath.c_str(), errMsg,
+     raw_fd_ostream::F_Binary);

I can see that nowadays it creates the file handle with a call to 
createTemporaryFile instead which is passed into the constructor for 
tool_output_file.  I need to figure out whether the temporary file is 
created as text or binary.  If it's text mode, then we can assume that 
Windows is up to all sorts of mischief messing around inserting carriage 
return characters all over the place which would account for the file 
being corrupted by the time llvm-nm wanted to read it.

Thanks,

-Greg

Rafael Espíndola <rafael.espindola at gmail.com> wrote on 02/10/2013 
15:53:31:

> From: Rafael Espíndola <rafael.espindola at gmail.com>
> To: Greg_Bedwell at sn.scee.net
> Cc: llvm-commits <llvm-commits at cs.uiuc.edu>
> Date: 02/10/2013 15:53
> Subject: Re: [PATCH] Enable building LTO on WIN32
> 
> The build now works, but the test was failing, so I disabled it again.
> 
> http://bb.pgr.jp/builders/ninja-clang-i686-msc17-R/builds/5216/
> steps/test_llvm/logs/LLVM%20%3A%3A%20LTO__cfi_endproc.ll
> 
> 
> 
> On 2 October 2013 10:08, Rafael Espíndola <rafael.espindola at gmail.com> 
wrote:
> > r191823.
> >
> > Thanks.
> >
> > On 2 October 2013 09:00,  <Greg_Bedwell at sn.scee.net> wrote:
> >>> LGTM
> >>
> >> Thanks!  Please could you (or anyone I guess?) commit for me?  Let me 
know
> >> if you need me to update my patch against the latest version.
> >>
> >> -Greg
> >>
> >> Rafael Espíndola <rafael.espindola at gmail.com> wrote on 01/10/2013
> >> 21:53:29:
> >>
> >>> From: Rafael Espíndola <rafael.espindola at gmail.com>
> >>> To: Greg_Bedwell at sn.scee.net
> >>> Cc: llvm-commits <llvm-commits at cs.uiuc.edu>
> >>> Date: 01/10/2013 21:53
> >>> Subject: Re: [PATCH] Enable building LTO on WIN32
> >>>
> >>> LGTM
> >>>
> >>> On 1 October 2013 16:45,  <Greg_Bedwell at sn.scee.net> wrote:
> >>> > Right... Let's try that again!
> >>> >
> >>> > So my last patch was reverted for breaking the build (sorry!).   I
> >> tracked
> >>> > down the problem to affecting builds through Ninja only.  The 
problem
> >> came
> >>> > down to the fact that I was specifying the def file option to the
> >> linker
> >>> > for the whole LTO CMake file.  On the Visual Studio and MinGW 
builds
> >> this
> >>> > was fine because the linker is used to build the DLL and the lib 
tool
> >> is
> >>> > used to create LTO_static so CMake was adding the def file to the 
DLL
> >>> > build only as expected.  The difference is that when building with
> >> Ninja
> >>> > the linker is used for both steps, with '/lib' specified for
> >> LTO_static to
> >>> > invoke the lib tool indirectly.   This difference meant that CMake 
was
> >>> > adding the def file to both the DLL and LTO_static builds, but it 
was
> >>> > clearly only valid for the DLL.  I've modified my change to now
> >> explicitly
> >>> > only add the def file to the DLL build.
> >>> >
> >>> > I've tested the Windows build with VS2010, VS2012, Ninja and MinGW 
so
> >>> > hopefully I've covered all my bases now...
> >>> >
> >>> > Here's my updated patch.  All feedback welcomed.
> >>> >
> >>> >
> >>> >
> >>> > Thanks,
> >>> >
> >>> > -Greg
> >>> >
> >>> > (BTW Ninja does seem really nice and fast, so all the future hours 
I
> >> save
> >>> > waiting for MSVC to do its thing is one big positive to come out 
of
> >> this
> >>> > :-)
> >>> >
> >>> >
> >>> >
> >>> > Rafael Espíndola <rafael.espindola at gmail.com> wrote on 30/09/2013
> >>> > 16:31:59:
> >>> >
> >>> >> From: Rafael Espíndola <rafael.espindola at gmail.com>
> >>> >> To: Greg_Bedwell at sn.scee.net
> >>> >> Cc: llvm-commits <llvm-commits at cs.uiuc.edu>
> >>> >> Date: 30/09/2013 16:32
> >>> >> Subject: Re: [PATCH] Enable building LTO on WIN32
> >>> >>
> >>> >> On 30 September 2013 10:57,  <Greg_Bedwell at sn.scee.net> wrote:
> >>> >> >> We should switch to in source annotations at some point, but 
we
> >>> > should
> >>> >> >> do so for windows and ELF/MachO at the same time. Using the 
.def
> >> for
> >>> >> >> now is OK.
> >>> >> >>
> >>> >> >
> >>> >> > Thanks.  I agree with this entirely.
> >>> >> >
> >>> >> >> Patch LGTM.
> >>> >> >
> >>> >> > Please can you commit for me?  I'm still working towards commit
> >>> > access. Do
> >>> >> > you want me to rebase against top of trunk?
> >>> >>
> >>> >>
> >>> >> r191670. Thanks.
> >>> >>
> >>> >> Cheers,
> >>> >> Rafael
> >>> >
> >>> >
> >>> > 
**********************************************************************
> >>> > This email and any files transmitted with it are confidential and
> >> intended
> >>> > solely for the use of the individual or entity to whom they are
> >> addressed.
> >>> > If you have received this email in error please notify
> >> postmaster at scee.net
> >>> > This footnote also confirms that this email message has been 
checked
> >> for
> >>> > all known viruses.
> >>> > Sony Computer Entertainment Europe Limited
> >>> > Registered Office: 10 Great Marlborough Street, London W1F 7LP, 
United
> >>> > Kingdom
> >>> > Registered in England: 3277793
> >>> > 
**********************************************************************
> >>> >
> >>> > P Please consider the environment before printing this e-mail
> >>
> >>
> >> 
**********************************************************************
> >> This email and any files transmitted with it are confidential and 
intended
> >> solely for the use of the individual or entity to whom they are 
addressed.
> >> If you have received this email in error please notify 
postmaster at scee.net
> >> This footnote also confirms that this email message has been checked 
for
> >> all known viruses.
> >> Sony Computer Entertainment Europe Limited
> >> Registered Office: 10 Great Marlborough Street, London W1F 7LP, 
United
> >> Kingdom
> >> Registered in England: 3277793
> >> 
**********************************************************************
> >>
> >> P Please consider the environment before printing this e-mail

**********************************************************************
This email and any files transmitted with it are confidential and intended 
solely for the use of the individual or entity to whom they are addressed. 
If you have received this email in error please notify postmaster at scee.net
This footnote also confirms that this email message has been checked for 
all known viruses.
Sony Computer Entertainment Europe Limited
Registered Office: 10 Great Marlborough Street, London W1F 7LP, United 
Kingdom
Registered in England: 3277793
**********************************************************************

P Please consider the environment before printing this e-mail