[Lldb-commits] [PATCH] D108937: [lldb] [ABI/X86] Support combining xmm* and ymm*h regs into ymm*

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I like the solution with multiple value_regs.

However, I am wondering if instead of the various piecemeal maps we couldn't just have one uber-map to rule them all. I'm imagining something like

  enum (class?) RegKind { GPR, FPR, XMM, ...};
  struct RegData {
    size_t index;
    StringRef name;
    SmallVector<StringRef> subregs;
  };
  
  map<pair<RegKind, unsigned>, RegData> reg_map = makeRegMap(); // whichever kind of a map
  // or possibly, map<RegKind, map<unsigned, RegData>> = ...
  
  for (const auto &kv : reg_map)
    subreg_name_set.insert(kv.second.subregs.begin(), kv.second.subregs.end());
  
  for (const auto &x : llvm::enumerate(regs)) {
    llvm::StringRef reg_name = x.value().name.GetStringRef();
    if (llvm::is_contained(subreg_name_set, reg_name))
      return;
    if (Optional<pair<RegKind, size_t>> reg = get_reg(reg_name))
      reg_map[reg].index = x.index(); 
  }
  
  ...

This would allow us to factor out two tedious jobs (constructing the register map and identifying registers) into separate functions, leaving only the only (simplified, I hope) core logic for the main function. The `get_reg` function could use whichever method is easiest for the translation of register names, or even a combination of methods (a StringSwitch for gprs and some consume_fronts for the regularly-named registers).

WDYT?



================
Comment at: lldb/source/Plugins/ABI/X86/ABIX86.cpp:171
+      "xmm14",
+      "xmm15",
+  }};
----------------
For a simple list like this, I'd probably remove the trailing comma to have clang-format format it more succinctly.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108937/new/

https://reviews.llvm.org/D108937