[RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Uros Bizjak]

Hello!

Attached patch:

a) warns for 64-bit values in GR asm operand on 32bit targets

It is impossible to pass 64-bit (long long) value in 32bit register.
We should warn for this situation, since only 32bit lowpart value is
passed. The warning can be omitted by explicitly casting asm operand
to int.

b) errors out for 8-bit values in invalid GR asm operand

We can detect invalid %sil, %dil and %bpl asm operands early, and
terminate compilation in the compiler, not later in the assembler.

2017-02-28  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/i386.c (print_reg): Warn for values of 64-bit size
    in integer register on 32-bit targets.  Error out for values of
    8-bit size in invalid integer register.

testsuite/ChangeLog:

2017-02-28  Uros Bizjak  <ubizjak@gmail.com>

    * gcc.target/i386/invsize-1.c: New test.
    * gcc.target/i386/invsize-2.c: Ditto.

OK for mainline in stage 4?

Uros.

--cut here--
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 14ac189..e64bbe7 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -17641,6 +17641,9 @@ print_reg (rtx x, int code, FILE *file)
   switch (msize)
     {
     case 8:
+      if (LEGACY_INT_REGNO_P (regno) && !TARGET_64BIT)
+       warning (0, "unsupported size for integer register in 32-bit mode");
+      /* FALLTHRU */
     case 4:
       if (LEGACY_INT_REGNO_P (regno))
        putc (msize == 8 && TARGET_64BIT ? 'r' : 'e', file);
@@ -17654,6 +17657,8 @@ print_reg (rtx x, int code, FILE *file)
     case 1:
       if (regno >= ARRAY_SIZE (qi_reg_name))
        goto normal;
+      if (!ANY_QI_REGNO_P (regno))
+       error ("unsupported size for integer register in 32-bit mode");
       reg = qi_reg_name[regno];
       break;
     case 0:
--cut here--

/* { dg-do compile { target ia32 } } */

void foo (long long x)
{
  __asm__ volatile ("# %0" : : "r" (x));   /* { dg-warning
"unsupported size" }  */
}

/* { dg-do compile { target ia32 } } */

void foo (char x)
{
  register char rx  __asm ("si") = x;

  __asm__ volatile ("# %0" : : "r" (rx));   /* { dg-error "unsupported
size" }  */
}

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Richard Biener]

On Tue, 28 Feb 2017, Uros Bizjak wrote:

> Hello!
> 
> Attached patch:
> 
> a) warns for 64-bit values in GR asm operand on 32bit targets
> 
> It is impossible to pass 64-bit (long long) value in 32bit register.
> We should warn for this situation, since only 32bit lowpart value is
> passed. The warning can be omitted by explicitly casting asm operand
> to int.
> 
> b) errors out for 8-bit values in invalid GR asm operand
> 
> We can detect invalid %sil, %dil and %bpl asm operands early, and
> terminate compilation in the compiler, not later in the assembler.
> 
> 2017-02-28  Uros Bizjak  <ubizjak@gmail.com>
> 
>     * config/i386/i386.c (print_reg): Warn for values of 64-bit size
>     in integer register on 32-bit targets.  Error out for values of
>     8-bit size in invalid integer register.
> 
> testsuite/ChangeLog:
> 
> 2017-02-28  Uros Bizjak  <ubizjak@gmail.com>
> 
>     * gcc.target/i386/invsize-1.c: New test.
>     * gcc.target/i386/invsize-2.c: Ditto.
> 
> OK for mainline in stage 4?

Yes.

Richard.

> Uros.
> 
> --cut here--
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 14ac189..e64bbe7 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -17641,6 +17641,9 @@ print_reg (rtx x, int code, FILE *file)
>    switch (msize)
>      {
>      case 8:
> +      if (LEGACY_INT_REGNO_P (regno) && !TARGET_64BIT)
> +       warning (0, "unsupported size for integer register in 32-bit mode");
> +      /* FALLTHRU */
>      case 4:
>        if (LEGACY_INT_REGNO_P (regno))
>         putc (msize == 8 && TARGET_64BIT ? 'r' : 'e', file);
> @@ -17654,6 +17657,8 @@ print_reg (rtx x, int code, FILE *file)
>      case 1:
>        if (regno >= ARRAY_SIZE (qi_reg_name))
>         goto normal;
> +      if (!ANY_QI_REGNO_P (regno))
> +       error ("unsupported size for integer register in 32-bit mode");
>        reg = qi_reg_name[regno];
>        break;
>      case 0:
> --cut here--
> 
> /* { dg-do compile { target ia32 } } */
> 
> void foo (long long x)
> {
>   __asm__ volatile ("# %0" : : "r" (x));   /* { dg-warning
> "unsupported size" }  */
> }
> 
> /* { dg-do compile { target ia32 } } */
> 
> void foo (char x)
> {
>   register char rx  __asm ("si") = x;
> 
>   __asm__ volatile ("# %0" : : "r" (rx));   /* { dg-error "unsupported
> size" }  */
> }
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Jakub Jelinek]

