On Thu, Nov 29, 2018 at 12:43 AM Jeff Law <l...@redhat.com> wrote:
>
> On 11/28/18 2:47 PM, Vladimir Makarov wrote:
> > The patch I committed today recently for
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88207
> >
> > creates a new regression for pr34256.c. 2 moves is expected but gcc
> > with the patch generates 3 moves. I think now RA generates the right code.
> >
> > We have the following code before RA
> >
> > (insn 7 6 13 2 (set (reg:V2SI 88)
> > (plus:V2SI (reg:V2SI 89 [ x ])
> > (mem/c:V2SI (symbol_ref:DI ("y") [flags 0x2] <var_decl
> > 0x7faad08b5b40 y>) [1 y+0 S8 A64])))"mmintrin.h":306:18 1115 {*mmx_addv2si3}
> >
> > (expr_list:REG_DEAD (reg:V2SI 89 [ x ])
> > (nil)))
> > (insn 13 7 14 2 (set (reg/i:DI 0 ax)
> > (subreg:DI (reg:V2SI 88) 0)) "pr34256.c":11:1 66 {*movdi_internal}
> > (expr_list:REG_DEAD (reg:V2SI 88)
> >
> > The test is expected to assign mmx reg to pseudo 88 but gcc with the
> > patch assigns memory to it. The cost of mmx to general reg move is 13,
> > while overall cost of mmx to mem and mem to general moves is 10. So IRA
> > now chooses memory for pseudo 88 according to the minimal cost.
> >
> > Now, if we want still assign mmx reg to the pseudo 88, we should change
> > the costs in machine-dependent x86 code. But I think it might create
> > other unexpected code generation. As mmx is basically not used nowadays
> > the test is not important, I just propose the following patch.
> >
> > Is it ok for the trunk?
The generated code is correct, in i386.c, secondary_memory_needed, we have:
/* ??? This is a lie. We do have moves between mmx/general, and for
mmx/sse2. But by saying we need secondary memory we discourage the
register allocator from using the mmx registers unless needed. */
if (MMX_CLASS_P (class1) != MMX_CLASS_P (class2))
return true;
But I noticed that for -mtune=generic we still generate direct move.
The cost of interunit MMX -> GR moves should always be higher than the
cost of indirect move (due to secondary_memory_needed), c.f. the
comment in ix86_register_move_cost:
/* In case we require secondary memory, compute cost of the store followed
by load. In order to avoid bad register allocation choices, we need
for this to be *at least* as high as the symmetric MEMORY_MOVE_COST. */
IMO, the issue with -mtune=generic warrants some additional analysis.
I'll look at target cost calculations.
Thanks,
Uros.
