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

Submitted by H.J. Lu on Nov. 5, 2012, 11:57 p.m.

Details

Message ID CAMe9rOpaoB0fd59wGeRqYr6QhaiE1eOroosUqgU=cUT1Zogqmg@mail.gmail.com
State New
Headers show

Commit Message

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.

Comments

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 hide | download patch | download mbox

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);