[Lldb-commits] [PATCH] D37420: [ABI] Rewrite RegisterIsCalleeSaved

[Davide Italiano via Phabricator via lldb-commits]

davide created this revision.

The goal of this patch is twofold:
First, it removes a wrong comment (at least, not correctly describing what the function does).
Then, it rewrites the function to use a stringswitch where the registers are enumerated explicitly instead of being computed programmatically. Other than being much shorter, it's much easier to read (and given the ABI won't change anytime soon, I don't think there's need to generalize).
While here, I added an assert that the register name is always empty, as the previous implementation of the function assumed so.

Do we want to add unittests for this to make sure the thing don't regress? I think it's a good thing to to anyway, but I wanted to reach consensus (and probably can be done in a follow-up commit).


https://reviews.llvm.org/D37420

Files:
  source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp


Index: source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
===================================================================
--- source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
+++ source/Plugins/ABI/SysV-x86_64/ABISysV_x86_64.cpp
@@ -14,6 +14,7 @@
 // Other libraries and framework includes
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/ADT/StringSwitch.h"
 
 // Project includes
 #include "lldb/Core/Module.h"
@@ -1908,52 +1909,17 @@
 // It's being revised & updated at https://github.com/hjl-tools/x86-psABI/
 
 bool ABISysV_x86_64::RegisterIsCalleeSaved(const RegisterInfo *reg_info) {
-  if (reg_info) {
-    // Preserved registers are :
-    //    rbx, rsp, rbp, r12, r13, r14, r15
-    //    mxcsr (partially preserved)
-    //    x87 control word
-
-    const char *name = reg_info->name;
-    if (name[0] == 'r') {
-      switch (name[1]) {
-      case '1': // r12, r13, r14, r15
-        if (name[2] >= '2' && name[2] <= '5')
-          return name[3] == '\0';
-        break;
-
-      default:
-        break;
-      }
-    }
-
-    // Accept shorter-variant versions, rbx/ebx, rip/ eip, etc.
-    if (name[0] == 'r' || name[0] == 'e') {
-      switch (name[1]) {
-      case 'b': // rbp, rbx
-        if (name[2] == 'p' || name[2] == 'x')
-          return name[3] == '\0';
-        break;
-
-      case 'i': // rip
-        if (name[2] == 'p')
-          return name[3] == '\0';
-        break;
-
-      case 's': // rsp
-        if (name[2] == 'p')
-          return name[3] == '\0';
-        break;
-      }
-    }
-    if (name[0] == 's' && name[1] == 'p' && name[2] == '\0') // sp
-      return true;
-    if (name[0] == 'f' && name[1] == 'p' && name[2] == '\0') // fp
-      return true;
-    if (name[0] == 'p' && name[1] == 'c' && name[2] == '\0') // pc
-      return true;
-  }
-  return false;
+  if (!reg_info)
+    return false;
+  assert(reg_info->name != nullptr && "unnamed register?");
+  std::string Name = std::string(reg_info->name);
+  bool IsCalleeSaved = llvm::StringSwitch<bool>(Name)
+    .Cases("r12", "r13", "r14", "r15",
+           "rbp", "rbx", "ebp", "ebx", true)
+    .Cases("rip", "eip", "rsp", "esp",
+           "sp", "fp", "pc", true)
+    .Default(false);
+  return IsCalleeSaved;
 }
 
 void ABISysV_x86_64::Initialize() {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D37420.113675.patch
Type: text/x-patch
Size: 2306 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170903/822d421d/attachment.bin>