Page MenuHomePhabricator

[lldb] [Process/FreeBSD] Introduce mips64 FPU reg support
Needs ReviewPublic

Authored by mgorny on Feb 16 2021, 2:56 AM.

Details

Diff Detail

Event Timeline

mgorny created this revision.Feb 16 2021, 2:56 AM
mgorny requested review of this revision.Feb 16 2021, 2:56 AM
jrtc27 added a subscriber: jrtc27.Feb 22 2021, 4:15 PM
jrtc27 added inline comments.
lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
141

MIPS calls it FCSR, FreeBSD's regnum.h calls it FSR. I don't know which is the right thing to use, but just thought I should point it out if you hadn't noticed.

161–167

These diffs should be separated from this commit IMO.

lldb/source/Plugins/Process/Utility/RegisterInfos_mips64.h
26

Should these not live in the respective RegisterContext<OS>_mips64.cpp?

37

This and DEFINE_GPR_INFO should not be defined like this on FreeBSD (or at all?).

Hm.... I'm very tempted to delete the linux mips implementation -- it uses several techniques which are getting in the way of this patch (and also some others in the past), for a lot of those, we now have different/better ways of doing it. On top of that, it's completely unmaintained (last non-nfc change to the file was in 2017, and the author (@nitesh.jain) does not appear to be active anymore.

mgorny marked an inline comment as done.Mar 2 2021, 11:24 AM

I'm sorry for not replying earlier.

lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
141

I have noticed. Others part of LLDB (and LLVM) call it FCSR, so I've figured out it's better to be consistent with that.

161–167

Do you mean reformatting? I suppose I can commit the result of clang-format earlier. Figured the changes are trivial enough.

lldb/source/Plugins/Process/Utility/RegisterInfos_mips64.h
26

I really don't know. I've followed suit with the other macros already here.

37

I suppose we could make them conditional too but I'm not sure if this wouldn't just add complexity for no real change.

Hm.... I'm very tempted to delete the linux mips implementation -- it uses several techniques which are getting in the way of this patch (and also some others in the past), for a lot of those, we now have different/better ways of doing it. On top of that, it's completely unmaintained (last non-nfc change to the file was in 2017, and the author (@nitesh.jain) does not appear to be active anymore.

Hi Labath,

As you notice, we no longer work for MIPS and not sure who is the current maintainer for the same.

Hm.... I'm very tempted to delete the linux mips implementation -- it uses several techniques which are getting in the way of this patch (and also some others in the past), for a lot of those, we now have different/better ways of doing it. On top of that, it's completely unmaintained (last non-nfc change to the file was in 2017, and the author (@nitesh.jain) does not appear to be active anymore.

Hi Labath,

As you notice, we no longer work for MIPS and not sure who is the current maintainer for the same.

Thanks for chiming in, Nitesh.

Email for linux/mips removal is here.