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

It would be nice to not pick UINT32_MAX for the unconditional condition and let 
each emulator picks it desired values. See inlined comments. Let me know what 
you think.


================
Comment at: include/lldb/Core/EmulateInstruction.h:406-410
@@ -405,4 +405,7 @@
 
-    virtual bool
-    IsInstructionConditional() { return false; }
+    // Returns a condition code in the for of uint32_t where UINT32_MAX means 
that the instruction
+    // is not conditional and other values specify the condition of the 
instruction in an
+    // architecture specififc way.
+    virtual uint32_t
+    GetInstructionCondition() { return UINT32_MAX; }
 
----------------
Add:
```
virtual uint32_t
GetUnconditionalCondition () { return 0; };
```

And change GetInstructionCondition to use the above function:

```
virtual uint32_t
GetInstructionCondition() { return GetUnconditionalCondition (); }
```

See inline comments below for more info...

================
Comment at: 
source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp:130
@@ +129,3 @@
+                // (UINT32_MAX means unconditional).
+                uint32_t last_condition = UINT32_MAX;
+                lldb::addr_t condition_block_start_offset = 0;
----------------
Maybe require the instruction emulator to give us an unconditional value here?

```
const uint32_t unconditional_condition = 
m_inst_emulator_ap->GetUnconditionalCondition();
uint32_t last_condition = unconditional_condition;
```

UINT32_MAX follows the ARM way of thinking, but other chips might have their 
own notion where zero is the unconditional value. It would be nice to not pick 
any default values for all instruction emulators.

================
Comment at: 
source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp:163
@@ +162,3 @@
+                        {
+                            if (m_inst_emulator_ap->GetInstructionCondition() 
!= UINT32_MAX &&
+                                saved_unwind_states.count(current_offset) == 0)
----------------
Change to:

```
if (m_inst_emulator_ap->GetInstructionCondition() != unconditional_condition &&
```

================
Comment at: 
source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp:175
@@ +174,3 @@
+                            // then the then current condition then restore 
the condition.
+                            if (last_condition != UINT32_MAX)
+                            {
----------------
Change to:

```
if (last_condition != unconditional_condition)
```


http://reviews.llvm.org/D16814



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to