P2 Instructions TESTB & TESTBN

Cluso99Cluso99 Posts: 14,254
edited December 6 in Propeller 2 Vote Up0Vote Down
The instructions TESTB & TESTBN seem to me to give the wrong results for the Z flags
                mov     lmm_x,#0
                testb   lmm_x,#0                wz      ' nz  <<<<<
                testbn  lmm_x,#0                wz      ' z   <<<<<
                testb   lmm_x,#0                wc      ' nc
                testbn  lmm_x,#0                wc      ' c
                and     lmm_x,#1                wz      ' z

                mov     lmm_x,#1
                testb   lmm_x,#0                wz      ' z   <<<<<
                testbn  lmm_x,#0                wz      ' nz  <<<<<
                testb   lmm_x,#0                wc      ' c
                testbn  lmm_x,#0                wc      ' nc
                and     lmm_x,#1                wz      ' nz
Am I missing something or is it a bug?
My Prop boards: P8XBlade2, RamBlade, CpuBlade, TriBlade
Prop OS (also see Sphinx, PropDos, PropCmd, Spinix)
Website: www.clusos.com
Prop Tools (Index) , Emulators (Index) , ZiCog (Z80)
«134

Comments

  • 120 Comments sorted by Date Added Votes
  • This was changed quite a few FPGA versions ago.
    Change wasn't mentioned though, broke a lot of code.
    Found out the hard way.
    IIRC caught garryj out too.
    Melbourne, Australia
  • I seem to recall mentioning it before. Just got caught again :(
    And it seems wrong to me - more like a bug unless someone can recall there being a specific reason to make it appear backwards.
    My Prop boards: P8XBlade2, RamBlade, CpuBlade, TriBlade
    Prop OS (also see Sphinx, PropDos, PropCmd, Spinix)
    Website: www.clusos.com
    Prop Tools (Index) , Emulators (Index) , ZiCog (Z80)
  • cgraceycgracey Posts: 10,549
    edited December 6 Vote Up0Vote Down
    Cluso99 wrote: »
    I seem to recall mentioning it before. Just got caught again :(
    And it seems wrong to me - more like a bug unless someone can recall there being a specific reason to make it appear backwards.

    I went back and forth on this and decided to make it so the bit goes into Z without being inverted.

    What prompted the change was that there are now flag operators like ANDZ which can quickly confuse things. I figured it was easier to have everyone picture moving bits into/from Z or C, rather than the usual Z semantics, which make things way more complex, in light things like XORZ, DRVZ, etc.
  • Cluso99Cluso99 Posts: 14,254
    edited December 6 Vote Up0Vote Down
    Z has always been confusing until you wrap your head around the fact that the Z flag is set if the result/etc is zero.

    However, IMHO the changes now make it even more confusing rather than simplifying, because it is no longer consistent. Sometimes Z is set to mean zero, and other times it is cleared to mean zero. This is evident as shown in the above example where AND produces the expected result whereas TESTB produces the reverse.

    BTW I hadn't realised there are other instructions that implement it reversed too :(
    My Prop boards: P8XBlade2, RamBlade, CpuBlade, TriBlade
    Prop OS (also see Sphinx, PropDos, PropCmd, Spinix)
    Website: www.clusos.com
    Prop Tools (Index) , Emulators (Index) , ZiCog (Z80)
  • Wrt the Z bit in TESTB and TESTBN, the instructions may be viewed as operating as a subtraction from those bit positions, thus in the first example of subtracting 1 from 0 results in non-zero. TESTBN works in a similar fashion.
  • Cluso99 wrote: »
    Z has always been confusing until you wrap your head around the fact that the Z flag is set if the result/etc is zero.

    However, IMHO the changes now make it even more confusing rather than simplifying, because it is no longer consistent. Sometimes Z is set to mean zero, and other times it is cleared to mean zero. This is evident as shown in the above example where AND produces the expected result whereas TESTB produces the reverse.

    BTW I hadn't realised there are other instructions that implement it reversed too :(

    For 'TEST D,S WZ, Z=1 when result=0. This is the case for all register-level operations.

    For the bit tests TESTB/TESTP, the bit/pin goes into or interacts with C/Z. For TESTBN/TESTPN the NOT bit goes into or interacts with C/Z.

    How would you like Z handled in these different cases?
    TESTP   {#}D           WC/WZ
    TESTPN  {#}D           WC/WZ
    TESTP   {#}D       ANDC/ANDZ
    TESTPN  {#}D       ANDC/ANDZ
    TESTP   {#}D         ORC/ORZ
    TESTPN  {#}D         ORC/ORZ
    TESTP   {#}D       XORC/XORZ
    TESTPN  {#}D       XORC/XORZ
    
  • Yeah, I got caught out too at the initial change. But in the context of discrete pins and bits, it made sense to have the C and Z bits directly reflect pin/bit state or !state.
    garryj
  • cgracey wrote: »
    Cluso99 wrote: »
    Z has always been confusing until you wrap your head around the fact that the Z flag is set if the result/etc is zero.

    However, IMHO the changes now make it even more confusing rather than simplifying, because it is no longer consistent. Sometimes Z is set to mean zero, and other times it is cleared to mean zero. This is evident as shown in the above example where AND produces the expected result whereas TESTB produces the reverse.

    BTW I hadn't realised there are other instructions that implement it reversed too :(

    For 'TEST D,S WZ, Z=1 when result=0. This is the case for all register-level operations.

    For the bit tests TESTB/TESTP, the bit/pin goes into or interacts with C/Z. For TESTBN/TESTPN the NOT bit goes into or interacts with C/Z.

    How would you like Z handled in these different cases?
    TESTP   {#}D           WC/WZ
    TESTPN  {#}D           WC/WZ
    TESTP   {#}D       ANDC/ANDZ
    TESTPN  {#}D       ANDC/ANDZ
    TESTP   {#}D         ORC/ORZ
    TESTPN  {#}D         ORC/ORZ
    TESTP   {#}D       XORC/XORZ
    TESTPN  {#}D       XORC/XORZ
    

    As TESTBx/TESTPx can write to either C or Z the two flags must be treated the same. Chip made the right decision.
    Formerly known as TonyB
  • I prefer the wau it currently is.
    To avoid any confusion you can always use these conditionals in Pnut.
    if_00
    if_01
    if_10
    if_11
    if_x0
    if_x1
    if_0x
    if_1x
    
    Eezy peezy :)
    Melbourne, Australia
  • To me, there is no difference between all the other instructions where Z is set if the result is "0". For those caught out, you proved my point - it's opposite to the rest of the tests.

    To me, TESTP and TESTB families are just the same. They are not a move of the pin/bit, they are a test of that pin/bit and Z is set if the result is "0" in the same way as the TEST instruction which is really an AND with NR.

    C in these instances is identical to other instructions that set C for "1" (excluding parity results).

    IIRC MOVCZ may be in this category too.

    FWIW I got caught when setting Z and C and then doing an IF_C_NE_Z instruction modifier.

    So, IMHO it is an unnecessary quirk and it has to go in the tricks and traps list.

    Just my 2c.
    My Prop boards: P8XBlade2, RamBlade, CpuBlade, TriBlade
    Prop OS (also see Sphinx, PropDos, PropCmd, Spinix)
    Website: www.clusos.com
    Prop Tools (Index) , Emulators (Index) , ZiCog (Z80)
  • Cluso99 wrote: »
    For those caught out, you proved my point...
    No.
    I was caught out because it was changed (discovered when existing code broke), not because I was confused.

    Melbourne, Australia
  • ozpropdev wrote: »
    Cluso99 wrote: »
    For those caught out, you proved my point...
    No.
    I was caught out because it was changed (discovered when existing code broke), not because I was confused.

    What ozpropdev said :smile:
    garryj
  • Cluso99Cluso99 Posts: 14,254
    edited December 7 Vote Up0Vote Down
    I still fail to see why it is reversed to the other instructions.
    Why would you say the bit instructions are a shift into Z rather than a test and set Z if 0?

    Convince me please as I cannot see why it is opposite to all the other instructions that set Z.

    I got caught out because it operated in reverse to the other instructions which set Z. To me, it's just plain and simply wrong :(
    My Prop boards: P8XBlade2, RamBlade, CpuBlade, TriBlade
    Prop OS (also see Sphinx, PropDos, PropCmd, Spinix)
    Website: www.clusos.com
    Prop Tools (Index) , Emulators (Index) , ZiCog (Z80)
  • Cluso99 wrote: »
    I still fail to see why it is reversed to the other instructions.
    Why would you say the bit instructions are a shift into Z rather than a test and set Z if 0?

    Convince me please as I cannot see why it is opposite to all the other instructions that set Z.

    I got caught out because it operated in reverse to the other instructions which set Z. To me, it's just plain and simply wrong :(

    For bit instructions, C and Z are just two bitvariables with that names. You can read them from registerbits or portbits and do logical operation with it. It's much harder to do that if one of the bitvariables (Z) has inverted logic level.

    C also has many other meanings than Carry, depending on the instruction, why is this a problem for Z ?
  • Cluso99Cluso99 Posts: 14,254
    edited December 8 Vote Up0Vote Down
    Ariba wrote: »
    Cluso99 wrote: »
    I still fail to see why it is reversed to the other instructions.
    Why would you say the bit instructions are a shift into Z rather than a test and set Z if 0?

    Convince me please as I cannot see why it is opposite to all the other instructions that set Z.

    I got caught out because it operated in reverse to the other instructions which set Z. To me, it's just plain and simply wrong :(

    For bit instructions, C and Z are just two bitvariables with that names. You can read them from registerbits or portbits and do logical operation with it. It's much harder to do that if one of the bitvariables (Z) has inverted logic level.

    C also has many other meanings than Carry, depending on the instruction, why is this a problem for Z ?

    By that assumption, then all the other instructions that set Z are wrong too :(

    C is set if the bit=1
    Z is set if the bit=0 (ie it IS zero). That is what the Z flag means.

    Currently, you test the bit, and if it is zero, then Z is cleared (ie not zero). Next you follow with a conditional instruction based on the flag setting..
    testb reg,#0 wz ' Z=SET=1, NZ=0
    if_nz xxxx
    Now that will be executed if NZ (ie if the bit was zero)
    My Prop boards: P8XBlade2, RamBlade, CpuBlade, TriBlade
    Prop OS (also see Sphinx, PropDos, PropCmd, Spinix)
    Website: www.clusos.com
    Prop Tools (Index) , Emulators (Index) , ZiCog (Z80)
  • I've always viewed "if_z" as a if-Z-flag-is-set rather than was the last compare a zero. So I'm good with it the new way.

    Money is a placeholder for cooperation
  • Same.
    Do not taunt Happy Fun Ball! @opengeekorg ---> Be Excellent To One Another SKYPE = acuity_doug
    Parallax colors simplified: https://forums.parallax.com/discussion/123709/commented-graphics-demo-spin<br>
  • Z means not always Zero, just as C means not always Carry.
  • testb reg,#0 wz
    if_x1 xxxx	'bit = 1, do xxxx
    if_x0 yyyy	'bit = 0 ,do yyyy
    
    Simple :)
    Melbourne, Australia
  • test reg,#0 w_x1
    
    There, fixed it. :P
    Money is a placeholder for cooperation
  • Cluso99Cluso99 Posts: 14,254
    edited December 8 Vote Up0Vote Down
    Still doesn't fix the fact that it is reversed to other instructions and will be just another trap and irritation to many.
    At this time, it's so simple to fix.
    My Prop boards: P8XBlade2, RamBlade, CpuBlade, TriBlade
    Prop OS (also see Sphinx, PropDos, PropCmd, Spinix)
    Website: www.clusos.com
    Prop Tools (Index) , Emulators (Index) , ZiCog (Z80)
  • Cluso99 wrote: »
    Still doesn't fix the fact that it is reversed to other instructions and will be just another trap and irritation to many.
    At this time, it's so simple to fix.

    I found the use of C and Z on the P1 to be counterintuitive at times.

    If you think of them as Condition Flags CF1 and CF2, then the operation being same for bitwise operations makes sense.

    Then for multi-bit operations: if the result overflows, set CF1, otherwise clear it; and if all bits are clear, set CF2, otherwise clear it.

    It just happens that the flags are named C and Z instead of CF1 and CF2.
  • Anything that's counterintuitive needs to be fixed. It's as simple as that.

    -Phil
    “Perfection is achieved not when there is nothing more to add, but when there is nothing left to take away. -Antoine de Saint-Exupery
  • Anything that's counterintuitive needs to be fixed. It's as simple as that.

    -Phil

    The problem is that counterintuitive for one person may be intuitive for another. I believe that's what we are seeing here.
  • Cluso99Cluso99 Posts: 14,254
    edited December 9 Vote Up0Vote Down
    The results are opposite to the majority of the other instructions, so it's definitely counterintuitive.
    I have had to place a special comment in my code to show that Z is set opposite to what is expected.
    My Prop boards: P8XBlade2, RamBlade, CpuBlade, TriBlade
    Prop OS (also see Sphinx, PropDos, PropCmd, Spinix)
    Website: www.clusos.com
    Prop Tools (Index) , Emulators (Index) , ZiCog (Z80)
  • The C and Z flags should be set based on the result of the operation occuring, regardless of what the operation is.

    So, I think I must agree with Cluso here.
  • Both C and Z are just a bit store in those operations, like with rotate instructions.

    Money is a placeholder for cooperation
  • I don't care about the implementation of the operation.

    C and Z flags should always be set based on the result of the operation, unless the operation (or part of it) is to explicitly set or clear the C/Z flag(s).

    Either you need to rename the TESTB and TESTBN instructions to indicated that they explicitly manipulate the C/Z flags differently than expected, or they should set the flags according to the result of the operation.

    Doing something else, just because you like it, makes something inconsistent (not just within this chip, but with ALL other chips across MPUs and CPUs).
  • cgraceycgracey Posts: 10,549
    edited December 9 Vote Up0Vote Down
    How would you like Z handled in these different cases?
    TESTB   {#}D         WZ
    TESTBN  {#}D         WZ
    TESTB   {#}D       ANDZ
    TESTBN  {#}D       ANDZ
    TESTB   {#}D        ORZ
    TESTBN  {#}D        ORZ
    TESTB   {#}D       XORZ
    TESTBN  {#}D       XORZ
    

    I don't see how these can be changed without multiplying the confusion. That none of you guys complaining have bothered to address this tells me that you don't have any solution, either.

    The problem here is that we've transcended traditional Z-flag simplicity and there's no going back.
  • evanhevanh Posts: 5,913
    edited December 9 Vote Up0Vote Down
    Shrug. How about BOOL/BOOLN .... Short for boolean operation.


    EDIT: I note there is already another TEST/TESTN instruction that has different operation. Changing the name might be the best answer alright.

    Money is a placeholder for cooperation
Sign In or Register to comment.