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 <ubizjak at gmail dot com>]

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.