jyu2 added a comment. In D150340#4343915 <https://reviews.llvm.org/D150340#4343915>, @efriedma wrote:
> LGTM... but I don't think the IR we're generating is really correct overall; > see https://github.com/llvm/llvm-project/issues/62723 > > On a side-note, other open issues related to -EHa/__try: > > https://github.com/llvm/llvm-project/issues/62606 > D124642 <https://reviews.llvm.org/D124642> Thanks Eli for the code review!! I will take look about missing seh.try.end in c++ try/catch part later. In term of D124642 <https://reviews.llvm.org/D124642>, I'd really want to look at IR for return inside __try/__finally, but for some reason, I an not build compiler with your patch: no member named 'setHasAddressTaken' in 'llvm::MachineBasicBlock'; did you mean 'hasAddressTaken'. I may missing some thing in my environment. ================ Comment at: clang/lib/CodeGen/CGException.cpp:650 + llvm::FunctionCallee SehCppScope = getSehTryBeginFn(CGM); + EmitSehScope(SehCppScope); + } ---------------- efriedma wrote: > jyu2 wrote: > > efriedma wrote: > > > Do we need to make the same change in EmitSEHTryStmt/ExitSEHTryStmt? > > > > > > Is there some reason not to just call EmitSehTryScopeBegin here? > > EmitSehTryScopeBegin: it is emit seh.scope.begin > > > > In here we want to emit seh.try.begin. call EmitSehSCope with different > > function. > > > > For EmitSEHTryStmt/ExitSEHTryStmt if is for __try /__except/__finally and > > it is for C code. I thought about that. And tried some test(BTW, try and > > __try can not be in same construct), I don't see the problem. So I did not > > add that. > Not sure I understand what you're saying about EmitSehTryScopeBegin. > CGException.cpp:45 refers to "llvm.seh.try.begin". CGCleanup.cpp:1370 also > refers to "llvm.seh.try.begin" > > For EmitSEHTryStmt/ExitSEHTryStmt, I guess __try codegen can't generate a > construct quite like the given testcase, so thta's fine. :-( Sorry, You are right!! It is seems EmitSehTryScopeBegin is not called anywhere. Changed. Thanks for the review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150340/new/ https://reviews.llvm.org/D150340 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
