[PATCH] D16763: [X86] Optimize WinEH state stores

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: lib/Target/X86/X86WinEHState.cpp:422
@@ -410,1 +421,3 @@
+  // Figure out what state we should assign calls in this block.
+  auto BaseStateForBB = [&](BasicBlock *BB) {
     int BaseState = -1;
----------------
I'm not really a fan of all these long inline lambdas. I'd rather see this out of line, despite the paramter passing boilerplate:
  static int getBaseStateForBB(... BBColors, ... FuncInfo, BasicBlock *BB) { ... }
You can practically guess the algorithm it's implementing from that prototype.

================
Comment at: lib/Target/X86/X86WinEHState.cpp:439
@@ +438,3 @@
+  // Calculate the state a call-site is in.
+  auto StateForCallSite = [&](CallSite CS) {
+    if (auto *II = dyn_cast<InvokeInst>(CS.getInstruction())) {
----------------
ditto

================
Comment at: lib/Target/X86/X86WinEHState.cpp:466
@@ +465,3 @@
+      CallSite CS(&I);
+      if (!CS || CS.doesNotThrow())
+        continue;
----------------
For SEH, I think we need state insertions prior to nounwind calls. This is the case I'm thinking of:
  void f() {
    __try { crash(); }
    __except(1) { }
    printf("%s\n", nullptr);
  }
The printf call will crash, and it should not end up being caught by the __except block. It needs a -1 store, despite being nounwind. You can avoid state stores prior to intrinsic calls in SEH though.

I guess this is a pre-existing bug.

================
Comment at: lib/Target/X86/X86WinEHState.cpp:490
@@ +489,3 @@
+  // predecessor.
+  auto CalcPredState = [&](BasicBlock *BB) {
+    // The entry block has no predecessors but we know that the prologue always
----------------
grumble grumble

================
Comment at: test/CodeGen/WinEH/wineh-statenumbering.ll:137
@@ -120,3 +136,3 @@
 ehcleanup:                                        ; preds = %catch.dispatch1
   %4 = cleanuppad within %1 []
   ; CHECK: ehcleanup:
----------------
We should probably add a non-trivial case like:
  void f(int cond) {
    if (cond) { __try { crash(); } __except(1) { } }
    g(); // need -1 store before g
  }


http://reviews.llvm.org/D16763