CMP flags not working like SUB with signed numbers

[nestor ariel]

MVI A,001h
MVI B,0FEh
CMP B
SUB B

flags after CMP not the same as SUB.

[Debjit Biswas (debjitbis08)]

Hi,

Since CMP is nothing but SUB without setting the accumulator, I modified the CMP instruction code to match the SUB instruction using a different variable in place of the accumulator variable.

I would like to test whether the flags set during the subtract operations are correct or not. Can anyone suggest test cases?

Thanks,
Debjit

[Onkar Shinde (onkarshinde)]

Aanjhan,

Can we close this bug soon? I am hoping to do 1.3.7 release sometime in October first week.

[Aanjhan Ranganathan (aanjhan)]

Hi Debajit,

Your patch wont work for B = A condition (where Z flag must be rised).

reason: the check_and_set_flags function just uses the result passed to it. Since you pass "a" , a temp variable whose value does not hold the value after subtraction the zero flag wont be set.

This one is tricky as like medling around with any instruction related errors. Thanks for the patch. I am trying to base the changes on your idea.

If I am wrong, feel free to shoot.

Regards,
Aanjhan
PS: Patch will follow soon

[Aanjhan Ranganathan (aanjhan)]

And though the bug report has helped in cleaning up that instruction, I doubt if the flags must be the same after and before a SUB instruction. A SUB instruction affects the Accumulator content (a "result" is generated). hence will affect the S, P flags as well. So partially fixing bug with a commit.

If any discrepancy with my explanation feel free to shoot as always.

Regards,
Aanjhan

[Aanjhan Ranganathan (aanjhan)]

- Log -----------------------------------------------------------------
commit 5d0aafb931d03d7487accdb0d2d331e105a8d68b
Author: Aanjhan Ranganathan <email address hidden>
Date: Fri Sep 24 23:16:30 2010 +0200

   Fixes bug LP: #579320. See change log file for details.

diff --git a/ChangeLog b/ChangeLog
index 7681e26..6c5783b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2010-09-24 Aanjhan Ranganathan <email address hidden>

+ * src/8085-instructions.c: CMP instruction should not check and
+ set S, P and AC flags as there is no result that is computed. It
+ affects only C and Z flags. Hence removed most code in that
+ function. Fixes bug LP: #579320.
+
       * src/8085-instructions.c: Fixed the long standing DAA
       bug. Followed http://www.ray.masmcode.com/BCDdaa.html plus had to
       ensure the addition operation on the accumulator must be done
diff --git a/src/8085-instructions.c b/src/8085-instructions.c
index 4c5c042..157e0ad 100644
--- a/src/8085-instructions.c
+++ b/src/8085-instructions.c
@@ -502,19 +502,16 @@ _eef_inst_func_xra (eef_addr_t opnd_addr, gchar sec)
 static gint
 _eef_inst_func_cmp_i (eef_addr_t opnd_addr, eef_data_t data)
 {
- _eef_find_and_set_flags (sys.reg.a);
-
- sys.flag.s = (sys.reg.a < data);
-
- _eef_flag_check_and_set_aux_c (sys.reg.a, data, '-');

+ /* Since this is just comparison, there is no "result"
+ that affects the S, P and AC flags is present. Hence
+ no need to check and set flags */
  if (sys.reg.a < data)
- sys.flag.c = 1, sys.flag.z = 0;
+ sys.flag.c = 1, sys.flag.z = 0;
  else if (sys.reg.a > data)
- sys.flag.c = 0, sys.flag.z = 0;
+ sys.flag.c = 0, sys.flag.z = 0;
  else
- sys.flag.c = 0, sys.flag.z = 1;
-
+ sys.flag.c = 0, sys.flag.z = 1;
  return 0;
 }

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog | 5 +++++
 src/8085-instructions.c | 15 ++++++---------
 2 files changed, 11 insertions(+), 9 deletions(-)

[Debjit Biswas (debjitbis08)]

Hi Aanjhan,

Although there is no result, the flags should be checked and set/unset. The intel 8080 manual (i don't have a reference to 8085 at present) says that the CMP instruction affects the following flags: C, Z, S, P and AC.

From what I know, internally a subtraction is performed but without changing either of the operands. It is only logical to do the same thing here, perform a subtraction set/unset flags based on the result but do not save the result.

Thanks,
Debjit

[Debjit Biswas (debjitbis08)]

Hi Aanjhan,

The code at line 148 (in function _eef_find_and_set_flags) is:

sys.flag.z = (result == 0);

where result is passed to the function.

I am calling the function as _eef_find_and_set_flags (a); thus the zero flag will be set.

Thanks,
Debjit

[Onkar Shinde (onkarshinde)]

Aanjhan,

Can you please close this bug soon?

[Aanjhan Ranganathan (aanjhan)]

- Log -----------------------------------------------------------------
commit 45667ab9b4b1e11954396226ae8aedf38b874829
Author: Aanjhan Ranganathan <aanjhan@yoda.(none)>
Date: Mon Oct 11 17:46:43 2010 +0200

   Fixes CMP instruction bug #579320. Patch from Debjit Biswas
   added. Removed my changes done previously for fixing this bug. See
   ChangeLog for details.

diff --git a/src/8085-instructions.c b/src/8085-instructions.c
index 8811cc5..57f7d80 100644
--- a/src/8085-instructions.c
+++ b/src/8085-instructions.c
@@ -502,16 +502,12 @@ _eef_inst_func_xra (eef_addr_t opnd_addr, gchar sec)
 static gint
 _eef_inst_func_cmp_i (eef_addr_t opnd_addr, eef_data_t data)
 {
-
- /* Since this is just comparison, there is no "result"
- that affects the S, P and AC flags is present. Hence
- no need to check and set flags */
- if (sys.reg.a < data)
- sys.flag.c = 1, sys.flag.z = 0;
- else if (sys.reg.a > data)
- sys.flag.c = 0, sys.flag.z = 0;
- else
- sys.flag.c = 0, sys.flag.z = 1;
+ /* Patch from Debjit Biswas for CMP bug #579320 */
+ eef_data_t a = sys.reg.a;
+ _eef_flag_check_and_set_c (sys.reg.a, data, '-');
+ _eef_flag_check_and_set_aux_c (sys.reg.a, data, '-');
+ a += -1 * data;
+ _eef_find_and_set_flags (a);
  return 0;
 }