Re: [Simh] DAA Emulation

[Richard Cini]

Al --

Thanks a lot for the email. I¹m sure you looked at this, but I pulled
the Intel 8080 Users Manual and under DAA, it says the following (snipping a
bit):

The 8-bit number in the accumulator is adjusted to form two 4-bit [BCD]
digits by the following process:

(1) If the value of the least significant 4 bits is greater than 9
or if the AC flag is set, 6 is added to the accumulator.
(2) if the value of the most significant 4 bits is now greater than
9 or if the CY flag is set, 6 is added to the most significant 4 bits of the
accumulator.

Based on this, I would say that the second part of the test in the
Altair32 code is wrong. Further, it looks like the SIMH code may be wrong as
well because it doesn¹t test the CY flag in the second test.

As far as the register display, I¹ll make that change ‹ oddly no one has
ever reported it.

Thanks again for locating this bug.

Rich

--
Rich Cini
Collector of Classic Computers
Build Master and lead engineer, Altair32 Emulator
http://www.altair32.com
http://www.classiccmp.org/cini



On 12/11/10 1:32 AM, "Al Williams"  wrote:

> Hi Rich and Bob,
> 
> I've been doing some work on Vince Briel's excellent AVR emulation of the 8080
> and while rewriting DAA I came across what I think to be a harmless bug but
> thought you might want to comment on it.
> 
> From what I can glean DAA effects all flags including half carry. And I
> _think_ that half carry occurs from the +6 (if it happens at all). So if you
> don't adjust the LSD you get AC=0. If you add 6 then if adding the 6 gives you
> a carry out of Bit 3 you get AC set. Note that the carry might ripple so
> that's NOT to say Bit 4 is necessarily 1.
> 
> Here's part of Altair32's code:
> 
> static void daa ( OP_ARG_U ) 
> {
> // Decimal Adjust Accumulator
> // DAA:: A=BCD format
> // Flags: SZAPC
> // *
> 
> register /* FJS */ word tmp = ACCUM;
> 
> if ( (( tmp & 0x0f ) > 0x09 ) || ( FLAGS & AC_FLAG ))
> tmp += 0x06;
> 
> if (tmp > 0x0f)
> FLAGS |= AC_FLAG; // if adjusted LSB > 0xf, set AC
> else
> FLAGS &= ~AC_FLAG; // else clear SC
> 
> 
> So since tmp is not masked off, any value >0xF gets AC set even if no carry or
> add occurred! In other words, pretend the value of ACCUM is 90 (and thus temp
> is 90) with AC=0. The first if does not fire. The second if does and AC gets
> set. That's got to be wrong. Granted, who checks AC after DAA? But still 
> 
> 
> So why copy Bob? Well... I think SIMH has a similar but different problem.
> Here's a snip from the DAA code:
> 
> /*opcode=0x27*/
> static void i86op_daa(PC_ENV *m)
> {
>    uint16 dbyte;
>    dbyte = m->R_AL;
>    if (ACCESS_FLAG(m,F_AF)|| (dbyte&0xf) > 9)
>      {
>     dbyte += 6;
>     if (dbyte&0x100)
>       SET_FLAG(m, F_CF);
>     SET_FLAG(m, F_AF);
>      }
>    else
> 
> 
> Here we add 6 to dbyte and then AC is always set. If no +6 then AC is cleared.
> This COULD be correct behavior, but I can't find any reference material that
> says it is. In any event, SIMH and Altair32 are doing something different
> here, so they both can't be right. Meanwhile I have my own version of DAA in
> AVR assembler that I will spare you unless you ask. The only real silicon I
> have even close to operational at the moment is a Z80 and not only is it not
> operational, the DAA is one place where it is a lot different so I don't trust
> the result there.
> 
> Oh. One other note about Altair32. In the debugger, the A and FLAGS display is
> swapped in the debug console window. The Register display up top left is ok
> though. I bet you've heard that one before.
> 
> If either of you can show documentation on the AC flag after DAA or point to a
> real piece of silicon's behavior I'd like to know so I can fix the DAA code in
> the Briel emulator to match. Otherwise, I thought you'd like to know about
> this bug even though it is pretty innocuous as far as I can tell.
> 
> 
> Al Williams
> http://www.ddj.com/embedded (among others)
> 
> 
> 
> 

___
Simh mailing list
Simh@trailing-edge.com
http://mailman.trailing-edge.com/mailman/listinfo/simh

Re: [Simh] DAA Emulation

[Al Williams]

Yeah I've read the docs, but even what you pasted in doesn't directly answer
the question: What is the state of the AC flag after a DAA? So I think the
code correctly implements the additions you mentioned. But I think we all 3
disagree on the AC flag code. Notice I didn't copy the whole function in
either case, so the handling of the MSD is "not shown".




On Sat, Dec 11, 2010 at 8:16 AM, Richard Cini  wrote:

>  Al --
>
> Thanks a lot for the email. I’m sure you looked at this, but I pulled
> the Intel 8080 Users Manual and under DAA, it says the following (snipping a
> bit):
>
> The 8-bit number in the accumulator is adjusted to form two 4-bit [BCD]
> digits by the following process:
>
> (1) If the value of the least significant 4 bits is greater than 9
> *or *if the AC flag is set, 6 is added to the accumulator.
> (2) if the value of the most significant 4 bits is *now *greater
> than 9 *or *if the CY flag is set, 6 is added to the most significant 4
> bits of the accumulator.
>
> Based on this, I would say that the second part of the test in the
> Altair32 code is wrong. Further, it looks like the SIMH code may be wrong as
> well because it doesn’t test the CY flag in the second test.
>
> As far as the register display, I’ll make that change — oddly no one
> has ever reported it.
>
> Thanks again for locating this bug.
>
> Rich
>
> --
> Rich Cini
> Collector of Classic Computers
> Build Master and lead engineer, Altair32 Emulator
> http://www.altair32.com
> http://www.classiccmp.org/cini
>
>
>
>
> On 12/11/10 1:32 AM, "Al Williams"  wrote:
>
> Hi Rich and Bob,
>
> I've been doing some work on Vince Briel's excellent AVR emulation of the
> 8080 and while rewriting DAA I came across what I think to be a harmless bug
> but thought you might want to comment on it.
>
> From what I can glean DAA effects all flags including half carry. And I
> _think_ that half carry occurs from the +6 (if it happens at all). So if you
> don't adjust the LSD you get AC=0. If you add 6 then if adding the 6 gives
> you a carry out of Bit 3 you get AC set. Note that the carry might ripple so
> that's NOT to say Bit 4 is necessarily 1.
>
> Here's part of Altair32's code:
>
> static void daa ( OP_ARG_U )
> {
> // Decimal Adjust Accumulator
> // DAA:: A=BCD format
> // Flags: SZAPC
> // *
>
> register /* FJS */ word tmp = ACCUM;
>
> if ( (( tmp & 0x0f ) > 0x09 ) || ( FLAGS & AC_FLAG ))
> tmp += 0x06;
>
> if (tmp > 0x0f)
> FLAGS |= AC_FLAG; // if adjusted LSB > 0xf, set AC
> else
> FLAGS &= ~AC_FLAG; // else clear SC
>
>
> So since tmp is not masked off, any value >0xF gets AC set even if no carry
> or add occurred! In other words, pretend the value of ACCUM is 90 (and thus
> temp is 90) with AC=0. The first if does not fire. The second if does and AC
> gets set. That's got to be wrong. Granted, who checks AC after DAA? But
> still
>
>
> So why copy Bob? Well... I think SIMH has a similar but different problem.
> Here's a snip from the DAA code:
>
> /*opcode=0x27*/
> static void i86op_daa(PC_ENV *m)
> {
>uint16 dbyte;
>dbyte = m->R_AL;
>if (ACCESS_FLAG(m,F_AF)|| (dbyte&0xf) > 9)
>  {
> dbyte += 6;
> if (dbyte&0x100)
>   SET_FLAG(m, F_CF);
> SET_FLAG(m, F_AF);
>  }
>else
>
>
> Here we add 6 to dbyte and then AC is always set. If no +6 then AC is
> cleared. This COULD be correct behavior, but I can't find any reference
> material that says it is. In any event, SIMH and Altair32 are doing
> something different here, so they both can't be right. Meanwhile I have my
> own version of DAA in AVR assembler that I will spare you unless you ask.
> The only real silicon I have even close to operational at the moment is a
> Z80 and not only is it not operational, the DAA is one place where it is a
> lot different so I don't trust the result there.
>
> Oh. One other note about Altair32. In the debugger, the A and FLAGS display
> is swapped in the debug console window. The Register display up top left is
> ok though. I bet you've heard that one before.
>
> If either of you can show documentation on the AC flag after DAA or point
> to a real piece of silicon's behavior I'd like to know so I can fix the DAA
> code in the Briel emulator to match. Otherwise, I thought you'd like to know
> about this bug even though it is pretty innocuous as far as I can tell.
>
>
> Al Williams
> http://www.ddj.com/embedded (among others)
>
>
>
>
>
___
Simh mailing list
Simh@trailing-edge.com
http://mailman.trailing-edge.com/mailman/listinfo/simh

Re: [Simh] DAA Emulation

[Timothe Litt]

According to the Sep 75 Intel 8080 Microcomputer Systems manual:
 
P 4-3: Auxiliary Carry: If the instruction caused a carry out of bit 3 and
into bit 4 of the resulting value, the auxiliary carry is set; otherwise it
is reset.  This flag is affected by single precision additions,
subtractions, increments, decrements, comparisons and logical operations,
but is principally used with additions and increments preceding a DAA
(Decimal Adjust Accumulator) instruction.
 
P 4-8: DAA description has the text previously cited, plus
NOTE: All flags are affected
Cycles: 1
States: 4
Flags: Z,S,P,CY,AC
 
I read this as AC should be set if the first add 6 (to the LSB of A)
happened and caused a carry and should be cleared otherwise.  (Only this add
can cause a carry from bit 3 to bit 4.)
 



-
This communication may not represent my employer's views,
if any, on the matters discussed.
  

 

  _  

From: simh-boun...@trailing-edge.com [mailto:simh-boun...@trailing-edge.com]
On Behalf Of Al Williams
Sent: Saturday, December 11, 2010 10:30
To: Richard Cini
Cc: simh@trailing-edge.com; Vince Briel
Subject: Re: [Simh] DAA Emulation


Yeah I've read the docs, but even what you pasted in doesn't directly answer
the question: What is the state of the AC flag after a DAA? So I think the
code correctly implements the additions you mentioned. But I think we all 3
disagree on the AC flag code. Notice I didn't copy the whole function in
either case, so the handling of the MSD is "not shown". 




On Sat, Dec 11, 2010 at 8:16 AM, Richard Cini  wrote:


Al --

Thanks a lot for the email. I'm sure you looked at this, but I pulled
the Intel 8080 Users Manual and under DAA, it says the following (snipping a
bit):

The 8-bit number in the accumulator is adjusted to form two 4-bit [BCD]
digits by the following process:

(1) If the value of the least significant 4 bits is greater than 9
or if the AC flag is set, 6 is added to the accumulator.
(2) if the value of the most significant 4 bits is now greater than
9 or if the CY flag is set, 6 is added to the most significant 4 bits of the
accumulator.

Based on this, I would say that the second part of the test in the
Altair32 code is wrong. Further, it looks like the SIMH code may be wrong as
well because it doesn't test the CY flag in the second test.

As far as the register display, I'll make that change - oddly no one has
ever reported it.

Thanks again for locating this bug.

Rich

--
Rich Cini
Collector of Classic Computers
Build Master and lead engineer, Altair32 Emulator
http://www.altair32.com
http://www.classiccmp.org/cini 




On 12/11/10 1:32 AM, "Al Williams"  wrote:



Hi Rich and Bob,

I've been doing some work on Vince Briel's excellent AVR emulation of the
8080 and while rewriting DAA I came across what I think to be a harmless bug
but thought you might want to comment on it.

>From what I can glean DAA effects all flags including half carry. And I
_think_ that half carry occurs from the +6 (if it happens at all). So if you
don't adjust the LSD you get AC=0. If you add 6 then if adding the 6 gives
you a carry out of Bit 3 you get AC set. Note that the carry might ripple so
that's NOT to say Bit 4 is necessarily 1.

Here's part of Altair32's code:

static void daa ( OP_ARG_U ) 
{
// Decimal Adjust Accumulator
// DAA:: A=BCD format
// Flags: SZAPC
// *

register /* FJS */ word tmp = ACCUM;

if ( (( tmp & 0x0f ) > 0x09 ) || ( FLAGS & AC_FLAG ))
tmp += 0x06;

if (tmp > 0x0f)
FLAGS |= AC_FLAG; // if adjusted LSB > 0xf, set AC
else
FLAGS &= ~AC_FLAG; // else clear SC 


So since tmp is not masked off, any value >0xF gets AC set even if no carry
or add occurred! In other words, pretend the value of ACCUM is 90 (and thus
temp is 90) with AC=0. The first if does not fire. The second if does and AC
gets set. That's got to be wrong. Granted, who checks AC after DAA? But
still 


So why copy Bob? Well... I think SIMH has a similar but different problem.
Here's a snip from the DAA code:

/*opcode=0x27*/
static void i86op_daa(PC_ENV *m)
{
   uint16 dbyte;
   dbyte = m->R_AL;
   if (ACCESS_FLAG(m,F_AF)|| (dbyte&0xf) > 9)
 {
dbyte += 6;
if (dbyte&0x100)
  SET_FLAG(m, F_CF);
SET_FLAG(m, F_AF);
 }
   else


Here we add 6 to dbyte and then AC is always set. If no +6 then AC is
cleared. This COULD be correct behavior, but I can't find any reference
material that says it is. In any event, SIMH and Altair32 are doing
something different here, so they both can't be right. Meanwhile I have my
own version of DAA in AVR assembler that I will spare you unless you ask.
The only real silicon I have even close to operational at the moment is a
Z80 and not only is it not operational, the DAA is one place where it is a
lot different so I don't trust the result there.

Oh. One other note about Altair32. In the debugger, the A and FLAGS display
is swapped in the debug c

Re: [Simh] DAA Emulation

[Al Williams]

I agree with your interpretation and by that interpretation the code in both
emulators is currently busted. In the case of SIMH adding 6 does not always
result in a half carry.

Granted if LSD is >9 it will cause AC to set. But in the case when AC is
already set this isn't always the case. Assuming the last add was BCD you
could have X9+Y9 = [X+Y+1]2.

So the LSD is 2 and AC is set. Adding 6 gives you 8 and no half carry. So
setting the flag every time you add 6 is incorrect.


On Sat, Dec 11, 2010 at 10:51 AM, Timothe Litt  wrote:

>  According to the Sep 75 Intel 8080 Microcomputer Systems manual:
>
> P 4-3: Auxiliary Carry: If the instruction caused a carry out of bit 3 and
> into bit 4 of the resulting value, the auxiliary carry is set; otherwise it
> is reset.  This flag is affected by single precision additions,
> subtractions, increments, decrements, comparisons and logical operations,
> but is principally used with additions and increments preceding a DAA
> (Decimal Adjust Accumulator) instruction.
>
> P 4-8: DAA description has the text previously cited, plus
> NOTE: All flags are affected
> Cycles: 1
> States: 4
> Flags: Z,S,P,CY,AC
>
> I read this as AC should be set if the first add 6 (to the LSB of A)
> happened and caused a carry and should be cleared otherwise.  (Only this add
> can cause a carry from bit 3 to bit 4.)
>
>
> -
> This communication may not represent my employer's views,
> if any, on the matters discussed.
>
>
>
>  --
> *From:* simh-boun...@trailing-edge.com [mailto:
> simh-boun...@trailing-edge.com] *On Behalf Of *Al Williams
> *Sent:* Saturday, December 11, 2010 10:30
> *To:* Richard Cini
> *Cc:* simh@trailing-edge.com; Vince Briel
> *Subject:* Re: [Simh] DAA Emulation
>
> Yeah I've read the docs, but even what you pasted in doesn't directly
> answer the question: What is the state of the AC flag after a DAA? So I
> think the code correctly implements the additions you mentioned. But I think
> we all 3 disagree on the AC flag code. Notice I didn't copy the whole
> function in either case, so the handling of the MSD is "not shown".
>
>
>
>
> On Sat, Dec 11, 2010 at 8:16 AM, Richard Cini wrote:
>
>> Al --
>>
>> Thanks a lot for the email. I’m sure you looked at this, but I pulled
>> the Intel 8080 Users Manual and under DAA, it says the following (snipping a
>> bit):
>>
>> The 8-bit number in the accumulator is adjusted to form two 4-bit
>> [BCD] digits by the following process:
>>
>> (1) If the value of the least significant 4 bits is greater than 9
>> *or *if the AC flag is set, 6 is added to the accumulator.
>> (2) if the value of the most significant 4 bits is *now *greater
>> than 9 *or *if the CY flag is set, 6 is added to the most significant 4
>> bits of the accumulator.
>>
>> Based on this, I would say that the second part of the test in the
>> Altair32 code is wrong. Further, it looks like the SIMH code may be wrong as
>> well because it doesn’t test the CY flag in the second test.
>>
>> As far as the register display, I’ll make that change — oddly no one
>> has ever reported it.
>>
>> Thanks again for locating this bug.
>>
>> Rich
>>
>> --
>> Rich Cini
>> Collector of Classic Computers
>> Build Master and lead engineer, Altair32 Emulator
>> http://www.altair32.com
>> http://www.classiccmp.org/cini
>>
>>
>>
>>
>> On 12/11/10 1:32 AM, "Al Williams"  wrote:
>>
>>   Hi Rich and Bob,
>>
>> I've been doing some work on Vince Briel's excellent AVR emulation of the
>> 8080 and while rewriting DAA I came across what I think to be a harmless bug
>> but thought you might want to comment on it.
>>
>> >From what I can glean DAA effects all flags including half carry. And I
>> _think_ that half carry occurs from the +6 (if it happens at all). So if you
>> don't adjust the LSD you get AC=0. If you add 6 then if adding the 6 gives
>> you a carry out of Bit 3 you get AC set. Note that the carry might ripple so
>> that's NOT to say Bit 4 is necessarily 1.
>>
>> Here's part of Altair32's code:
>>
>> static void daa ( OP_ARG_U )
>> {
>> // Decimal Adjust Accumulator
>> // DAA:: A=BCD format
>> // Flags: SZAPC
>> // *
>>
>> register /* FJS */ word tmp = ACCUM;
>>
>> if ( (( tmp & 0x0f ) > 0x09 ) || ( FLAGS & AC_FLAG ))
>> tmp += 0x06;
>>
>> if (tmp > 0x0f)
>> FLAGS |= AC_FLAG; // if adjusted LSB > 0xf, set AC
>> else
>> FLAGS &= ~AC_FLAG; // else clear SC
>>
>>
>> So since tmp is not masked off, any value >0xF gets AC set even if no
>> carry or add occurred! In other words, pretend the value of ACCUM is 90 (and
>> thus temp is 90) with AC=0. The first if does not fire. The second if does
>> and AC gets set. That's got to be wrong. Granted, who checks AC after DAA?
>> But still
>>
>>
>> So why copy Bob? Well... I think SIMH has a similar but different problem.
>> Here's a snip from the DAA code:
>>
>> /*opcode=0x27*/
>> static void i86op_daa(PC_EN

Re: [Simh] DAA Emulation

[Timothe Litt]

I was not defending the simh code, and agree that it does not conform.  
 
Interestingly, a version of altair_cpu.c c.a. 2003 seems to be more correct,
so it appears that some "code cleanup" introduced this form of the bug at a
later date.  The old code still wasn't correct, as it didn't clear AC if the
add is NOT done...
 
Obviously this needs a regression test!
case 047: { /* DAA */

DAR = A & 0x0F;

if (DAR > 9 || AC > 0) {

DAR += 6;

A &= 0xF0;

A |= DAR & 0x0F;

if (DAR & 0x10)

AC = 020;

else

AC = 0;

}

DAR = (A >> 4) & 0x0F;

if (DAR > 9 || AC > 0) {

DAR += 6;

if (AC) DAR++;

A &= 0x0F;

A |= (DAR << 4);

}

if ((DAR << 4) & 0x100)

C = 020;

else

C = 0;

if (A & 0x80) {

S = 020;

} else {

S = 0;

}

if ((A & 0xff) == 0)

Z = 020;

else

Z = 0;

parity(A);

A = A & 0xFF;

break;

}


-
This communication may not represent my employer's views,
if any, on the matters discussed.
  

 

  _  

From: Al Williams [mailto:al.willi...@awce.com] 
Sent: Saturday, December 11, 2010 12:08
To: Timothe Litt
Cc: Richard Cini; simh@trailing-edge.com; Vince Briel
Subject: Re: [Simh] DAA Emulation


I agree with your interpretation and by that interpretation the code in both
emulators is currently busted. In the case of SIMH adding 6 does not always
result in a half carry.  

Granted if LSD is >9 it will cause AC to set. But in the case when AC is
already set this isn't always the case. Assuming the last add was BCD you
could have X9+Y9 = [X+Y+1]2.

So the LSD is 2 and AC is set. Adding 6 gives you 8 and no half carry. So
setting the flag every time you add 6 is incorrect.


On Sat, Dec 11, 2010 at 10:51 AM, Timothe Litt  wrote:


According to the Sep 75 Intel 8080 Microcomputer Systems manual:
 
P 4-3: Auxiliary Carry: If the instruction caused a carry out of bit 3 and
into bit 4 of the resulting value, the auxiliary carry is set; otherwise it
is reset.  This flag is affected by single precision additions,
subtractions, increments, decrements, comparisons and logical operations,
but is principally used with additions and increments preceding a DAA
(Decimal Adjust Accumulator) instruction.
 
P 4-8: DAA description has the text previously cited, plus
NOTE: All flags are affected
Cycles: 1
States: 4
Flags: Z,S,P,CY,AC
 
I read this as AC should be set if the first add 6 (to the LSB of A)
happened and caused a carry and should be cleared otherwise.  (Only this add
can cause a carry from bit 3 to bit 4.)

 



-
This communication may not represent my employer's views,
if any, on the matters discussed.
  

 

  _  

From: simh-boun...@trailing-edge.com [mailto:simh-boun...@trailing-edge.com]
On Behalf Of Al Williams
Sent: Saturday, December 11, 2010 10:30
To: Richard Cini
Cc: simh@trailing-edge.com; Vince Briel
Subject: Re: [Simh] DAA Emulation


Yeah I've read the docs, but even what you pasted in doesn't directly answer
the question: What is the state of the AC flag after a DAA? So I think the
code correctly implements the additions you mentioned. But I think we all 3
disagree on the AC flag code. Notice I didn't copy the whole function in
either case, so the handling of the MSD is "not shown". 




On Sat, Dec 11, 2010 at 8:16 AM, Richard Cini  wrote:


Al --

Thanks a lot for the email. I'm sure you looked at this, but I pulled
the Intel 8080 Users Manual and under DAA, it says the following (snipping a
bit):

The 8-bit number in the accumulator is adjusted to form two 4-bit [BCD]
digits by the following process:

(1) If the value of the least significant 4 bits is greater than 9
or if the AC flag is set, 6 is added to the accumulator.
(2) if the value of the most significant 4 bits is now greater than
9 or if the CY flag is set, 6 is added to the most significant 4 bits of the
accumulator.

Based on this, I would say that the second part of the test in the
Altair32 code is wrong. Further, it looks like the SIMH code may be wrong as
well because it doesn't test the CY flag in the second test.

As far as the register display, I'll make that change - oddly no one has
ever reported it.

Thanks again for locating this bug.

Rich

--
Rich Cini
Collector of Classic Computers
Build Master and lead engineer, Altair32 Emulator
http://www.altair32.com
http://www.classiccmp.org/cini 




On 12/11/10 1:32 AM, "Al Williams"  wrote:



Hi Rich and Bob,

I've been doing some work on Vince Briel's excellent AVR emulation of the
8080 and while rewriting DAA I came across what I think to be a harmless bug
but thought you might want to comment on it.

>From what I can glean DAA effects all flags including half carry. And I
_think_ that half carry occurs from the +6 (if it happens at all). So if you
don't adjust the LSD you get AC=0. If you add 6 then if adding the 6 gives
you a carry out of Bit 3 you get AC set. Note that the carry might ripp

Re: [Simh] DAA Emulation

[Peter Schorn]

Please note:

1. altair_cpu.c is for an 8080 CPU and contains the code listed below in
the most recent version of SIMH. No "code cleanup".

2. i86op_daa (from i86_ops.c) is for an 8086 CPU and I'd be interested
in any evidence that this is not correct 8086 CPU behavior resp. why
this should behave exactly the same as for an 8080.

Peter
---
peter.sch...@acm.org


On 11.12.10 23:11, Timothe Litt wrote:
> I was not defending the simh code, and agree that it does not conform. 
>  
> Interestingly, a version of altair_cpu.c c.a. 2003 seems to be more
> correct, so it appears that some "code cleanup" introduced this form of
> the bug at a later date.  The old code still wasn't correct, as it
> didn't clear AC if the add is NOT done...
>  
> Obviously this needs a regression test!
> 
> case 047: { /* DAA */
> 
> DAR = A & 0x0F;
> 
> if (DAR > 9 || AC > 0) {
> 
> DAR += 6;
> 
> A &= 0xF0;
> 
> A |= DAR & 0x0F;
> 
> if (DAR & 0x10)
> 
> AC = 020;
> 
> else
> 
> AC = 0;
> 
> }
> 
> DAR = (A >> 4) & 0x0F;
> 
> if (DAR > 9 || AC > 0) {
> 
> DAR += 6;
> 
> if (AC) DAR++;
> 
> A &= 0x0F;
> 
> A |= (DAR << 4);
> 
> }
> 
> if ((DAR << 4) & 0x100)
> 
> C = 020;
> 
> else
> 
> C = 0;
> 
> if (A & 0x80) {
> 
> S = 020;
> 
> } else {
> 
> S = 0;
> 
> }
> 
> if ((A & 0xff) == 0)
> 
> Z = 020;
> 
> else
> 
> Z = 0;
> 
> parity(A);
> 
> A = A & 0xFF;
> 
> break;
> 
> }
> 
> 
> -
> This communication may not represent my employer's views,
> if any, on the matters discussed.
>  
> 
>  
> 
> 
> *From:* Al Williams [mailto:al.willi...@awce.com]
> *Sent:* Saturday, December 11, 2010 12:08
> *To:* Timothe Litt
> *Cc:* Richard Cini; simh@trailing-edge.com; Vince Briel
> *Subject:* Re: [Simh] DAA Emulation
> 
> I agree with your interpretation and by that interpretation the code in
> both emulators is currently busted. In the case of SIMH adding 6 does
> not always result in a half carry. 
> 
> Granted if LSD is >9 it will cause AC to set. But in the case when AC is
> already set this isn't always the case. Assuming the last add was BCD
> you could have X9+Y9 = [X+Y+1]2.
> 
> So the LSD is 2 and AC is set. Adding 6 gives you 8 and no half carry.
> So setting the flag every time you add 6 is incorrect.
> 
> 
> On Sat, Dec 11, 2010 at 10:51 AM, Timothe Litt  > wrote:
> 
> According to the Sep 75 Intel 8080 Microcomputer Systems manual:
>  
> P 4-3: Auxiliary Carry: If the instruction caused a carry out of bit
> 3 and into bit 4 of the resulting value, the auxiliary carry is set;
> otherwise it is reset.  This flag is affected by single precision
> additions, subtractions, increments, decrements, comparisons and
> logical operations, but is principally used with additions and
> increments preceding a DAA (Decimal Adjust Accumulator) instruction.
>  
> P 4-8: DAA description has the text previously cited, plus
> NOTE: All flags are affected
> Cycles: 1
> States: 4
> Flags: Z,S,P,CY,AC
>  
> I read this as AC should be set if the first add 6 (to the LSB of A)
> happened and caused a carry and should be cleared otherwise.  (Only
> this add can cause a carry from bit 3 to bit 4.)
>  
> 
> -
> This communication may not represent my employer's views,
> if any, on the matters discussed.
>  
> 
>  
> 
> 
> *From:* simh-boun...@trailing-edge.com
> 
> [mailto:simh-boun...@trailing-edge.com
> ] *On Behalf Of *Al Williams
> *Sent:* Saturday, December 11, 2010 10:30
> *To:* Richard Cini
> *Cc:* simh@trailing-edge.com ; Vince
> Briel
> *Subject:* Re: [Simh] DAA Emulation
> 
> Yeah I've read the docs, but even what you pasted in doesn't
> directly answer the question: What is the state of the AC flag after
> a DAA? So I think the code correctly implements the additions you
> mentioned. But I think we all 3 disagree on the AC flag code. Notice
> I didn't copy the whole function in either case, so the handling of
> the MSD is "not shown".
> 
> 
> 
> 
> On Sat, Dec 11, 2010 at 8:16 AM, Richard Cini  > wrote:
> 
> Al --
> 
> Thanks a lot for the email. I’m sure you looked at this, but
> I pulled the Intel 8080 Users Manual and under DAA, it says the
> following (snipping a bit):
> 
> The 8-bit number in the accumulator is adjusted to form two
> 4-bit [BCD] digits by the following process:
> 
> (1) If the value of the least significant 4 bits is
> greater than 9 *or *if the

Re: [Simh] DAA Emulation

[Al Williams]

Maybe I'm looking in the wrong place. I just went and pulled the source off
the SIMH web page again. This is the daa code below. This DOES look correct
because DAR is set to just the LSD and then if DAR gets bit 4 set there must
have been a carry. Whoops -- so it is correct. What I had been looking at
was from http://www.schorn.ch/cpm/zip/altairz80source.zip which states on
the containing web site: The Altair 8800 simulator is part of the
SIMH
 family of simulators currently at version 3.8-1. Unlike a real Altair 8800
it features several enhancements. I didn't notice one of
the enhancements was the possibility of an 8086 CPU!
However -- and I'm guessing -- that code appears to do a Z80 DAA even if you
are simulating an 8080 (unless there is some other 8080 code I didn't find).
Probably no sane program would notice, of course.

However, as to the 8086 code...  its odd. The Intel docs (
http://download.intel.com/design/intarch/manuals/24319101.pdf page 3-143)
show in an algorithm that it the simulator logic is correct for x86. But the
text is a bit more fuzzy:

 If a decimal carry is detected, the CF and AF flags are set accordingly.

That doesn't match the logic in the associated pseudo code on the same page.
On the other hand, I have a slew of x86 processors handy so that could be
proven one way or the other.

So looks like I was incorrect about the SIMH code. The Z80 code looks ok
(although not good fidelity to the 8080). The 8080 code looks ok. And the
8086 code is probably ok based on that page's code (but maybe not based on
the text which is interpretable; but easy enough to find out if that's right
or wrong).

Another interesting point is the handling of the carry flag. The 8086 bleeds
the carry through if it is already set. That makes sense because you if you
added , say 90 + 90 you'd get (carry) + 20 going in. When you adjust you
would like to get (carry) + 80. The 8086 code does this (and the document
supports that). My code in the AVR does it too, but I can't find a reference
for if it really does behave that way or not. The SIMH 8080 code does NOT do
this (right or wrong) because it will clear it if the add did not generate a
carry to start with:
   if ((DAR << 4) & 0x100)
C = 020;
   else
C = 0;

If that is correct behavior, probably no sane program would notice my
incorrect behavior since it would know it had to "OR" the old carry with the
new carry to get a meaningful answer (after all, 50+50 = A0 = (cy) 00); so
you can't treat either carry as off as being significant). Does anyone have
a reference to that behavior or result from a real piece of silicon?

Sorry I got confused wrt to the 8086.





Code from schom.ch (z-80):
 case 0x27:  /* DAA */
tStates += 4;
sim_brk_pend[0] = FALSE;
acu = HIGH_REGISTER(AF);
temp = LOW_DIGIT(acu);
cbits = TSTFLAG(C);
if (TSTFLAG(N)) {   /* last operation was a subtract */
int hd = cbits || acu > 0x99;
if (TSTFLAG(H) || (temp > 9)) { /* adjust low digit */
if (temp > 5) {
SETFLAG(H, 0);
}
acu -= 6;
acu &= 0xff;
}
if (hd)
acu -= 0x160;   /* adjust high digit */
}
else {  /* last operation was an add */
if (TSTFLAG(H) || (temp > 9)) { /* adjust low digit */
SETFLAG(H, (temp > 9));
acu += 6;
}
if (cbits || ((acu & 0x1f0) > 0x90))
acu += 0x60;   /* adjust high digit */
}
AF = (AF & 0x12) | rrdrldTable[acu & 0xff] | ((acu >> 8) &
1) | cbits;
break;




SIMH code:

  case 047: { /* DAA */
DAR = A & 0x0F;
if (DAR > 9 || AC > 0) {
DAR += 6;
A &= 0xF0;
A |= DAR & 0x0F;
if (DAR & 0x10)
AC = 020;
   else
AC = 0;
}
DAR = (A >> 4) & 0x0F;
if (DAR > 9 || AC > 0) {
DAR += 6;
if (AC) DAR++;
A &= 0x0F;
A |= (DAR << 4);
}
if ((DAR << 4) & 0x100)
C = 020;
   else
C = 0;
if (A & 0x80) {
S = 020;
} else {
S = 0;
}
if ((A & 0xff) == 0)
Z = 020;
  else
Z = 0;
parity(A);
A = A & 0xFF;
break;
}

On Sat, Dec 11, 2010 at 

Re: [Simh] DAA Emulation

[Richard]

In article ,
"Timothe Litt"  writes:

> Obviously this needs a regression test!

As I've said on this list before, if the code had unit tests you would
know that you broke something as soon as you broke it.

Without unit tests, bugs like this one get introduced and lay dormant
for 7 years
-- 
"The Direct3D Graphics Pipeline" -- DirectX 9 draft available for download
 

  Legalize Adulthood! 
___
Simh mailing list
Simh@trailing-edge.com
http://mailman.trailing-edge.com/mailman/listinfo/simh