[LLVMdev] WinEH work to be done (in progress and otherwise)

[Ristow, Warren]

A tangential comment related to:

> I think the best we can do is add a subdirectory of test-suite that's
> outside it's makefile system and go from there. Or we could add a new repo
> similar to the ABI test suite that Sony contributed. However, you may see
> that as a cautionary tale. Sony added the test suite, and nobody else runs
> it and the bot has been broken for a month.

Thanks for pointing that out.  We're now monitoring it, and this is fixed.  (The issue was a Ninja-build problem that prevented the build from succeeding.  So fortunately for those interested in release-to-release compatibility, not an ABI-breaking compiler change.)

In any case, that was our bad (Sony's).  I wouldn't want a decision here on how to organize the WinEH testing to be biased by the fact that we neglected to keep an eye on the ABI test suite bot.

Thanks,
-Warren

From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On Behalf Of Reid Kleckner
Sent: Tuesday, May 19, 2015 2:56 PM
To: Kaylor, Andrew
Cc: llvmdev at cs.uiuc.edu
Subject: Re: [LLVMdev] WinEH work to be done (in progress and otherwise)

On Tue, May 19, 2015 at 2:16 PM, Kaylor, Andrew <andrew.kaylor at intel.com<mailto:andrew.kaylor at intel.com>> wrote:
What follows is primarily addressed to Reid and David, but since it some of it is relevant to the Windows exception handling proposal under discussion, I thought I should copy the LLVMDev distribution list.  Anyone interested should feel free to jump in and discuss.

My primary purpose here is to talk about what I’m working on so that anyone who is also working on the same things can let me know and avoid duplicated efforts.

Right now, in addition to the patch I have up on Phabricator to get 64-bit C++ EH mostly working with unoptimized IR, I have a couple of local changes that fix “failure to demote” problems with optimized IR.  My goal here has been to use the current implementation to discover what pitfalls remain that could give us problems in the future with any alternate implementation.  I still think the patch I have up for review is worth committing because it gets a lot of cases in a state where we can compile and execute correctly which serves as a useful point of comparison for the failures of the same cases when optimizations are applied.   I’m not sure about the other changes I just mentioned, as they are just delving further into untangling the mess we’ve gotten ourselves into.  I’ll keep them around and would be happy to share them if anyone is interested, but I don’t plan to push to commit them in the foreseeable future.

Yeah, I think we should probably commit it, it's just big and I'm struggling to field all the email that the exception proposal kicked up and find time to review it.

I’m shifting gears today, putting aside the optimization related problems, and have begun exploring 32-bit C++ EH support.  At least part of the work to be done here seems like it will remain relevant even after the current redesign comes to fruition and even what won’t remain relevant looks like it will be interesting as input to the redesign.

Maybe that makes more sense. I was kind of trying to put aside WinEHPrepare issues and tackle 32-bit state numbering myself, but I kept getting dragged back to outlining problems.

Have either of you (Reid and David) looked closely at the 32-bit C++ EH case?  I saw the preliminary support in X86WinEHState, and I’m currently working on addressing the problem described in the FIXME comment there.  I had been under the impression that 32-bit C++ EH was pretty much just a matter of making the equivalent of the xdata tables accessible via the stack and everything would more or less fall into place.  Now that I’m taking a closer look it also appears that live code must be generated to keep the EH state updated -- at least that appears to be what MSVC is doing.

Yep. Whatever numbering we do for 64-bit, we should reuse the same code to get the numbers and insert a bunch of stores in the IR. At least, that's the idea.

The live numbering of EH states is an interesting contrast to the ip2state table approach used in the x86_64 case, and it makes me wonder what complications would arise from trying to generate the EH state numbers in the front end.  Reid’s recent RFC mentioned that, “The natural scoping rules of C++ make it so that doing this numbering at the source level is trivial, but once we go to LLVM IR CFG soup, scopes are gone.”  So why not generate these numbers in the front end?  Would keeping them correct through the optimization passes require blocking too many useful optimizations?  Alternatively, can we insert this as part of the invoke lowering?

There's a couple reasons why I want to do it later. First, it's not clear where we'd tack on the number and how to make sure it survives optimization. Second, we'd have to teach the inliner to go and update it. It just doesn't feel like the right long term semantic model. If we go through with the new EH representation, the numbering should be easy. I've tried it by hand on a number of examples, and it works out.

Apart from this, the other WinEH-related task that I have on my radar (but as of yet haven’t done anything about) is getting “nightly” tests running on Windows.  I did look into the instructions for running the “test-suite” tests, and it looks like that test suite is assuming a Unix-type platform to some extent.  I tried to re-interpret the instruction and run this suite on Windows, but it didn’t work and I haven’t looked into trying to fix whatever the problems were.  I saw that someone submitted a patch last November that claimed to improve the running of these tests on Windows, but the patch was never reviewed or committed.

So I was wondering, is anyone running compile-and-execute type tests with clang on a native (i.e. non-cygwin/mingw) Windows platform?  It seems like this will be an important thing to do in general, but particularly so for Windows EH, dependent as it is on interaction with the runtime library.  That is, having tests which verify that we’re producing the xdata entries we intend to produce doesn’t seem like a very thorough method of testing and is no guarantee at all that we are producing working executables.  In addition to the existing C++ EH tests in the “test-suite” tests, I have a suite of tests (not currently in any public repository) that were specifically developed to exercise Windows-specific problem cases.  I’d eventually like to see these tests run on some regular basis against clang on Windows.  Is there any work currently under way to make something like this happen?

I'd like to have a place where we can put small, general purpose execution tests for clang, but we don't have a good place. test-suite is just too big and non-portable. The results it generates are also not suitable for consumption by developers. If someone upstream breaks a C++ EH execution test, what should they do, how should they debug? A portable assembly-emission test works because they can see the before and after and evaluate whether their change was correct or not.

For the rest of the MS C++ ABI, we've gotten by self-hosting Clang and then building Chromium, and that's enough coverage. Neither project uses C++ exceptions, so that's not going to cut it for this. Long ago Timur used to maintain a small repo in Google Code for this:
https://code.google.com/p/smoke-cpp-tests/
Nobody runs it though and we haven't updated as we encountered and solved problems.

I think the best we can do is add a subdirectory of test-suite that's outside it's makefile system and go from there. Or we could add a new repo similar to the ABI test suite that Sony contributed. However, you may see that as a cautionary tale. Sony added the test suite, and nobody else runs it and the bot has been broken for a month.

I guess my conclusion is that this will probably be a lot of work, and I think it'll provide less value than you expect from it. But, if you think it's worth it and want to push it through, then it costs LLVM very little check a few test cases in outside the main clang/llvm repos.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150602/453b7452/attachment.html>