This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: Feature request: Set the direction flag on x86/x64 register->register operations
- From: Stefan Dösinger <stefan at codeweavers dot com>
- To: <binutils at sourceware dot org>
- Date: Tue, 9 Dec 2008 13:51:15 +0100
- Subject: RE: Feature request: Set the direction flag on x86/x64 register->register operations
Hi,
Here is another version of my patch to set the direction flag on
register->register operations.
The main change is that it is now controlled by a per-instruction suffix
instead of a command line switch. Alexandre Julliard didn't like the idea of
the command line switch, he prefers gcc to control the bit. I have to modify
gcc to make it generate a "movl %edi, %edi" like Windows has it anyway, so
going this route is ok for me.
I added a test for this feature which specifically tests the instructions
that are important to me(mov %edi, %edi and mov %esp, %ebp) and a few more.
I ran the existing tests with the flag forced on for all instructions, and
they pass. There are a few false positives obviously because the patch
changes the encoding, but I checked all of those cases and the code works
correctly.
I verified the behavior against msvc, and it does set the flag on 8, 16 and
32 bit operations for sure. I didn't check 64 bit ones because I do not have
a 64 bit Windows installation to test, but this is moot now anyway because
gcc controls the flag. It doesn't affect this patch any longer.
On Mon, Dec 1, 2008 at 1:42 PM, H.J. Lu <hjl dot tools at gmail dot com>
wrote:
> Your new patch still doesn't handle movdqu and other instructions,
> like instructions with more than 1 source operands.
movdqu doesn't have the D flag set in the opcode table. Still my patch now
doesn't care about the operand types and just looks at the D flag and the
possible operands for the template, so from my code's point of view it works
with movdqu now.
As for more than 1 source operand: It doesn't seem to me that the existing
code that deals with the direction flag can works with such instructions. In
the branch that sets the flag only 2 operands are handled.
Thanks again for your help,
Stefan
Index: gas/config/tc-i386.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-i386.c,v
retrieving revision 1.356
diff -u -r1.356 tc-i386.c
--- gas/config/tc-i386.c 8 Dec 2008 17:59:00 -0000 1.356
+++ gas/config/tc-i386.c 9 Dec 2008 12:24:38 -0000
@@ -82,6 +82,7 @@
#define QWORD_MNEM_SUFFIX 'q'
#define XMMWORD_MNEM_SUFFIX 'x'
#define YMMWORD_MNEM_SUFFIX 'y'
+#define REVERSE_OP_SUFFIX 'r'
/* Intel Syntax. Use a non-ascii letter since since it never appears
in instructions. */
#define LONG_DOUBLE_MNEM_SUFFIX '\1'
@@ -252,6 +253,10 @@
unsigned int flags[MAX_OPERANDS];
#define Operand_PCrel 1
+ /* Flags for the instruction */
+ unsigned int insn_flags;
+#define Insn_reverse 1
+
/* Relocation type for operand */
enum bfd_reloc_code_real reloc[MAX_OPERANDS];
@@ -2906,7 +2911,7 @@
char *l = line;
char *token_start = l;
char *mnem_p;
- int supported;
+ int supported, suffix_pos;
const template *t;
/* Non-zero if we found a prefix only acceptable with string insns. */
@@ -2991,45 +2996,60 @@
if (!current_templates)
{
/* See if we can get a match by trimming off a suffix. */
+ suffix_pos = -1;
switch (mnem_p[-1])
{
- case WORD_MNEM_SUFFIX:
- if (intel_syntax && (intel_float_operand (mnemonic) & 2))
- i.suffix = SHORT_MNEM_SUFFIX;
- else
- case BYTE_MNEM_SUFFIX:
- case QWORD_MNEM_SUFFIX:
- i.suffix = mnem_p[-1];
- mnem_p[-1] = '\0';
- current_templates = hash_find (op_hash, mnemonic);
- break;
- case SHORT_MNEM_SUFFIX:
- case LONG_MNEM_SUFFIX:
- if (!intel_syntax)
- {
- i.suffix = mnem_p[-1];
- mnem_p[-1] = '\0';
- current_templates = hash_find (op_hash, mnemonic);
- }
- break;
+ case WORD_MNEM_SUFFIX:
+ if (intel_syntax && (intel_float_operand (mnemonic) & 2))
+ i.suffix = SHORT_MNEM_SUFFIX;
+ else
+ case BYTE_MNEM_SUFFIX:
+ case QWORD_MNEM_SUFFIX:
+ i.suffix = mnem_p[-1];
+ mnem_p[-1] = '\0';
+ current_templates = hash_find (op_hash, mnemonic);
+ suffix_pos = -2;
+ break;
+ case SHORT_MNEM_SUFFIX:
+ case LONG_MNEM_SUFFIX:
+ if (!intel_syntax)
+ {
+ i.suffix = mnem_p[-1];
+ mnem_p[-1] = '\0';
+ current_templates = hash_find (op_hash, mnemonic);
+ }
+ suffix_pos = -2;
+ break;
/* Intel Syntax. */
- case 'd':
- if (intel_syntax)
- {
- if (intel_float_operand (mnemonic) == 1)
- i.suffix = SHORT_MNEM_SUFFIX;
- else
- i.suffix = LONG_MNEM_SUFFIX;
- mnem_p[-1] = '\0';
- current_templates = hash_find (op_hash, mnemonic);
- }
- break;
+ case 'd':
+ if (intel_syntax)
+ {
+ if (intel_float_operand (mnemonic) == 1)
+ i.suffix = SHORT_MNEM_SUFFIX;
+ else
+ i.suffix = LONG_MNEM_SUFFIX;
+ mnem_p[-1] = '\0';
+ current_templates = hash_find (op_hash, mnemonic);
+ }
+ suffix_pos = -2;
+ break;
}
if (!current_templates)
{
- as_bad (_("no such instruction: `%s'"), token_start);
- return NULL;
+ /* See if we find a reverse prefix */
+ if(mnem_p[suffix_pos] == REVERSE_OP_SUFFIX)
+ {
+ i.insn_flags |= Insn_reverse;
+ mnem_p[suffix_pos] = '\0';
+ }
+
+ current_templates = hash_find (op_hash, mnemonic);
+ if (!current_templates)
+ {
+ as_bad (_("no such instruction: `%s'"), token_start);
+ return NULL;
+ }
}
}
@@ -3543,6 +3563,7 @@
unsigned int j;
unsigned int found_cpu_match;
unsigned int check_register;
+ unsigned int op0 = 0, op1 = 1;
#if MAX_OPERANDS != 5
# error "MAX_OPERANDS must be 5."
@@ -3712,16 +3733,32 @@
&& operand_type_equal (&i.types [0], &acc32)
&& operand_type_equal (&i.types [1], &acc32))
continue;
+
+ if ((i.insn_flags & Insn_reverse)
+ && t->opcode_modifier.d)
+ {
+ found_reverse_match |= Opcode_D;
+ overlap0 = operand_type_and (i.types[1], operand_types[0]);
+ op0 = 1;
+ op1 = 0;
+ }
+ else
+ {
+ found_reverse_match &= ~Opcode_D;
+ op0 = 0;
+ op1 = 1;
+ }
+
case 3:
case 4:
case 5:
- overlap1 = operand_type_and (i.types[1], operand_types[1]);
- if (!operand_type_match (overlap0, i.types[0])
- || !operand_type_match (overlap1, i.types[1])
+ overlap1 = operand_type_and (i.types[op1], operand_types[1]);
+ if (!operand_type_match (overlap0, i.types[op0])
+ || !operand_type_match (overlap1, i.types[op1])
|| (check_register
- && !operand_type_register_match (overlap0, i.types[0],
+ && !operand_type_register_match (overlap0, i.types[op0],
operand_types[0],
- overlap1, i.types[1],
+ overlap1, i.types[op1],
operand_types[1])))
{
/* Check if other direction is valid ... */
@@ -3729,16 +3766,16 @@
continue;
/* Try reversing direction of operands. */
- overlap0 = operand_type_and (i.types[0], operand_types[1]);
- overlap1 = operand_type_and (i.types[1], operand_types[0]);
- if (!operand_type_match (overlap0, i.types[0])
- || !operand_type_match (overlap1, i.types[1])
+ overlap0 = operand_type_and (i.types[op0], operand_types[1]);
+ overlap1 = operand_type_and (i.types[op1], operand_types[0]);
+ if (!operand_type_match (overlap0, i.types[op0])
+ || !operand_type_match (overlap1, i.types[op1])
|| (check_register
&& !operand_type_register_match (overlap0,
- i.types[0],
+ i.types[op0],
operand_types[1],
overlap1,
- i.types[1],
+ i.types[op1],
operand_types[0])))
{
/* Does not match either direction. */
@@ -3747,7 +3784,7 @@
/* found_reverse_match holds which of D or FloatDR
we've found. */
if (t->opcode_modifier.d)
- found_reverse_match = Opcode_D;
+ found_reverse_match ^= Opcode_D;
else if (t->opcode_modifier.floatd)
found_reverse_match = Opcode_FloatD;
else
@@ -3801,7 +3838,7 @@
if (!operand_type_match (overlap2, i.types[2])
|| (check_register
&& !operand_type_register_match (overlap1,
- i.types[1],
+ i.types[op1],
operand_types[1],
overlap2,
i.types[2],
@@ -3855,6 +3892,11 @@
affect assembly of the next line of code. */
as_warn (_("stand-alone `%s' prefix"), t->name);
}
+ if (!t->opcode_modifier.d
+ && i.insn_flags & Insn_reverse)
+ {
+ as_warn (_("%s does not support the direction reversion flag"), t->name);
+ }
}
/* Copy the template we found. */
Index: gas/testsuite/gas/i386/i386.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/i386/i386.exp,v
retrieving revision 1.106
diff -u -r1.106 i386.exp
--- gas/testsuite/gas/i386/i386.exp 12 Oct 2008 12:37:09 -0000 1.106
+++ gas/testsuite/gas/i386/i386.exp 9 Dec 2008 12:24:38 -0000
@@ -140,6 +140,7 @@
run_dump_test "arch-avx-1"
run_list_test "arch-avx-1-1" "-march=generic32+avx -I${srcdir}/$subdir -al"
run_list_test "arch-avx-1-2" "-march=generic32+aes -I${srcdir}/$subdir -al"
+ run_dump_test "reverse"
# These tests require support for 8 and 16 bit relocs,
# so we only run them for ELF and COFF targets.
--- /dev/null 2008-12-09 11:08:33.653321976 +0100
+++ gas/testsuite/gas/i386/reverse.s 2008-12-09 01:49:12.000000000 +0100
@@ -0,0 +1,17 @@
+.text
+f:
+ movrl %esp, %ebp
+ movl %esp, %ebp
+ movrl %edi, %edi
+ movl %edi, %edi
+ xorrl %eax, %eax
+ xorl %eax, %eax
+ movrl %ebx,%eax
+ movl %ebx,%eax
+ xorrw %bx, %ax
+ xorw %bx, %ax
+ andrb %bh,%ah
+ andb %bh,%ah
+ addrb %bl,%al
+ addb %bl,%al
+ movdqur %xmm4,%xmm6
--- /dev/null 2008-12-09 11:08:33.653321976 +0100
+++ gas/testsuite/gas/i386/reverse.d 2008-12-09 01:51:24.000000000 +0100
@@ -0,0 +1,23 @@
+#objdump: -dr
+#name: i386 reverse
+
+.*: +file format .*
+
+Disassembly of section .text:
+
+0+000 <f>:
+ 0: 8b ec [ ]*mov %esp,%ebp
+ 2: 89 e5 [ ]*mov %esp,%ebp
+ 4: 8b ff [ ]*mov %edi,%edi
+ 6: 89 ff [ ]*mov %edi,%edi
+ 8: 33 c0 [ ]*xor %eax,%eax
+ a: 31 c0 [ ]*xor %eax,%eax
+ c: 8b c3 [ ]*mov %ebx,%eax
+ e: 89 d8 [ ]*mov %ebx,%eax
+ 10: 66 33 c3 [ ]*xor %bx,%ax
+ 13: 66 31 d8 [ ]*xor %bx,%ax
+ 16: 22 e7 [ ]*and %bh,%ah
+ 18: 20 fc [ ]*and %bh,%ah
+ 1a: 02 c3 [ ]*add %bl,%al
+ 1c: 00 d8 [ ]*add %bl,%al
+ 1e: f3 0f 6f f4 [ ]*movdqu %xmm4,%xmm6