Bug 56114 - x86_64-linux-gnu-gcc generate wrong asm instruction MOVABS for intel syntax
Summary: x86_64-linux-gnu-gcc generate wrong asm instruction MOVABS for intel syntax
Status: VERIFIED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.7.2
: P3 normal
Target Milestone: 4.6.4
Assignee: Uroš Bizjak
URL: http://gcc.gnu.org/ml/gcc-patches/201...
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-25 22:47 UTC by Alexander Kobets
Modified: 2013-01-27 20:48 UTC (History)
0 users

See Also:
Host:
Target: x86_64
Build:
Known to work:
Known to fail:
Last reconfirmed: 2013-01-26 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Kobets 2013-01-25 22:47:41 UTC
For example file l.c

long foo2 (void)
{
  return *(volatile long*)0xFEE00000;
}

x86_64-linux-gnu-gcc -c -save-temps l.c -O2 -masm=intel
cat l.s
	.file	"l.c"
	.intel_syntax noprefix
	.text
	.p2align 4,,15
	.globl	foo2
	.type	foo2, @function
foo2:
.LFB0:
	.cfi_startproc
	movabs	rax, 4276092928
	ret

This is erroneous instruction, because movabs rax, 4276092928 loads immediate data. This code must be
movabs	rax, [4276092928]
with square brackets.

When we try to read 32-bit data from memory, then we get error message
long foo2 (void)
{
  return *(volatile int*)0xFEE00000;
}
x86_64-linux-gnu-gcc -c -save-temps l.c -O2 -masm=intel
cat l.s
	.file	"l.c"
	.intel_syntax noprefix
	.text
	.p2align 4,,15
	.globl	foo2
	.type	foo2, @function
foo2:
.LFB0:
	.cfi_startproc
	movabs	eax, 4276092928
	cdqe
	ret

Because movabs allows load 64-bit only immediate data. Here is GCC loose square brackets.
Comment 1 Alexander Kobets 2013-01-25 23:06:39 UTC
For *(volatile int*)0xFEE00000

x86_64-linux-gnu-gcc -c -save-temps l.c -O2 -masm=intel
l.s: Assembler messages:
l.s:10: Error: operand type mismatch for `movabs'

Alse tried
gcc version 4.7.3 20130119 (prerelease) (GCC)
with same result.
Comment 2 Uroš Bizjak 2013-01-26 09:21:30 UTC
Proposed patch:

--cut here--
Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md (revision 195486)
+++ config/i386/i386.md (working copy)
@@ -2308,7 +2308,7 @@
        (match_operand:SWI1248x 1 "nonmemory_operand" "a,r<i>"))]
   "TARGET_LP64 && ix86_check_movabs (insn, 0)"
   "@
-   movabs{<imodesuffix>}\t{%1, %P0|%P0, %1}
+   movabs{<imodesuffix>}\t{%1, %P0|%A0, %1}
    mov{<imodesuffix>}\t{%1, %a0|%a0, %1}"
   [(set_attr "type" "imov")
    (set_attr "modrm" "0,*")
@@ -2322,7 +2322,7 @@
         (mem:SWI1248x (match_operand:DI 1 "x86_64_movabs_operand" "i,r")))]
   "TARGET_LP64 && ix86_check_movabs (insn, 1)"
   "@
-   movabs{<imodesuffix>}\t{%P1, %0|%0, %P1}
+   movabs{<imodesuffix>}\t{%P1, %0|%0, %A1}
    mov{<imodesuffix>}\t{%a1, %0|%0, %a1}"
   [(set_attr "type" "imov")
    (set_attr "modrm" "0,*")
--cut here--
Comment 3 Alexander Kobets 2013-01-26 14:30:42 UTC
(In reply to comment #2)
> Proposed patch:
It is correcting this code, but break another. For example (-mcmodel=large):
--cut here--
long* p1;

long foo2 (void)
{
	p1 = (long*)0x123;
	return 5;
}
--cut here--
$ x86_64-linux-gnu-gcc -c -mcmodel=large -save-temps l.c -O1 -masm=intel
l.s: Assembler messages:
l.s:10: Error: operand type mismatch for `movabs'

