Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable test ObjCXXEHInterop_arc on Windows for Clang 16+ #233

Conversation

weliveindetail
Copy link
Contributor

This test used to fail on Windows, because Clang's codegen and transformation haven't been in-line with the requirements of the WinEH backend in LLVM. The following changes implement the missing pieces and let the test to pass in both debug and release mode:

https://reviews.llvm.org/D128190 [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls in the course of IR transforms
https://reviews.llvm.org/D134441 [ObjC][ARC] Fix target register for call expanded from CALL_RVMARKER on Windows
https://reviews.llvm.org/D137939 [CGObjC] Open cleanup scope before SaveAndRestore CurrentFuncletPad and push CatchRetScope early
https://reviews.llvm.org/D137944 [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens

This test used to fail on Windows, because Clang's codegen and transformation haven't been in-line with the requirements of the WinEH backend in LLVM. The following changes implement the missing pieces and let the test to pass in both debug and release mode:

https://reviews.llvm.org/D128190 [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls in the course of IR transforms
https://reviews.llvm.org/D134441 [ObjC][ARC] Fix target register for call expanded from CALL_RVMARKER on Windows
https://reviews.llvm.org/D137939 [CGObjC] Open cleanup scope before SaveAndRestore CurrentFuncletPad and push CatchRetScope early
https://reviews.llvm.org/D137944 [ObjC][ARC] Teach the OptimizeSequences step of ObjCARCOpts about WinEH funclet tokens
@weliveindetail
Copy link
Contributor Author

This should be ready once D137939 and D137944 landed

@weliveindetail
Copy link
Contributor Author

The following tests FAILED:
	  7 - ARCTest_arc
	  8 - ARCTest_arc_optimised
	 41 - PropertyIntrospectionTest2_arc
	 42 - PropertyIntrospectionTest2_arc_optimised

These tests will pass in release mode as well with Clang 16+

@davidchisnall
Copy link
Member

Can you disable those for clang <16 and then we can merge this and have a follow-on to bump the clang version in CI once it's released?

@weliveindetail
Copy link
Contributor Author

weliveindetail commented Nov 21, 2022

This should be ready to merge now. Thx
Btw this is addressing #222

@weliveindetail weliveindetail marked this pull request as ready for review November 21, 2022 18:09
@davidchisnall davidchisnall merged commit 8c600f5 into gnustep:master Dec 14, 2022
@weliveindetail
Copy link
Contributor Author

First 16.x Release Candidates are out: https://github.com/llvm/llvm-project/releases It might be worth double-checking that my CMake changes do enable the tests on Windows with Clang 16 and that they actually succeed in release mode (and not only work for me).

@triplef
Copy link
Member

triplef commented Feb 20, 2023

Confirmed with Clang 16 rc2:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants