Patchwork [x86] Fix gcc.c-torture/compile/20080806-1.c failures

login
register
mail settings
Submitter H.J. Lu
Date Nov. 5, 2012, 11:57 p.m.
Message ID <CAMe9rOpaoB0fd59wGeRqYr6QhaiE1eOroosUqgU=cUT1Zogqmg@mail.gmail.com>
Download mbox | patch
Permalink /patch/197363/
State New
Headers show

Comments

H.J. Lu - Nov. 5, 2012, 11:57 p.m.
On Mon, Nov 5, 2012 at 3:34 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Nov 5, 2012 at 10:56 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> Uros Bizjak <ubizjak@gmail.com> writes:
>>> On Mon, Nov 5, 2012 at 9:30 AM, Richard Sandiford
>>> <rdsandiford@googlemail.com> wrote:
>>>> gcc/
>>>>         PR target/55204
>>>>         * config/i386/i386.c (ix86_address_subreg_operand): Remove stack
>>>>         pointer check.
>>>>         (print_reg): Use true_regnum rather than REGNO.
>>>>         (ix86_print_operand_address): Remove SUBREG handling.
>>>
>>> The patch is OK for mainline and 4.7, if it passes H.J.'s tests with
>>> -maddress-mode={short,long} on x32.
>>>
>>>> +  unsigned int regno = true_regnum (x);
>>>
>>> I'd rather see the declaration at the beginning of the function.
>>
>> OK, thanks, applied to both with that change.
>>
>> Richard
>
>
> On 4.7 branch, this failed to build x32 run-time library:
>
> /tmp/ccID3vr6.s: Assembler messages:
> /tmp/ccID3vr6.s:2788: Error: bad register name `%rr15'
>
> There is an extra 'r' in register name.
>
> --
> H.J.

I think this is a real bug:

    case 8:
    case 4:
    case 12:
      if (! ANY_FP_REG_P (x))
        putc (code == 8 && TARGET_64BIT ? 'r' : 'e', file);
      /* FALLTHRU */
    case 16:
    case 2:
    normal:
      reg = hi_reg_name[regno];
      break;

hi_reg_name has

#define HI_REGISTER_NAMES                                               \
{"ax","dx","cx","bx","si","di","bp","sp",                               \
 "st","st(1)","st(2)","st(3)","st(4)","st(5)","st(6)","st(7)",          \
 "argp", "flags", "fpsr", "fpcr", "frame",                              \
 "xmm0","xmm1","xmm2","xmm3","xmm4","xmm5","xmm6","xmm7",               \
 "mm0", "mm1", "mm2", "mm3", "mm4", "mm5", "mm6", "mm7",                \
 "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",                  \
 "xmm8", "xmm9", "xmm10", "xmm11", "xmm12", "xmm13", "xmm14", "xmm15"}

If we ever use r8 to r15, we print wrong register name.  We should
catch it with

  /* Irritatingly, AMD extended registers use different naming convention
     from the normal registers: "r%d[bwd]"  */
  if (REX_INT_REG_P (x))

But it doesn't work with subreg change.   I am testing this


I will check it into trunk and 4.7 branch if test passes.

Thanks.
Richard Sandiford - Nov. 6, 2012, 7:38 a.m.
"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Mon, Nov 5, 2012 at 3:34 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Mon, Nov 5, 2012 at 10:56 AM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> Uros Bizjak <ubizjak@gmail.com> writes:
>>>> On Mon, Nov 5, 2012 at 9:30 AM, Richard Sandiford
>>>> <rdsandiford@googlemail.com> wrote:
>>>>> gcc/
>>>>>         PR target/55204
>>>>>         * config/i386/i386.c (ix86_address_subreg_operand): Remove stack
>>>>>         pointer check.
>>>>>         (print_reg): Use true_regnum rather than REGNO.
>>>>>         (ix86_print_operand_address): Remove SUBREG handling.
>>>>
>>>> The patch is OK for mainline and 4.7, if it passes H.J.'s tests with
>>>> -maddress-mode={short,long} on x32.
>>>>
>>>>> +  unsigned int regno = true_regnum (x);
>>>>
>>>> I'd rather see the declaration at the beginning of the function.
>>>
>>> OK, thanks, applied to both with that change.
>>>
>>> Richard
>>
>>
>> On 4.7 branch, this failed to build x32 run-time library:
>>
>> /tmp/ccID3vr6.s: Assembler messages:
>> /tmp/ccID3vr6.s:2788: Error: bad register name `%rr15'
>>
>> There is an extra 'r' in register name.

Argh!  Interesting that you only hit this on 4.7.  Perhaps that's
an indication that 4.8 is better at not leaving unsimplified subregs
in the insn stream.

> If we ever use r8 to r15, we print wrong register name.  We should
> catch it with
>
>   /* Irritatingly, AMD extended registers use different naming convention
>      from the normal registers: "r%d[bwd]"  */
>   if (REX_INT_REG_P (x))
>
> But it doesn't work with subreg change.   I am testing this
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 37498ef..b42870f 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -13729,7 +13729,7 @@ print_reg (rtx x, int code, FILE *file)
>
>    /* Irritatingly, AMD extended registers use different naming convention
>       from the normal registers: "r%d[bwd]"  */
> -  if (REX_INT_REG_P (x))
> +  if (REX_INT_REGNO_P (regno))
>      {
>        gcc_assert (TARGET_64BIT);
>        putc ('r', file);
>
> I will check it into trunk and 4.7 branch if test passes.

Thanks HJ.

Richard

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 37498ef..b42870f 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -13729,7 +13729,7 @@  print_reg (rtx x, int code, FILE *file)

   /* Irritatingly, AMD extended registers use different naming convention
      from the normal registers: "r%d[bwd]"  */
-  if (REX_INT_REG_P (x))
+  if (REX_INT_REGNO_P (regno))
     {
       gcc_assert (TARGET_64BIT);
       putc ('r', file);