$ cat l.s
	.file	"l.c"
	.intel_syntax noprefix
	.text
	.globl	foo2
	.type	foo2, @function
foo2:
.LFB0:
	.cfi_startproc
	mov	eax, 291
	movabs	[OFFSET FLAT:p1], rax
	mov	ax, 5
	ret
	.cfi_endproc
.LFE0:
	.size	foo2, .-foo2
	.comm	p1,8,8
	.ident	"GCC: (GNU) 4.7.2"
	.section	.note.GNU-stack,"",@progbits

The instruction movabs [OFFSET FLAT:p1], rax
before patch was without square brackets and normal compiled.
Comment 4 Uroš Bizjak 2013-01-26 15:08:35 UTC
Well, following patch won't break then:

--cut here--
Index: i386.md
===================================================================
--- i386.md     (revision 195486)
+++ i386.md     (working copy)
@@ -2308,7 +2308,7 @@
        (match_operand:SWI1248x 1 "nonmemory_operand" "a,r<i>"))]
   "TARGET_LP64 && ix86_check_movabs (insn, 0)"
   "@
-   movabs{<imodesuffix>}\t{%1, %P0|%P0, %1}
+   movabs{<imodesuffix>}\t{%1, %P0|[%P0], %1}
    mov{<imodesuffix>}\t{%1, %a0|%a0, %1}"
   [(set_attr "type" "imov")
    (set_attr "modrm" "0,*")
@@ -2322,7 +2322,7 @@
         (mem:SWI1248x (match_operand:DI 1 "x86_64_movabs_operand" "i,r")))]
   "TARGET_LP64 && ix86_check_movabs (insn, 1)"
   "@
-   movabs{<imodesuffix>}\t{%P1, %0|%0, %P1}
+   movabs{<imodesuffix>}\t{%P1, %0|%0, [%P1]}
    mov{<imodesuffix>}\t{%a1, %0|%0, %a1}"
   [(set_attr "type" "imov")
    (set_attr "modrm" "0,*")
--cut here--
Comment 5 Alexander Kobets 2013-01-26 16:54:50 UTC
(In reply to comment #4)
> Well, following patch won't break then:
Yes, thank you.
Comment 6 uros 2013-01-27 13:17:00 UTC
Author: uros
Date: Sun Jan 27 13:16:54 2013
New Revision: 195494

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195494
Log:
	PR target/56114
	* config/i386/i386.md (*movabs<mode>_1): Add square brackets around
	operand 0 in movabs insn template for -masm=intel asm alternative.
	(*movabs<mode>_2): Ditto for operand 1.

testsuite/ChangeLog:

	PR target/56114
	* gcc.target/i386/pr56114.c: New test.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr56114.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.md
    trunk/gcc/testsuite/ChangeLog
Comment 7 uros 2013-01-27 16:03:55 UTC
Author: uros
Date: Sun Jan 27 16:03:40 2013
New Revision: 195496

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195496
Log:
	Backport from mainline
	2013-01-27  Uros Bizjak  <ubizjak@gmail.com>

	PR target/56114
	* config/i386/i386.md (*movabs<mode>_1): Add square brackets around
	operand 0 in movabs insn template for -masm=intel asm alternative.
	(*movabs<mode>_2): Ditto for operand 1.


Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/config/i386/i386.md
Comment 8 uros 2013-01-27 18:37:28 UTC
Author: uros
Date: Sun Jan 27 18:37:23 2013
New Revision: 195497

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195497
Log:
	Backport from mainline
	2013-01-27  Uros Bizjak  <ubizjak@gmail.com>

	PR target/56114
	* config/i386/i386.md (*movabs<mode>_1): Add square brackets around
	operand 0 in movabs insn template for -masm=intel asm alternative.
	(*movabs<mode>_2): Ditto for operand 1.


Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/config/i386/i386.md
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
Comment 9 Uroš Bizjak 2013-01-27 18:46:05 UTC
Fixed everywhere.
Comment 10 Alexander Kobets 2013-01-27 20:48:52 UTC
Good job.