[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-30 Thread Aaron Smith via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4eabd0061254: [Windows SEH] Fix abnormal-exits in _try (authored by asmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77936/new/ https://reviews.llvm.o

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-16 Thread Ten Tzen via Phabricator via cfe-commits
tentzen updated this revision to Diff 257977. tentzen added a comment. remade a patch after re-sync Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77936/new/ https://reviews.llvm.org/D77936 Files: clang/lib/CodeGen/CGCleanup.cpp clang/lib/CodeG

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-15 Thread Ten Tzen via Phabricator via cfe-commits
tentzen updated this revision to Diff 257849. tentzen added a comment. Replace F_HasSehAbnormalExits with F_HasExitSwitch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77936/new/ https://reviews.llvm.org/D77936 Files: clang/lib/CodeGen/CGCleanup

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-13 Thread Ten Tzen via Phabricator via cfe-commits
tentzen marked 2 inline comments as done. tentzen added inline comments. Comment at: clang/lib/CodeGen/CGCleanup.cpp:848 // Build a switch-out if we need it: // - if there are branch-afters threaded through the scope rnk wrote: > Are we sure thi

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I like the implementation strategy, but I have one corner case, which is probably worth a test. Comment at: clang/lib/CodeGen/CGCleanup.cpp:848 // Build a switch-out if we need it: // - if there are branch-afters threaded through the scop

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77936/new/ https://reviews.llvm.org/D77936 ___

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-12 Thread Ten Tzen via Phabricator via cfe-commits
tentzen added a comment. In D77936#1976984 , @efriedma wrote: > I'm not concerned about the performance implications of whatever approach we > take here. In the worst case, an "icmp+zext" corresponds to two extra > arithmetic instructions; that's not en

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-12 Thread Ten Tzen via Phabricator via cfe-commits
tentzen updated this revision to Diff 256906. tentzen added a comment. Per Eli's suggestion, Use icmp (and zext) to check the JumpDest index, instead of directly passing the Index to _finally(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77936/n

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-12 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I'm not concerned about the performance implications of whatever approach we take here. In the worst case, an "icmp+zext" corresponds to two extra arithmetic instructions; that's not enough to matter. And I expect usually it'll get optimized away. I'd prefer to avoi

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-12 Thread Ten Tzen via Phabricator via cfe-commits
tentzen added a comment. In D77936#1976938 , @efriedma wrote: > Instead of asserting there are less than 256 cleanup destinations, can you > emit an icmp against zero, or something like that? I did consider that. but it splits the block and induces one

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-12 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Instead of asserting there are less than 256 cleanup destinations, can you emit an icmp against zero, or something like that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77936/new/ https://reviews.llvm.org/D77936 ___

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-12 Thread Ten Tzen via Phabricator via cfe-commits
tentzen updated this revision to Diff 256840. tentzen marked an inline comment as done. tentzen added a comment. Add option -fms-extensions in test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77936/new/ https://reviews.llvm.org/D77936 File

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-11 Thread Ten Tzen via Phabricator via cfe-commits
tentzen updated this revision to Diff 256836. tentzen marked an inline comment as done. tentzen added a comment. Per Eli's feedbacks: (1) a test case (windows-seh-abnormal-exit.c) is added under clang\test\CodeGen directory. (2) an assert is added to catch the extremely rare case that the numbe

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-11 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/CodeGen/CGException.cpp:1651 + llvm::Value* Load = CGF.Builder.CreateLoad(Addr, "cleanup.dest"); + IsForEH = CGF.Builder.CreateTrunc(Load, CGM.Int8Ty); +} Is just truncating the value really corr

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-11 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Needs a testcase in clang/test/CodeGen to verify the generated IR. I haven't looked at this code in a while, but this looks reasonable. Comment at: clang/lib/CodeGen/EHScopeStack.h:164 +F_IsEHCleanupKind = 0x4, +F_HasSehAbnormalExi

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

2020-04-11 Thread Ten Tzen via Phabricator via cfe-commits
tentzen created this revision. tentzen added reviewers: rnk, eli.friedman, JosephTremoulet, asmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. Per Windows SEH Spec, except _leave, all other early exits of a _try (goto/return/continue/break) are considered abnormal exit