[PATCH] D15325: [WinEH] Update CoreCLR EH state numbering

Joseph Tremoulet via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 25 20:22:29 PST 2015


JosephTremoulet added inline comments.

================
Comment at: lib/CodeGen/WinEHPrepare.cpp:641-642
@@ -521,3 +640,4 @@
 
+  // Step three: transfer information from pads to invokes.
   calculateStateNumbersForInvokes(Fn, FuncInfo);
 }
----------------
majnemer wrote:
> Is `calculateStateNumbersForInvokes` correct for your personality?  I'm surprised it went through untouched.  Specifically, the bit where we use `getCleanupRetUnwindDest` seems a little sketchy...
Yes, it works out, although we're not really making use of its logic.  This bit is the part that's irrelevant for CLR EH:

```
    BasicBlock *FuncletUnwindDest;
    auto *FuncletPad =
        dyn_cast<FuncletPadInst>(FuncletEntryBB->getFirstNonPHI());
    assert(FuncletPad || FuncletEntryBB == &Fn->getEntryBlock());
    if (!FuncletPad)
      FuncletUnwindDest = nullptr;
    else if (auto *CatchPad = dyn_cast<CatchPadInst>(FuncletPad))
      FuncletUnwindDest = CatchPad->getCatchSwitch()->getUnwindDest();
    else if (auto *CleanupPad = dyn_cast<CleanupPadInst>(FuncletPad))
      FuncletUnwindDest = getCleanupRetUnwindDest(CleanupPad);
    else
      llvm_unreachable("unexpected funclet pad!");

    BasicBlock *InvokeUnwindDest = II->getUnwindDest();
    int BaseState = -1;
    if (FuncletUnwindDest == InvokeUnwindDest) {
      auto BaseStateI = FuncInfo.FuncletBaseStateMap.find(FuncletPad);
      if (BaseStateI != FuncInfo.FuncletBaseStateMap.end())
        BaseState = BaseStateI->second;
    }
```

You're right that `FuncletUnwindDest` will take on a meaningless value for cleanupret-less cleanups, but since we never populate `FuncletBaseStateMap` when targeting CLR, we never reach `BaseState = BaseStateI->second`, regardless of whether `FuncletUnwindDest` happens to match `InvokeUnwindDest`.


http://reviews.llvm.org/D15325





More information about the llvm-commits mailing list