On Tue, Feb 28, 2017 at 11:41:56AM +0100, Richard Biener wrote:
> > 2017-02-28  Uros Bizjak  <ubizjak@gmail.com>
> > 
> >     * config/i386/i386.c (print_reg): Warn for values of 64-bit size
> >     in integer register on 32-bit targets.  Error out for values of
> >     8-bit size in invalid integer register.
> > 
> > testsuite/ChangeLog:
> > 
> > 2017-02-28  Uros Bizjak  <ubizjak@gmail.com>
> > 
> >     * gcc.target/i386/invsize-1.c: New test.
> >     * gcc.target/i386/invsize-2.c: Ditto.
> > 
> > OK for mainline in stage 4?
> 
> Yes.

Have you tried to build say Linux kernel or firefox or similar large
codebase with lots of inline asm with that?
What constraint should people use for long long vars in 32-bit code?
"A" constraint is used a lot in 32-bit code (say for inline asm with
rdtsc), but what if you need more than one long long input?

	Jakub

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Uros Bizjak]

On Tue, Feb 28, 2017 at 12:06 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Feb 28, 2017 at 11:41:56AM +0100, Richard Biener wrote:
>> > 2017-02-28  Uros Bizjak  <ubizjak@gmail.com>
>> >
>> >     * config/i386/i386.c (print_reg): Warn for values of 64-bit size
>> >     in integer register on 32-bit targets.  Error out for values of
>> >     8-bit size in invalid integer register.
>> >
>> > testsuite/ChangeLog:
>> >
>> > 2017-02-28  Uros Bizjak  <ubizjak@gmail.com>
>> >
>> >     * gcc.target/i386/invsize-1.c: New test.
>> >     * gcc.target/i386/invsize-2.c: Ditto.
>> >
>> > OK for mainline in stage 4?
>>
>> Yes.
>
> Have you tried to build say Linux kernel or firefox or similar large
> codebase with lots of inline asm with that?

No...

> What constraint should people use for long long vars in 32-bit code?
> "A" constraint is used a lot in 32-bit code (say for inline asm with
> rdtsc), but what if you need more than one long long input?

Hm, we don't guarantee DImode register pairs other that "A"
constraint. But you are right, we have to allow "A" for 64bit eax/edx
pair.

Please consider the 64bit warning part dropped.

Uros.

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Jakub Jelinek]

On Tue, Feb 28, 2017 at 04:34:20PM +0100, Uros Bizjak wrote:
> > Have you tried to build say Linux kernel or firefox or similar large
> > codebase with lots of inline asm with that?
> 
> No...
> 
> > What constraint should people use for long long vars in 32-bit code?
> > "A" constraint is used a lot in 32-bit code (say for inline asm with
> > rdtsc), but what if you need more than one long long input?
> 
> Hm, we don't guarantee DImode register pairs other that "A"
> constraint. But you are right, we have to allow "A" for 64bit eax/edx
> pair.
> 
> Please consider the 64bit warning part dropped.

The 8-bit error is fine, unless the operand isn't used, it would hopefully
fail to assemble anyway.

	Jakub

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Uros Bizjak]

On Tue, Feb 28, 2017 at 4:34 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Tue, Feb 28, 2017 at 12:06 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Tue, Feb 28, 2017 at 11:41:56AM +0100, Richard Biener wrote:
>>> > 2017-02-28  Uros Bizjak  <ubizjak@gmail.com>
>>> >
>>> >     * config/i386/i386.c (print_reg): Warn for values of 64-bit size
>>> >     in integer register on 32-bit targets.  Error out for values of
>>> >     8-bit size in invalid integer register.
>>> >
>>> > testsuite/ChangeLog:
>>> >
>>> > 2017-02-28  Uros Bizjak  <ubizjak@gmail.com>
>>> >
>>> >     * gcc.target/i386/invsize-1.c: New test.
>>> >     * gcc.target/i386/invsize-2.c: Ditto.
>>> >
>>> > OK for mainline in stage 4?
>>>
>>> Yes.
>>
>> Have you tried to build say Linux kernel or firefox or similar large
>> codebase with lots of inline asm with that?
>
> No...
>
>> What constraint should people use for long long vars in 32-bit code?
>> "A" constraint is used a lot in 32-bit code (say for inline asm with
>> rdtsc), but what if you need more than one long long input?
>
> Hm, we don't guarantee DImode register pairs other that "A"
> constraint. But you are right, we have to allow "A" for 64bit eax/edx
> pair.

