VOGONS


XADD Flags?

Topic actions

First post, by danoon

User metadata
Rank Member
Rank
Member

I noticed that XADD on the normal core does not set flags. The docs I've read said it should said flags like ADD. I don't have a use case where this causes any problems. I was just tracking down a separate bug when I noticed this and was wondering if it was intentional.

Normal Core:

CASE_0F_D(0xc1) /* XADD Gd,Ed */
435 {
436 if (CPU_ArchitectureType<CPU_ARCHTYPE_486OLDSLOW) goto illegal_opcode;
437 GetRMrd;Bit32u oldrmrd=*rmrd;
438 if (rm >= 0xc0 ) {GetEArd;*rmrd=*eard;*eard+=oldrmrd;}
439 else {GetEAa;*rmrd=LoadMd(eaa);SaveMd(eaa,LoadMd(eaa)+oldrmrd);}
440 break;

Reply 2 of 11, by cfoesch

User metadata
Rank Newbie
Rank
Newbie

In agreement, according to the P3 Instruction Set Reference from Intel, the XADD does set CF, PF, AF, SF, ZF, and OF flags according to the result of the addition.

Reply 5 of 11, by ripsaw8080

User metadata
Rank DOSBox Author
Rank
DOSBox Author

The approach in the attached patch is to start with the same code used for ADD and also write the old destination value to the source operand. This way ADDB(), ADDW(), and ADDD() handle the flags exactly the same as ADD.

In my research the operands of opcode (0F C0 nn) are decoded like (00 nn), and (0F C1 nn) like (01 nn), so it seems the comments currently on the XADD instructions have the operand types reversed.

Note that the DOSBox debugger incorrectly disassembles byte XADD as having word operands. I posted a suggested fix for that issue in another thread.

EDIT: included dword instruction in patch.

Attachments

  • Filename
    xadd_flags.diff
    File size
    1.93 KiB
    Downloads
    349 downloads
    File license
    Fair use/fair dealing exception

Reply 6 of 11, by ripsaw8080

User metadata
Rank DOSBox Author
Rank
DOSBox Author

Oops, forgot the dword instruction; added it to the patch.

I thought there might be examples of XADD use where programmers challenge themselves to write the smallest possible code, such as 32, 64, 128, and 256-byte demos; and there are indeed a few, but haven't found any that care about flags.

Reply 7 of 11, by ripsaw8080

User metadata
Rank DOSBox Author
Rank
DOSBox Author

DOSBox is only changing ZF for the CMPXCHG instruction, and it's true that some docs only mention that flag, but others (example, example) state that all comparison flags are affected. I'm not an expert on the subject, but it would make sense to have consistent behavior for comparisons.

The attached patch implements flag handling for CMPXCHG based on CMP (which includes ZF, so no special handling of it is needed).

Attachments

  • Filename
    cmpxchg_flags.diff
    File size
    2.37 KiB
    Downloads
    290 downloads
    File license
    Fair use/fair dealing exception

Reply 10 of 11, by ripsaw8080

User metadata
Rank DOSBox Author
Rank
DOSBox Author

It seems XADD and CMPXCHG are rarely used, which I suppose is not unexpected for such CISC-ish 486 instructions. Any broken behavior resulting from unhandled flags could be quite subtle, so I've put log messages on the instructions in case I stumble across examples of their use in the process of testing. I think handling the flags, like fixing BSWAP, is mostly for the sake of correctness at this point, and in no way a priority.