[Lldb-commits] [PATCH] D55718: [ARC] Basic support in gdb-remote process plugin

Greg Clayton via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Dec 18 13:39:31 PST 2018

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?




More information about the lldb-commits mailing list