In summary we have no less than six patches required to support Win64 SEH MinGW. The first five could be committed after review and LGTM but the last one also requires Ray Donnelly approval. Please comment in the Phabricator so the comments would be kept in context. 'unreachable' trap http://reviews.llvm.org/D3417 Win64 SEH (LLVM) http://reviews.llvm.org/D3418 Win64 SEH (clang) http://reviews.llvm.org/D3419 MinGW toolchain http://reviews.llvm.org/D3420 TLS (clang) http://reviews.llvm.org/D3421 Register names instead of numbers http://reviews.llvm.org/D3422 2014-04-18 14:42 GMT+03:00 Martell Malone <martellmalone at gmail.com>:> Hi chandler, > > Sorry for the confusion. > I've never submitted patches before. > I'll cc Ray to get him to confirm that the patch can be merged. His header > should be on the patch also so that's how you know it is his. > > Yes Yaron the names above seem and the detail seem correct to me except > that the MinGW driver derived from Rubens submission to LLVM reviews some > months ago. > I said that at the start of the thread. > > Basically the guys including Ray were looking to have an x64 mingw64 > working with clang. So I fixed Rubens patch and kai's patch for head > > I'm not concerned with having my name on any attribution I just want them > all merged as this combination of patches fixes mingw64 x64 with clang. > > Kind Regards > Martell > On 18 Apr 2014 11:11, "Yaron Keren" <yaron.keren at gmail.com> wrote: > >> Hi Chandler, >> >> There were five SEH releated patches posted in two threads in the last >> days. >> >> Two different patches in Martell e-mail starting this thread: the win64 >> seh (llvm) and the register names >> >> Three more related SEH patches in another thread: one for win64 seh >> clang, one for MinGW toolchain and another for unreachable prologue. >> >> To clarify and allow proper reviews for the different patches I opened >> reviews for four of them (the fifth got LGTM in the discussion but it does >> have the ownership issue you wrote) >> >> D3417 >> Emit a trap instruction for IR 'unreachable<http://reviews.llvm.org/D3417> >> >> D3418 >> SEH exceptions on Win64 (LLVM) <http://reviews.llvm.org/D3418> >> >> D3419 >> SEH exceptions on Win64 (clang part) <http://reviews.llvm.org/D3419> >> >> D3420 >> MinGW toolchain <http://reviews.llvm.org/D3420> >> >> >> Per your suggestion the link to the original discussion in D3418 was >> added. >> >> Code authors are: >> unreachable is by Vadim Chugunov. >> win64 seh llvm is by Kai Nacke + Martell re-posting. >> win64 seh clang is by Martell. >> mingw toolchain is by Martell. >> register names is by Ray Donnelly + Martell posting it. >> seh test for "register names" ( test is r206566) by myself. >> >> All but the "register names" patch were posted by their author on the >> llvm lists. >> >> Yaron >> >> >> >>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140418/66c67e5e/attachment.html>
On Fri, Apr 18, 2014 at 5:29 AM, Yaron Keren <yaron.keren at gmail.com> wrote:> In summary we have no less than six patches required to support Win64 SEH > MinGW. The first five could be committed after review and LGTM but the last > one also requires Ray Donnelly approval. >No, it really requires that Ray Donnelly *contribute* the patch. That is different. Also, folks informed us of at least one place where a patch posted to the list by Kai in the past from the LDC / redstar.de work contained copied copyrighted material that Kai did not hold the rights to, and was not available under *any* license. This is really concerning, and it means that any significant contributions from this body of work need to be very carefully audited for other places where this has happened. I'm not sure of any good way to do that at this point. This isn't to say we don't want to support the win64 ABI and exception handling stuff, we really do (and thanks for working on it!), but we need to be careful about how we do it. My suggestion if you want to move this forward quickly would be for folks who are interested author their own patches independently, without any reference to or basis on existing work, and contribute that patch. That is what folks here are planning to do for their ABI work. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140418/f205189f/attachment.html>
Hi Chandler, This is what Ray wrote on Phabricator http://reviews.llvm.org/D3422 : mingwandroid <http://reviews.llvm.org/p/mingwandroid/> commented on this revision. Hi. I am the original author of this patch and am happy for our to be submitted for inclusion via this submission. Best regards, Ray Donnelly. Is this acceptable or do we need anything else? Yaron 2014-04-18 23:08 GMT+03:00 Chandler Carruth <chandlerc at google.com>:> On Fri, Apr 18, 2014 at 5:29 AM, Yaron Keren <yaron.keren at gmail.com>wrote: > >> In summary we have no less than six patches required to support Win64 SEH >> MinGW. The first five could be committed after review and LGTM but the last >> one also requires Ray Donnelly approval. >> > > No, it really requires that Ray Donnelly *contribute* the patch. That is > different. > > > Also, folks informed us of at least one place where a patch posted to the > list by Kai in the past from the LDC / redstar.de work contained copied > copyrighted material that Kai did not hold the rights to, and was not > available under *any* license. This is really concerning, and it means that > any significant contributions from this body of work need to be very > carefully audited for other places where this has happened. I'm not sure of > any good way to do that at this point. > > This isn't to say we don't want to support the win64 ABI and exception > handling stuff, we really do (and thanks for working on it!), but we need > to be careful about how we do it. > > My suggestion if you want to move this forward quickly would be for folks > who are interested author their own patches independently, without any > reference to or basis on existing work, and contribute that patch. That is > what folks here are planning to do for their ABI work. >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140418/62984e62/attachment.html>
Hi Chandler, On 18.04.2014 22:08, Chandler Carruth wrote:> Also, folks informed us of at least one place where a patch posted to > the list by Kai in the past from the LDC / redstar.de > <http://redstar.de/> work contained copied copyrighted material that Kai > did not hold the rights to, and was not available under *any* license.To which patch/post are you refering? I am not aware of such an issue. The original patch is from Charles Davis. I attach the communication how I got the code. Or is here more to do? Regards, Kai -------------- next part -------------- Subject: Re: Win64 Exception Handling From: kai <kai at redstar.de> Date: 17.01.2012 18:05 To: Charles Davis <cdavis at mymail.mines.edu> Hi Chip! Thanks for the patch. Maybe I can fill in the missing pieces.... Regards Kai On 16.01.2012 23:46, Charles Davis wrote:> Hi Kai, > > On Jan 16, 2012, at 12:22 PM, Kai wrote: > >> Hi Charles! >> >> From the LLVM mailing list I know that you are working on Win64 exception handling support. >> >> I try to port LDC2 (the LLVM based D compiler) to Win64. After creating TLS support for Windows, the native exception handling seems to be the last missing piece. >> >> I like to know the status of your work. Is the implementation usable? > Not yet. There's one big piece missing: the part where the backend actually emits the SEH assembler directives for various instructions. I have an incomplete patch for that, but I've been busy lately, so I haven't had much time to work on it. (Then, of course, it needs to be turned on on x86_64/Windows, but I wasn't going to do that until I had this finished, tests in place passing, etc.) > > I'll attach what I have so far; maybe you'll have better luck than me :). >> I found a lot of supporting code, but the Win64 exception handling code seems not to be enabled. >> >> Thanks for your help. >> >> Best regards >> Kai > Chip