Some more thoughts on 64-bit reg on 32-bit targets warning.

Actually, we never *print* register name for instruction that use "A"
constraint, since %eax/%edx is always implicit  The warning does not
deal with constraints, so unless we want to output DImode register
name, there is no warning.

Runing the complete testsuite on a patched compiler, following
testcases trigger wartning on 32-bit targets:

FAIL: gcc.target/i386/pr66274.c (test for excess errors)
FAIL: gcc.target/i386/stackalign/asm-1.c -mstackrealign (test for excess errors)
FAIL: gcc.target/i386/stackalign/asm-1.c -mno-stackrealign (test for
excess errors)

Some analysis:

pr66274:

void f()
{
  asm ("push %0" : : "r" ((unsigned long long) 456));
}

compiles to:

f:
        movl    $456, %eax
        movl    $0, %edx
#APP
# 6 "pr66274.c" 1
        push %eax
# 0 "" 2
#NO_APP

The warning is correct, we didn't push the whole longlong value.

stackaling/asm-1.c:

void f(){asm("%0"::"r"(1.5F));}void g(){asm("%0"::"r"(1.5));}

resulting in:

g:
        ...
        fldl    .LC1
        fstpl   -16(%ebp)
        movl    -16(%ebp), %eax
        movl    -12(%ebp), %edx
#APP
# 7 "asm-1.c" 1
        %eax
# 0 "" 2
#NO_APP

We want to handle DFmode value (1.5) in the asm on 32bit target. Since
the value is 64bit, and we use 32bit register, the warning is correct
and beneficial in this case.

I will compile linux kernel with the patched compiler, but looking at
the above two cases, I'd say that warning is indeed helpful.

Uros.

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Jakub Jelinek]

On Wed, Mar 01, 2017 at 09:34:53AM +0100, Uros Bizjak wrote:
> Some more thoughts on 64-bit reg on 32-bit targets warning.
> 
> Actually, we never *print* register name for instruction that use "A"
> constraint, since %eax/%edx is always implicit  The warning does not
> deal with constraints, so unless we want to output DImode register
> name, there is no warning.

Ah, indeed, we don't have a modifier that would print the high register
of a register pair (i.e. essentially print REGNO (x) + 1 instead of REGNO
(x)), guess that might be useful not just for 64-bit GPR operands in 32-bit
code, but also 128-bit GPR operands in 64-bit code.

While looking at ix86_print_operand, I've noticed duplication in the
comment:
   w -- print the operand as if it's a "word" (HImode) even if it isn't.
   s -- print a shift double count, followed by the assemblers argument
        delimiter.
   b -- print the QImode name of the register for the indicated operand.
        %b0 would print %al if operands[0] is reg 0.
   w --  likewise, print the HImode name of the register.
   k --  likewise, print the SImode name of the register.
   q --  likewise, print the DImode name of the register.
   x --  likewise, print the V4SFmode name of the register.
   t --  likewise, print the V8SFmode name of the register.
   g --  likewise, print the V16SFmode name of the register.

w is documented twice, guess the first line should be removed.

	Jakub

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Uros Bizjak]

On Wed, Mar 1, 2017 at 9:48 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Mar 01, 2017 at 09:34:53AM +0100, Uros Bizjak wrote:
>> Some more thoughts on 64-bit reg on 32-bit targets warning.
>>
>> Actually, we never *print* register name for instruction that use "A"
>> constraint, since %eax/%edx is always implicit  The warning does not
>> deal with constraints, so unless we want to output DImode register
>> name, there is no warning.
>
> Ah, indeed, we don't have a modifier that would print the high register
> of a register pair (i.e. essentially print REGNO (x) + 1 instead of REGNO
> (x)), guess that might be useful not just for 64-bit GPR operands in 32-bit
> code, but also 128-bit GPR operands in 64-bit code.

