[PATCH v2] x86: Add -munaligned-vector-move to assembler

[H.J. Lu]

On Fri, Oct 22, 2021 at 1:06 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 21.10.2021 19:50, H.J. Lu wrote:
> > Unaligned load/store instructions on aligned memory or register are as
> > fast as aligned load/store instructions on modern Intel processors.  Add
> > a command-line option, -munaligned-vector-move, to x86 assembler to
> > encode encode aligned vector load/store instructions as unaligned
> > vector load/store instructions.
>
> As said I'm still lacking the spelling out here of some form of
> motivation for the change. The resulting code, afaict, isn't going
> to be better than what we have now, yet there's the price of extra
> new code that you introduce.

We'd like to have such an option just in case.

> > @@ -4080,6 +4083,26 @@ optimize_encoding (void)
> >  {
> >    unsigned int j;
> >
> > +  /* Encode aligned vector move as unaligned vector move if asked.  */
> > +  if (unaligned_vector_move)
> > +    switch (i.tm.base_opcode)
> > +      {
> > +      case 0x28:
> > +     /* movaps/movapd/vmovaps/vmovapd.  */
> > +     if (i.tm.opcode_modifier.opcodespace == 1
> > +         && i.tm.opcode_modifier.opcodeprefix <= 1)
>
> I don't think the prefix needs checking here? F3 and F2 encodings
> don't exist, so maybe at most gas_assert() this?

I changed it to

    if (i.tm.opcode_modifier.opcodespace == SPACE_0F
          && i.tm.opcode_modifier.opcodeprefix <= PREFIX_0X66)
        i.tm.base_opcode = 0x10;

in case that F3 and F2 are used in the future.

> > +       i.tm.base_opcode = 0x10;
> > +     break;
> > +      case 0x6f:
> > +     /* movdqa/vmovdqa/vmovdqa64/vmovdqa32 */
> > +     if (i.tm.opcode_modifier.opcodespace == 1
> > +         && i.tm.opcode_modifier.opcodeprefix == 1)
> > +       i.tm.opcode_modifier.opcodeprefix = 2;
>
> Please can you avoid using literal numbers here? This not only makes it
> needlessly harder to potentially change the SPACE_* and PREFIX_* values
> (despite me hoping / assuming that we would never have to do so), but
> also makes the code not sufficiently self-documenting.

I changed it to

     if (i.tm.opcode_modifier.opcodespace == SPACE_0F
          && i.tm.opcode_modifier.opcodeprefix == PREFIX_0X66)
        i.tm.opcode_modifier.opcodeprefix = PREFIX_0XF3;

> > @@ -5053,7 +5076,8 @@ md_assemble (char *line)
> >        i.disp_operands = 0;
> >      }
> >
> > -  if (optimize && !i.no_optimize && i.tm.opcode_modifier.optimize)
> > +  if (unaligned_vector_move
> > +      || (optimize && !i.no_optimize && i.tm.opcode_modifier.optimize))
> >      optimize_encoding ();
>
> Isn't this fragile? You now depend on optimize_encoding() to do nothing
> optimization-wise if unaligned_vector_move is set. I was rather hoping
> you would introduce a separate helper.
>

I added encode_with_unaligned_vector_move and renamed the option
to -muse-unaligned-vector-move.

Thanks.

-- 
H.J.