This is an archive of the discontinued LLVM Phabricator instance.

[lldb] [debugserver] Add stN aliases for stmmN for compatibility
ClosedPublic

Authored by mgorny on Nov 20 2020, 12:47 AM.

Details

Summary

Add stN aliases for the FPU (stmmN) registers on MacOSX. This should
improve compatibility between MacOSX and other platforms, and fix
x86*-fp-write tests without having to duplicate them.

Diff Detail

Event Timeline

mgorny created this revision.Nov 20 2020, 12:47 AM
mgorny requested review of this revision.Nov 20 2020, 12:47 AM

Thanks for doing this. The changes look pretty straight-forward.

The main thing I think is missing here is the shelltest equivalent of @skipIfOutOfTreeDebugserver -- most people build/test lldb with the system debug server (because, reasons...) and so the test will still fail in those configurations...

mgorny updated this revision to Diff 306751.Nov 20 2020, 11:36 AM
mgorny edited the summary of this revision. (Show Details)

Added aliases to all regsets, not just the first ones. Updated test status.

amd64 tests still fail due to missing fip/fdp. I wonder how hard would it be to port it to Darwin.

Still looks good to me. :) Jason, do you have any concerns?

Unfortunately, we still need a @skipIfOutOfTreeDebugserver equivalent....

mgorny updated this revision to Diff 306986.Nov 23 2020, 1:37 AM

Rebase and fix TestRegisters.

Still looks good to me. :) Jason, do you have any concerns?

Unfortunately, we still need a @skipIfOutOfTreeDebugserver equivalent....

Long term, probably yes. Short term, the tests are broken again because of ftag changes, so I think we can get away without it.

That said, I need to test this more. I'll also try running tests with out-of-tree debugserver to see if anything else fails.

I would mark stmmX as an alias to stX and keep stX as the default for all platforms. stmmX could be an alias for everybody for legacy reasons.

mgorny updated this revision to Diff 307021.Nov 23 2020, 3:16 AM

Added system-debugserver feature to skip tests.

labath accepted this revision.Jan 6 2021, 5:54 AM

I think this is fine -- sorry this dropped off my radar.

@jasonmolenda, do you have any thoughts about this?

This revision is now accepted and ready to land.Jan 6 2021, 5:54 AM
labath added a comment.Jan 6 2021, 5:55 AM

I would mark stmmX as an alias to stX and keep stX as the default for all platforms. stmmX could be an alias for everybody for legacy reasons.

Possibly. Though we could also do that later, as the next step towards total deprecation...

mgorny added a comment.Jan 6 2021, 3:48 PM

I suppose this has waited long enough to deserve another test run.

Herald added a project: Restricted Project. · View Herald TranscriptJan 6 2021, 5:11 PM