#1406 ucsim: ADD/ADDC Command: PSW is not set correct!

closed-invalid
Simulator (19)
5
2013-05-25
2007-12-03
No

Hi All,

I'm pretty sure that there exists a bug concerning the PSW value after an ADD instruction. I assume that this bug exists in the ADDC instruction as well.

Example:

While executing the 63th instruction of the attached *.hex file the PSW is not set correct:

The instruction is:
0x0a51 24 fa -> which is a: ADD A,#data instruction

situation:
A: 0x09
#data: 0xFA
PSW: 0x00

now the Instruction 0x24 = ADD A,#data
is executed:

in binary:
#data: 11111010
A 00001001
___________________+
res: 100000011

Now the ucSim sets the PSW to 0xC0:
0xC0 = 11000000 => CY=1, AC=1, OV=0

but it should be:
0x84 = 10000100 => CY=1, AC=0, OV=1

to set the PSW after an ACC instruction IMHO it should be done according to following code fragment:

tmp3 = current Accumulator Value.

/** Set Carry Flags * */
/** CY: Check if there is a carry-out at bit 7 */
if (((tmp3 & 0x100 == 0x100)){
mcu.psw.bitSet(C51PSW.FLAG_CY);
} else {
mcu.psw.bitClear(C51PSW.FLAG_CY);
}

/** AC: Check if there is a carry-out at bit 7 */
if (((tmp3 & 0x10) == 0x10)){
mcu.psw.bitSet(C51PSW.FLAG_AC);
} else {
mcu.psw.bitClear(C51PSW.FLAG_AC);
}

/** Check if Overflow Bit should be set */
if (((tmp3 & 0x80) == 0x80) && ((tmp3 & 0x100) != 0x100)) {
mcu.psw.bitSet(C51PSW.FLAG_OV);
}
else if(((tmp3 & 0x100) == 0x100) && ((tmp3 & 0x80) != 0x80)){
mcu.psw.bitSet(C51PSW.FLAG_OV);
}
else {
mcu.psw.bitClear(C51PSW.FLAG_OV);
}

According to INTEL 8051 user manual and 8052.com flag setting after an ADD/ADDC instruction should be done in this way:

The Carry bit (C) is set if there is a carry-out of bit 7. In other words, if the unsigned summed value of the Accumulator, operand and (in the case of ADDC) the Carry flag exceeds 255 Carry is set. Otherwise, the Carry bit is cleared.

The Auxillary Carry (AC) bit is set if there is a carry-out of bit 3. In other words, if the unsigned summed value of the low nibble of the Accumulator, operand and (in the case of ADDC) the Carry flag exceeds 15 the Auxillary Carry flag is set. Otherwise, the Auxillary Carry flag is cleared.

The Overflow (OV) bit is set if there is a carry-out of bit 6 or out of bit 7, but not both. In other words, if the addition of the Accumulator, operand and (in the case of ADDC) the Carry flag treated as signed values results in a value that is out of the range of a signed byte (-128 through +127) the Overflow flag is set. Otherwise, the Overflow flag is cleared.

lg from Vienna,
Thomas

Discussion

  • threinbacher

    threinbacher - 2007-12-03

    the used *.hex file for reproducing the bug

     
  • threinbacher

    threinbacher - 2007-12-03

    Logged In: YES
    user_id=1949775
    Originator: YES

    Some more infos:

    Im using the ucsim S51 that is delivered with sdcc version:
    sdcc-snapshot-i386-unknown-linux2.5-20071202-4974
    My kernel Version is:
    2.6.22.5-31-default | opensuse 10.3

    lg
    Thomas

     
  • threinbacher

    threinbacher - 2007-12-03

    Logged In: YES
    user_id=1949775
    Originator: YES

    Hi all,

    Just now i simulated the exact Program Code with the Keil C51 Simulator.

    And Keil behaves exactly like ucsim in this case, but Why??

    Can somebody give me a hint how the 8051 sets its flag after an ADD instruction??

    In my opinion the datasheet of ther 8051 specifies this in exaclty the other way....

    lg from Vienna,
    Thomas

     
  • Daniel Drotos

    Daniel Drotos - 2007-12-04
    • assigned_to: nobody --> drdani
     
  • Daniel Drotos

    Daniel Drotos - 2007-12-04

    Logged In: YES
    user_id=7015
    Originator: NO

    I've just checked your example on a real HW, and it results C0 in PSW.

     
  • Daniel Drotos

    Daniel Drotos - 2007-12-04
    • milestone: --> non_bugs
    • status: open --> closed-invalid
     
  • Nobody/Anonymous

    Logged In: NO

    Allow me to throw in my $0.000001010001111011 and explain the flaw in your argument:

    I assume 'tmp3' is the result of your addition of (A + #data) and can hold 9 bits (seeing as how you're comparing it to 0x100).

    >> /** Set Carry Flags * */
    >> /** CY: Check if there is a carry-out at bit 7 */
    >> if (((tmp3 & 0x100 == 0x100)){
    >> mcu.psw.bitSet(C51PSW.FLAG_CY);
    >> } else {
    >> mcu.psw.bitClear(C51PSW.FLAG_CY);
    >> }

    This logic is sound.

    >> /** AC: Check if there is a carry-out at bit 7 */
    >> if (((tmp3 & 0x10) == 0x10)){
    >> mcu.psw.bitSet(C51PSW.FLAG_AC);
    >> } else {
    >> mcu.psw.bitClear(C51PSW.FLAG_AC);
    >> }

    You're saying "if bit 4 is set in the output, there was a half-carry". This is NOT correct. These are the eight possibilities (let A4 be A bit 4, D4 be #data bit 4, C3 be carry-out from A3+D3, R4 be the result's bit 4)

    A4=0, D4=0, C3=0: R4 = 0
    A4=0, D4=0, C3=1: R4 = 1
    A4=0, D4=1, C3=0: R4 = 0
    A4=0, D4=1, C3=1: R4 = 0 (C4 = 1)
    A4=1, D4=0, C3=0: R4 = 1
    A4=1, D4=0, C3=1: R4 = 0 (C4 = 1)
    A4=1, D4=1, C3=0: R4 = 0 (C4 = 1)
    A4=1, D4=1, C3=1: R4 = 1 (C4 = 1)

    As you can see here, in only half of the cases does C3 happen to be equal to R4. The real way to test for a carry is to check if R4 != (A4 xor D4), which is the same as (R4 xor A4 xor D4).

    >> /** Check if Overflow Bit should be set */
    >> if (((tmp3 & 0x80) == 0x80) && ((tmp3 & 0x100) != 0x100)) {
    >> mcu.psw.bitSet(C51PSW.FLAG_OV);
    >> }
    >> else if(((tmp3 & 0x100) == 0x100) && ((tmp3 & 0x80) != 0x80)){
    >> mcu.psw.bitSet(C51PSW.FLAG_OV);
    >> }
    >> else {
    >> mcu.psw.bitClear(C51PSW.FLAG_OV);
    >> }

    This suffers the same problem as your AC calculation: R7 does not necessarily equal C6. Instead, to test for carry from bit 6 you need to check (R7 xor A7 xor D7).

    The reason why your "bit 8 test" works for the CY flag is because R8 == (A8 + D8 + C7). Since your registers are 8-bits long, A8 and D8 are always zero, and therefore R8 == C7.

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks