[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

[Ten Tzen via Phabricator via cfe-commits]

tentzen marked 4 inline comments as done.
tentzen added inline comments.


================
Comment at: clang/lib/CodeGen/CGException.cpp:1798
+
+    // if the parent is a _finally, need to retrive Establisher's FP,
+    //  2nd paramenter, saved & named frame_pointer in parent's frame
----------------
efriedma wrote:
> Maybe worth expanding this comment a little, to explain that a "finally" should have at most one localescape'ed variable.  (At least, that's my understanding.)
ok, will add more explanation.


================
Comment at: clang/lib/CodeGen/CGException.cpp:1800
+    //  2nd paramenter, saved & named frame_pointer in parent's frame
+    if (ParentCGF.ParentCGF != NULL) {
+      // Locate and escape Parent's frame_pointer.addr alloca
----------------
efriedma wrote:
> nullptr
> 
> Do you actually use ParentCGF.ParentCGF anywhere?  If not, do you really need to save it?
Yes, I used it in other patches.  I found it's very handy to have a way to access parent CGF & Function.


================
Comment at: clang/lib/CodeGen/CGException.cpp:1805
+        llvm::AllocaInst *II = dyn_cast<llvm::AllocaInst>(&I);
+        if (II && II->getName().startswith("frame_pointer")) {
+          FramePtrAddrAlloca = II;
----------------
efriedma wrote:
> Using the name isn't reliable. You should be using data stored somewhere in the CodeGenFunction or something like that.
good sense.  will update it.


================
Comment at: clang/lib/CodeGen/CGException.cpp:1822
+      llvm::Function *FrameRecoverFn = llvm::Intrinsic::getDeclaration(
+          &CGM.getModule(), llvm::Intrinsic::localrecover);
+      llvm::Constant *ParentI8Fn =
----------------
efriedma wrote:
> Is there some reason you can't reuse recoverAddrOfEscapedLocal here?
because recoverAddrOfEscapedLocal() is used to get escaped locals of outermost frame.  The escaped frame-pointer we are looking for here is in _finally frame, not outermost frame.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77982/new/

https://reviews.llvm.org/D77982