clayborg added a comment.
See inline comments and let me know what you think.
================
Comment at: include/lldb/Utility/ArchSpec.h:91-92
+ // ARC configuration flags
+ enum ARCflags { eARC_rf32 = 0 /*0b00*/, eARC_rf16 = 2 /*0b10*/ };
+
----------------
Since no other place needs these flags, it would be nice to define them in
ProcessGDBRemote.cpp and remove them from here? Eventually we should get rid of
all flags in here and move then to the Architecture.h subclass headers.
================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4549
+ // The target is configured to use reduced register file.
+ arch_to_use.SetFlags(ArchSpec::eARC_rf16);
+ // ABI uses this information to determine how many registers it may
----------------
Do we need to set this flags in the arch_to_use still? The eARC_rf16 only seems
to be used in this file. If you plan on using eARC_rf16 in other parts of the
code, then we should define it in ArchSpec as you have it, otherwise we should
remove it and avoid polluting that header if possible
================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4564
+ if (LLDB_INVALID_REGNUM == reg_num) {
+ if (ArchSpec::eARC_rf16 & arch.GetFlags()) {
+ // In reduced mode we don't have r4-r9, r16-r25.
----------------
the rf16 could be passed into this function as an argument and then the enum
can be removed from ArchSpec.h?
================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4704-4707
+ if (!arc::ConfigureArchitecture(*this, m_register_info, arch_to_use))
+ return false;
+
+ arc::AdjustRegisterInfo(m_register_info, arch_to_use);
----------------
This logic still seems possibly incorrect as we will skip the finalize call
below? Shouldn't this just be:
```
if (arc::ConfigureArchitecture(*this, m_register_info, arch_to_use))
arc::AdjustRegisterInfo(m_register_info, arch_to_use);
```
And fall through to below?
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55718/new/
https://reviews.llvm.org/D55718
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