The issue here is that (modulo ax/dx with "A" constraint) we don't
guarantee double-register sequence order, so any change in register
allocation order would break any assumptions. For implicit ax/dx, user
should explicitly use register name (e.g. DImode operand in "rdtscp;
mov %0, mem" asm should be corrected to use %%eax instead of %0).

And, yes - we should add similar warning for 128-bit GPRs. The only
way to use register pair with  width > machine_mode is with implicit
operands or with explicit regnames.

> While looking at ix86_print_operand, I've noticed duplication in the
> comment:
>    w -- print the operand as if it's a "word" (HImode) even if it isn't.
>    s -- print a shift double count, followed by the assemblers argument
>         delimiter.
>    b -- print the QImode name of the register for the indicated operand.
>         %b0 would print %al if operands[0] is reg 0.
>    w --  likewise, print the HImode name of the register.
>    k --  likewise, print the SImode name of the register.
>    q --  likewise, print the DImode name of the register.
>    x --  likewise, print the V4SFmode name of the register.
>    t --  likewise, print the V8SFmode name of the register.
>    g --  likewise, print the V16SFmode name of the register.
>
> w is documented twice, guess the first line should be removed.

Indeed. The first one should be removed.

Uros.

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Uros Bizjak]

On Wed, Mar 1, 2017 at 10:00 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Mar 1, 2017 at 9:48 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Wed, Mar 01, 2017 at 09:34:53AM +0100, Uros Bizjak wrote:
>>> Some more thoughts on 64-bit reg on 32-bit targets warning.
>>>
>>> Actually, we never *print* register name for instruction that use "A"
>>> constraint, since %eax/%edx is always implicit  The warning does not
>>> deal with constraints, so unless we want to output DImode register
>>> name, there is no warning.
>>
>> Ah, indeed, we don't have a modifier that would print the high register
>> of a register pair (i.e. essentially print REGNO (x) + 1 instead of REGNO
>> (x)), guess that might be useful not just for 64-bit GPR operands in 32-bit
>> code, but also 128-bit GPR operands in 64-bit code.
>
> The issue here is that (modulo ax/dx with "A" constraint) we don't
> guarantee double-register sequence order, so any change in register
> allocation order would break any assumptions. For implicit ax/dx, user
> should explicitly use register name (e.g. DImode operand in "rdtscp;
> mov %0, mem" asm should be corrected to use %%eax instead of %0).
>
> And, yes - we should add similar warning for 128-bit GPRs. The only
> way to use register pair with  width > machine_mode is with implicit
> operands or with explicit regnames.

Something like the following patch I'm testing:

--cut here--
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 2b11aa1..943b2a0 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -17646,13 +17646,16 @@ print_reg (rtx x, int code, FILE *file)

   switch (msize)
     {
+    case 16:
+    case 12:
     case 8:
+      if (GENERAL_REGNO_P (regno) && msize > GET_MODE_SIZE (word_mode))
+       warning (0, "unsupported size for integer register");
+      /* FALLTHRU */
     case 4:
       if (LEGACY_INT_REGNO_P (regno))
        putc (msize == 8 && TARGET_64BIT ? 'r' : 'e', file);
       /* FALLTHRU */
-    case 16:
-    case 12:
     case 2:
     normal:
       reg = hi_reg_name[regno];
--cut here--

Uros.

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 2155 bytes --]

On Wed, Mar 1, 2017 at 11:41 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Wed, Mar 1, 2017 at 10:00 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Wed, Mar 1, 2017 at 9:48 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Wed, Mar 01, 2017 at 09:34:53AM +0100, Uros Bizjak wrote:
>>>> Some more thoughts on 64-bit reg on 32-bit targets warning.
>>>>
>>>> Actually, we never *print* register name for instruction that use "A"
>>>> constraint, since %eax/%edx is always implicit  The warning does not
>>>> deal with constraints, so unless we want to output DImode register
>>>> name, there is no warning.
>>>
>>> Ah, indeed, we don't have a modifier that would print the high register
>>> of a register pair (i.e. essentially print REGNO (x) + 1 instead of REGNO
>>> (x)), guess that might be useful not just for 64-bit GPR operands in 32-bit
>>> code, but also 128-bit GPR operands in 64-bit code.
>>
>> The issue here is that (modulo ax/dx with "A" constraint) we don't
>> guarantee double-register sequence order, so any change in register
>> allocation order would break any assumptions. For implicit ax/dx, user
>> should explicitly use register name (e.g. DImode operand in "rdtscp;
>> mov %0, mem" asm should be corrected to use %%eax instead of %0).
>>
>> And, yes - we should add similar warning for 128-bit GPRs. The only
>> way to use register pair with  width > machine_mode is with implicit
>> operands or with explicit regnames.
>
> Something like the following patch I'm testing:

Attached is the patch I have committed to mainline SVN after a full
bootstrap and regression test.

2017-03-01  Uros Bizjak  <ubizjak@gmail.com>

    * config/i386/i386.c (print_reg): Warn for values of
    unsupported size in integer register.

testsuite/ChangeLog:

2017-03-01  Uros Bizjak  <ubizjak@gmail.com>

    * gcc.target/i386/invsize-2.c: New test.
    * gcc.target/i386/invsize-3.c: Ditto.
    * gcc.target/i386/invsize-4.c: Ditto.
    * gcc.target/i386/pr66274.c: Expect "unsuported size" warning.
    * gcc.target/i386/stackalign/asm-1.c: Ditto.

Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

Committed to mainline.

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 3052 bytes --]

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 245811)
+++ config/i386/i386.c	(working copy)
@@ -17646,13 +17646,16 @@ print_reg (rtx x, int code, FILE *file)
 
   switch (msize)
     {
+    case 16:
+    case 12:
     case 8:
+      if (GENERAL_REGNO_P (regno) && msize > GET_MODE_SIZE (word_mode))
+	warning (0, "unsupported size for integer register");
+      /* FALLTHRU */
     case 4:
       if (LEGACY_INT_REGNO_P (regno))
-	putc (msize == 8 && TARGET_64BIT ? 'r' : 'e', file);
+	putc (msize > 4 && TARGET_64BIT ? 'r' : 'e', file);
       /* FALLTHRU */
-    case 16:
-    case 12:
     case 2:
     normal:
       reg = hi_reg_name[regno];
Index: testsuite/gcc.target/i386/invsize-2.c
===================================================================
--- testsuite/gcc.target/i386/invsize-2.c	(nonexistent)
+++ testsuite/gcc.target/i386/invsize-2.c	(working copy)
@@ -0,0 +1,7 @@
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "" } */
+
+void foo (long long x)
+{
+  __asm__ volatile ("# %0" : : "r" (x));
+} /* { dg-warning "unsupported size" }  */
Index: testsuite/gcc.target/i386/invsize-3.c
===================================================================
--- testsuite/gcc.target/i386/invsize-3.c	(nonexistent)
+++ testsuite/gcc.target/i386/invsize-3.c	(working copy)
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+void foo (long double x)
+{
+  __asm__ volatile ("# %0" : : "r" (x));
+} /* { dg-warning "unsupported size" }  */
Index: testsuite/gcc.target/i386/invsize-4.c
===================================================================
--- testsuite/gcc.target/i386/invsize-4.c	(nonexistent)
+++ testsuite/gcc.target/i386/invsize-4.c	(working copy)
@@ -0,0 +1,7 @@
+/* { dg-do compile { target int128 } } */
+/* { dg-options "" } */
+
+void foo (__int128 x)
+{
+  __asm__ volatile ("# %0" : : "r" (x));
+} /* { dg-warning "unsupported size" }  */
Index: testsuite/gcc.target/i386/pr66274.c
===================================================================
--- testsuite/gcc.target/i386/pr66274.c	(revision 245811)
+++ testsuite/gcc.target/i386/pr66274.c	(working copy)
@@ -3,7 +3,7 @@
 
 void f()
 {
-  asm ("push %0" : : "r" ((unsigned long long) 456));
-}
+  asm ("push %0" : : "r" ((unsigned long long) 456 >> 32));
+} /* { dg-warning "unsupported size" }  */
 
