[Lldb-commits] [PATCH] D12079: [MIPS] microMIPS breakpoints, disassembly and compressed addresses

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

A few more little things with respect to not calling accessors multiple times in if statements and this will be good to go.


================
Comment at: source/Core/Address.cpp:474
@@ +473,3 @@
+            {
+                ArchSpec arch = target->GetArchitecture();
+                if (arch.GetMachine() == llvm::Triple::mips || arch.GetMachine() == llvm::Triple::mipsel
----------------
Change this to be:

```
const llvm::Triple::ArchType llvm_arch = target->GetArchitecture().GetMachine();
```

and use llvm_arch in if statement below instead of calling accessor 4 times.

================
Comment at: source/Plugins/Disassembler/llvm/DisassemblerLLVMC.cpp:750-763
@@ -753,11 +749,16 @@
     // For arm CPUs that can execute arm or thumb instructions, also create a thumb instruction disassembler.
     if (triple.getArch() == llvm::Triple::arm)
     {
         std::string thumb_triple(thumb_arch.GetTriple().getTriple());
         m_alternate_disasm_ap.reset(new LLVMCDisassembler(thumb_triple.c_str(), "", "", flavor, *this));
         if (!m_alternate_disasm_ap->IsValid())
         {
             m_disasm_ap.reset();
             m_alternate_disasm_ap.reset();
         }
     }
+    else if (arch.GetTriple().getArch() == llvm::Triple::mips
+            || arch.GetTriple().getArch() == llvm::Triple::mipsel
+            || arch.GetTriple().getArch() == llvm::Triple::mips64
+            || arch.GetTriple().getArch() == llvm::Triple::mips64el)
+    {
----------------
We should store the llvm::Triple::ArchType into a local const variable up on line 750 and use it on line 750 and in this "else if". Also note that we have "triple" which is already the "arch.GetTriple()".

================
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2094-2095
@@ +2093,4 @@
+            */
+            if (arch.GetMachine() == llvm::Triple::mips || arch.GetMachine() == llvm::Triple::mipsel
+                || arch.GetMachine() == llvm::Triple::mips64 || arch.GetMachine() == llvm::Triple::mips64el)
+            {
----------------
```
const llvm::Triple::ArchType llvm_arch = target->GetArchitecture().GetMachine();
```

Then use llvm_arch in the if statement.


Repository:
  rL LLVM

http://reviews.llvm.org/D12079