Re: [PATCH, IRA] Fix ALLOCNO_MODE in the case of paradoxical subreg.

[Vladimir Makarov <vmakarov at redhat dot com>]

On 09/24/2013 07:57 PM, Wei Mi wrote:
> Hi,
>
> This patch is to address the problem described here:
> http://gcc.gnu.org/ml/gcc/2013-09/msg00187.html
>
> The patch changes ALLOCNO_MODE of a pseudo reg to be outermode if the
> pseudo reg is used in a paradoxical subreg, so IRA will not mistakenly
> assign an operand with a bigger mode to a smaller hardreg which
> couldn't find a pair register.
>
> No test is added because I cannot create a small testcase to reproduce
> the problem on trunk, the difficulty of which was described in the
> above post.
>
> bootstrap and regression pass. ok for trunk?
>
> Thanks,
> Wei Mi.
>
> 2013-09-24  Wei Mi  <wmi@google.com>
>
>         * ira-build.c (create_insn_allocnos): Fix ALLOCNO_MODE
>         in the case of paradoxical subreg.
>
> Index: ira-build.c
> ===================================================================
> --- ira-build.c (revision 201963)
> +++ ira-build.c (working copy)
> @@ -1688,6 +1688,30 @@ create_insn_allocnos (rtx x, bool output
>         }
>        return;
>      }
> +  else if (code == SUBREG)
> +    {
> +      int regno;
> +      rtx subreg_reg = SUBREG_REG (x);
> +      enum machine_mode outermode, innermode;
> +
> +      create_insn_allocnos (subreg_reg, output_p);
> +      /* For paradoxical subreg, set allocno's mode to be
> +        the outermode.  */
> +      outermode = GET_MODE (x);
> +      innermode = GET_MODE (subreg_reg);
> +      if (REG_P (subreg_reg)
> +         && (GET_MODE_SIZE (outermode) > GET_MODE_SIZE (innermode)))
> +       {
> +         regno = REGNO (subreg_reg);
> +         if (regno >= FIRST_PSEUDO_REGISTER)
> +           {
> +             ira_allocno_t a = ira_curr_regno_allocno_map[regno];
> +             ira_assert (a != NULL);
> +             ALLOCNO_MODE (a) = outermode;
> +           }
> +       }
> +      return;
> +    }
>    else if (code == SET)
>      {
>        create_insn_allocnos (SET_DEST (x), true);
Wei Mi, sorry for misleading you.  I thought more about the problem.  It
is not a trivial one.

The patch is ok for the code correctness but it might hurt code
performance. For example, we have code

... (reg:DI) ...
...
... (subreg:TI (reg:DI))
...
...(reg:DI)

We need two hard regs only for the second place by transforming

p = (reg:DI)

...(subreg:TI p)

With this patch we requires two hard regs for the all live range of the
original pseudo (which can be quite long).  It might considerably worsen
code performance.

So the problem could be fixed in LRA which can make this transformation
or even in IRA (that would be even better as we put pseudo P into the
global picture vs. probably spilling some pseudos for P in LRA).

I need some time to think what is better (e.g. I don't know how to
implement it in IRA without big compilation slow down).  In any case,
the solution for the problem will be not that easy as in the patch.

Sorry again.