-/* { dg-final { scan-assembler-not "push %r" } } */
+/* { dg-final { scan-assembler-not "push\[ \t]+%r" } } */
Index: testsuite/gcc.target/i386/stackalign/asm-1.c
===================================================================
--- testsuite/gcc.target/i386/stackalign/asm-1.c	(revision 245811)
+++ testsuite/gcc.target/i386/stackalign/asm-1.c	(working copy)
@@ -4,4 +4,4 @@
 
 /* This case is to detect a compile time regression introduced in stack
    branch development. */
-void f(){asm("%0"::"r"(1.5F));}void g(){asm("%0"::"r"(1.5));}
+void f(){asm("%0"::"r"(1.5F));}void g(){asm("%0"::"r"(1.5));} /* { dg-warning "unsupported size" }  */

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Jakub Jelinek]

Hi!

On Wed, Mar 01, 2017 at 08:26:36PM +0100, Uros Bizjak wrote:
> 2017-03-01  Uros Bizjak  <ubizjak@gmail.com>
> 
>     * config/i386/i386.c (print_reg): Warn for values of
>     unsupported size in integer register.
> 
> testsuite/ChangeLog:
> 
> 2017-03-01  Uros Bizjak  <ubizjak@gmail.com>
> 
>     * gcc.target/i386/invsize-2.c: New test.
>     * gcc.target/i386/invsize-3.c: Ditto.
>     * gcc.target/i386/invsize-4.c: Ditto.
>     * gcc.target/i386/pr66274.c: Expect "unsuported size" warning.
>     * gcc.target/i386/stackalign/asm-1.c: Ditto.
> 
> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.

