[Lldb-commits] [PATCH] D17535: [LLDB][MIPS] Single step atomic sequences

[Pavel Labath via lldb-commits]

labath added a subscriber: labath.
labath added a comment.

I'll leave the final review to Tamas, just a couple of comments here.


================
Comment at: packages/Python/lldbsuite/test/functionalities/single_step_atomic_sequence/TestStepInAtomicSequence.py:70
@@ +69,3 @@
+            inst_opcode = inst.GetMnemonic(target)
+            if inst_opcode in self.atomic_start_insns:
+                # Get the address of instruction starting atomic sequence
----------------
If you don't find the instructions you are looking for (perhaps due to a compiler change), the test will end up being a no-op. You might want to assert that you actually find the instructions you are looking for.

================
Comment at: packages/Python/lldbsuite/test/functionalities/single_step_atomic_sequence/TestStepInAtomicSequence.py:94
@@ +93,3 @@
+
+                print ("Sequence Starts at 0x%x:"% start_of_seq)
+                print ("Sequence ends at 0x%x:"% end_of_seq)
----------------
These kinds of debug statements are usually made conditional on `self.TraceOn()` (`-t` flag to dotest), to minimize the noise it the general case when you're not debugging this test.

================
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:1689
@@ -1677,6 +1688,3 @@
     }
-    else if (m_arch.GetMachine() == llvm::Triple::mips64
-            || m_arch.GetMachine() == llvm::Triple::mips64el
-            || m_arch.GetMachine() == llvm::Triple::mips
-            || m_arch.GetMachine() == llvm::Triple::mipsel)
-        error = SetSoftwareBreakpoint(next_pc, 4);
+    else if (arch_machine == llvm::Triple::mips64 ||
+             arch_machine == llvm::Triple::mips64el ||
----------------
I've been seeing this pattern a lot lately. Isn't it time to make a `isMipsArchitecture` utility function somewhere?


Repository:
  rL LLVM

http://reviews.llvm.org/D17535