[PATCH] D11405: [MS ABI] Hook clang up to the new EH instructions

[Reid Kleckner]

rnk added inline comments.

================
Comment at: include/clang/Basic/LangOptions.def:111
@@ -110,2 +110,3 @@
 LANGOPT(SjLjExceptions    , 1, 0, "setjmp-longjump exception handling")
+LANGOPT(NewMSEH           , 1, 0, "new IR representation for MS exceptions")
 LANGOPT(TraditionalCPP    , 1, 0, "traditional CPP emulation")
----------------
Seems like it should be a CodeGenOption.

================
Comment at: lib/CodeGen/CGCleanup.cpp:907
@@ +906,3 @@
+    llvm::BasicBlock *NextAction = getEHDispatchBlock(EHParent);
+    if (CGM.getLangOpts().NewMSEH && getTarget().getCXXABI().isMicrosoft()) {
+      if (NextAction)
----------------
Should we do anything to synchronize these getCXXABI checks with the EHPersonality? At the very least, I think it's more intuitive to the reader to make the decisions about the IR we use to be based on the personality.

Probably the right thing to do here is to declare EHPersonality in CGCleanup.h, and then acknowledge that CGCleanup.h is actually CGException.h. Sounds like a separable patch.

================
Comment at: lib/CodeGen/CGException.cpp:897
@@ +896,3 @@
+                                                    EHCatchScope &catchScope) {
+  llvm::BasicBlock *dispatchBlock = catchScope.getCachedEHDispatchBlock();
+  assert(dispatchBlock);
----------------
Should we rename the locals here according to the prevailing LLVM style?

================
Comment at: lib/CodeGen/CGException.cpp:911
@@ +910,3 @@
+    llvm::Value *typeValue = handler.Type;
+    assert((typeValue != nullptr || handler.isCatchAll()));
+    if (!typeValue)
----------------
Drop the extra parens?

================
Comment at: lib/CodeGen/CGException.cpp:944
@@ -824,3 +943,3 @@
 /// It is an invariant that the dispatch block already exists.
-static void emitCatchDispatchBlock(CodeGenFunction &CGF,
-                                   EHCatchScope &catchScope) {
+static llvm::BasicBlock *emitCatchDispatchBlock(CodeGenFunction &CGF,
+                                                EHCatchScope &catchScope) {
----------------
Please add a comment documenting what the return value is. I'm thinking:
  If the catchblock instructions are used for EH dispatch, then the basic block holding the final catchendblock instruction is returned.


http://reviews.llvm.org/D11405