I am afraid I don't know much about exceptions on Windows in general. A few comments below.

LGTM

Thanks!

@thanm: can you please confirm that D53698 is the behavior you were testing?

LGTM, thanks for looking into this Reid!

Thanks Hal!

Is there anything left to discuss or is this ready to land?

• Remove some out-of-date comments.

And...update the tests.

Address Jessica's latest comments.

Fix some spacing issues.

Address Jonas' comments.

Thanks Jessica for the review.

Thanks Jonas for the review.

• Fixed bugs where physregs were involved and were treated as having an unique def.
• Added more tests.
• Try harder to check the registers by looking through all the vregs until a physreg is found. Then compare definitions or check if it's part of the liveins list.

LGTM. It would be nice to have a test too.

Still LGTM.

Can you please also add support for the new CFI directive in MIR? Mostly in lib/CodeGen/MIRParser/MIParser.cpp, lib/CodeGen/MIRParser/MILexer.cpp/h, lib/CodeGen/MIRPrinter.cpp and test/CodeGen/MIR/AArch64/cfi.mir.

LGTM, thanks for improving this!

LGTM, thanks!

I did a quick experiment to avoid storing the FrameIndex and get rid of the FrameAccess struct, by using this:

Thanks @sdesmalen, LGTM!

Added a test. Let me know if that's what you had in mind.

Looks good, thanks!

Could you also add what you said in the previous comment to the test case?

Thanks for working on this! Other than the comments I left it looks good to me.

Other than the comments it LGTM, thanks!

Thanks a lot Matthias, this looks really great! I hopefully did not miss anything, it LGTM!

Thank you! A few comments on the MIR test though. Thanks for putting up the test.

Sorry, not sure I can help much here. I haven't looked at the new PM yet.

Looks better now. Thanks for adding the MIR test.

This looks ok, but I am not sure the FrameSetup flag was made to be used in this way.

I fixed a couple of issues in https://reviews.llvm.org/rL340034. Let me know if that doesn't work for you.

Sorry, I'm afraid I can't help reviewing this patch as I know nothing about MachineTraceMetrics. Although, I suggest adding a test case, as it will help reviewers to understand better what the changes are.

Ping

In D43136#1199886, @george.karpenkov wrote:

I'm not sure whether anyone else would object ( @MatzeB @ab @thegameg ?)

Please add a MIR test for this, it would make the test much clearer.

Looks like it's fixed on trunk. Could this be fixed by: https://reviews.llvm.org/rL329691, https://reviews.llvm.org/rL329797, https://reviews.llvm.org/rL329805 and https://reviews.llvm.org/rL330976?