This is an archive of the discontinued LLVM Phabricator instance.

[Frontend] Disallow non-MSVC exception models for windows-msvc targets
ClosedPublic

Authored by smeenai on Jun 6 2018, 4:41 PM.

Details

Summary

The windows-msvc target is used for MSVC ABI compatibility, including
the exceptions model. It doesn't make sense to pair a windows-msvc
target with a non-MSVC exception model. This would previously cause an
assertion failure; explicitly error out for it in the frontend instead.
This also allows us to reduce the matrix of target/exception models a
bit (see the modified tests), and we can possibly simplify some of the
personality code in a follow-up.

Diff Detail

Repository
rL LLVM

Event Timeline

smeenai created this revision.Jun 6 2018, 4:41 PM
mstorsjo added inline comments.Jun 7 2018, 2:00 AM
test/CodeGen/personality.c
10 ↗(On Diff #150223)

I'd prefer if you didn't remove these tests, but instead retarget them to use a -gnu triplet, to keep testing where you can explicitly choose between sjlj/dwarf/seh for mingw setups.

test/CodeGenCXX/personality.cpp
9 ↗(On Diff #150223)

Same here, please keep the existing tests but retarget them to gnu/mingw.

test/Frontend/windows-exceptions.cpp
19 ↗(On Diff #150223)

Ok, I see you're readding some sort of tests for the EH mode switching for mingw cases here, but you don't actually check that they produce the right thing here, only that it doesn't error out. So keeping the existing tests in the personality test files would probably be best.

smeenai marked 2 inline comments as done.Jun 7 2018, 12:57 PM
smeenai added inline comments.
test/CodeGen/personality.c
10 ↗(On Diff #150223)

I'll re-add those back; I didnt' realize they weren't already covered. I'm gonna drop the SEH_EXCEPTIONS thing for MinGW, since AFAIK SEH __try/__finally doesn't work there.

test/CodeGenCXX/personality.cpp
9 ↗(On Diff #150223)

This should all be covered by the group of RUN lines right below this one.

smeenai updated this revision to Diff 150394.Jun 7 2018, 12:59 PM
smeenai marked 2 inline comments as done.

Add back missing MinGW coverage

mstorsjo accepted this revision.Jun 7 2018, 1:10 PM

LGTM, thanks!

test/CodeGenCXX/personality.cpp
9 ↗(On Diff #150223)

Oh, right.

This revision is now accepted and ready to land.Jun 7 2018, 1:10 PM
rnk accepted this revision.Jun 7 2018, 3:42 PM

lgtm

This revision was automatically updated to reflect the committed changes.
cfe/trunk/test/CodeGen/personality.c