PATCH [1/n] addr32: Properly use Pmode and word_mode

On Sun, Mar 4, 2012 at 2:40 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Sun, Mar 4, 2012 at 11:01 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>>> @@ -13637,7 +13665,8 @@ ix86_print_operand (FILE *file, rtx x, int code)
>>>              gcc_unreachable ();
>>>            }
>>>
>>> -         ix86_print_operand (file, x, 0);
>>> +         ix86_print_operand (file, x,
>>> +                             TARGET_64BIT && REG_P (x) ? 'q' : 0);
>>>          return;
>>>
>>> This is too big hammer. You output everything in DImode, so even if
>>> the address is in fact in SImode, you output it in DImode with an
>>> addr32 prefix.
>>>
>>
>> "%A" is only used in "jmp\t%A0" and there is no "jmp *%eax" instruction in
>> 64bit mode, only "jmp *%rax":
>>
>> [hjl@gnu-4 tmp]$ cat j.s
>>        jmp *%eax
>>        jmp *%rax
>> [hjl@gnu-4 tmp]$ gcc -c j.s
>> j.s: Assembler messages:
>> j.s:1: Error: operand type mismatch for `jmp'
>> [hjl@gnu-4 tmp]$
>>
>> It is OK for x32 since the upper 32bits are zero when we are loading "%eax".
>
> Just zero_extend register in wrong mode to DImode in indirect_jump and
> tablejump expanders. If above is true, then gcc will remove this
> extension automatically.
>

I tried:


and compiler does generate the same output. i386.c also has

	xasm = "jmp\t%A0";
    xasm = "call\t%A0";

for calls.  There are no separate indirect call patterns.  For x32,
only indirect register calls have to be in DImode.  The direct call
should be in Pmode (SImode).

Since x86-64 hardware always zero-extends upper 32bits of 64bit
registers when loading its lower 32bits, it is safe and easier to just
to output 64bit registers for %A than zero-extend it by hand for all
jump/call patterns.

On Mon, Mar 5, 2012 at 4:53 AM, H.J. Lu <hjl.tools@gmail.com> wrote:

> and compiler does generate the same output. i386.c also has
>
>        xasm = "jmp\t%A0";
>    xasm = "call\t%A0";
>
> for calls.  There are no separate indirect call patterns.  For x32,
> only indirect register calls have to be in DImode.  The direct call
> should be in Pmode (SImode).

Direct call just expects label to some abolute address that is assumed
to fit in 32 bits (see constant_call_address_operand_p).

call and jmp insn expect word_mode operands, so please change
ix86_expand_call and call patterns in the same way as jump
instructions above.

> Since x86-64 hardware always zero-extends upper 32bits of 64bit
> registers when loading its lower 32bits, it is safe and easier to just
> to output 64bit registers for %A than zero-extend it by hand for all
> jump/call patterns.

No, the instruction expects word_mode operands, so we have to extend
values to expected mode. I don't think that patching at insn output
time is acceptable.

BTW: I propose to split the patch into smaller pieces, dealing with
various independent parts separately. Handling jump/call insn is
definitely one of them, the other is stringops handling, another
prologue/epilogue expansion.

Uros.

On Sun, Mar 4, 2012 at 11:47 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Mar 5, 2012 at 4:53 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>> and compiler does generate the same output. i386.c also has
>>
>>        xasm = "jmp\t%A0";
>>    xasm = "call\t%A0";
>>
>> for calls.  There are no separate indirect call patterns.  For x32,
>> only indirect register calls have to be in DImode.  The direct call
>> should be in Pmode (SImode).
>
> Direct call just expects label to some abolute address that is assumed
> to fit in 32 bits (see constant_call_address_operand_p).
>
> call and jmp insn expect word_mode operands, so please change
> ix86_expand_call and call patterns in the same way as jump
> instructions above.
>
>> Since x86-64 hardware always zero-extends upper 32bits of 64bit
>> registers when loading its lower 32bits, it is safe and easier to just
>> to output 64bit registers for %A than zero-extend it by hand for all
>> jump/call patterns.
>
> No, the instruction expects word_mode operands, so we have to extend
> values to expected mode. I don't think that patching at insn output
> time is acceptable.

You are right. I found a testcase to show problem:

struct foo
{
  void (*f) (void);
  int i;
};

void
__attribute__ ((noinline))
bar (struct foo x)
{
  x.f ();
}

"x" is passed in RDI and the uppper 32bits of RDI is "int i".

> BTW: I propose to split the patch into smaller pieces, dealing with
> various independent parts separately. Handling jump/call insn is
> definitely one of them, the other is stringops handling, another
> prologue/epilogue expansion.
>

I will do that.

Thanks.