This broke
FAIL: gcc.dg/pr57134.c (test for excess errors)
on i686-linux, the warning triggers there.

It seems "# %0 %1" actually isn't needed for reproduction of the original
bug (tried powerpc64-linux cross at r200000 where it ICEs even with ""
and succeeds at r204000), so is this ok for trunk?

Other option is to add { dg-final { dg-warning "..." { target ia32 } } }
but that really doesn't scale to other targets, or -w.

2017-03-02  Jakub Jelinek  <jakub@redhat.com>

	* gcc.dg/pr57134.c: Use empty inline asm string literal.

--- gcc/testsuite/gcc.dg/pr57134.c.jj	2013-06-19 19:28:33.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr57134.c	2017-03-02 23:24:40.329317713 +0100
@@ -18,7 +18,7 @@ static __inline__ __attribute__((always_
 atomic64_read(const atomic64_t *v)
 {
  int64_t t;
- __asm__ __volatile__("# %0, %1" : "=r"(t) : "m"(v->counter));
+ __asm__ __volatile__("" : "=r"(t) : "m"(v->counter));
  return t;
 }
 


	Jakub

Re: [RFA PATCH, i386]: Warn for 64-bit values in general-reg asm operands and error out for 8-bit values in invalid GR asm operand

[Uros Bizjak]

On Thu, Mar 2, 2017 at 11:31 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On Wed, Mar 01, 2017 at 08:26:36PM +0100, Uros Bizjak wrote:
>> 2017-03-01  Uros Bizjak  <ubizjak@gmail.com>
>>
>>     * config/i386/i386.c (print_reg): Warn for values of
>>     unsupported size in integer register.
>>
>> testsuite/ChangeLog:
>>
>> 2017-03-01  Uros Bizjak  <ubizjak@gmail.com>
>>
>>     * gcc.target/i386/invsize-2.c: New test.
>>     * gcc.target/i386/invsize-3.c: Ditto.
>>     * gcc.target/i386/invsize-4.c: Ditto.
>>     * gcc.target/i386/pr66274.c: Expect "unsuported size" warning.
>>     * gcc.target/i386/stackalign/asm-1.c: Ditto.
>>
>> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
>
> This broke
> FAIL: gcc.dg/pr57134.c (test for excess errors)
> on i686-linux, the warning triggers there.
>
> It seems "# %0 %1" actually isn't needed for reproduction of the original
> bug (tried powerpc64-linux cross at r200000 where it ICEs even with ""
> and succeeds at r204000), so is this ok for trunk?
>
> Other option is to add { dg-final { dg-warning "..." { target ia32 } } }
> but that really doesn't scale to other targets, or -w.
>
> 2017-03-02  Jakub Jelinek  <jakub@redhat.com>
>
>         * gcc.dg/pr57134.c: Use empty inline asm string literal.

Looks obvious to me, and just the case we want to catch on x86_32.

Uros.

> --- gcc/testsuite/gcc.dg/pr57134.c.jj   2013-06-19 19:28:33.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr57134.c      2017-03-02 23:24:40.329317713 +0100
> @@ -18,7 +18,7 @@ static __inline__ __attribute__((always_
>  atomic64_read(const atomic64_t *v)
>  {
>   int64_t t;
> - __asm__ __volatile__("# %0, %1" : "=r"(t) : "m"(v->counter));
> + __asm__ __volatile__("" : "=r"(t) : "m"(v->counter));
>   return t;
>  }
>
>
>
>         Jakub