Hi Carl,

On Wed, Apr 28, 2021 at 10:39:14AM -0700, Carl Love wrote:
> The agreement for the sign extension builtin was to just make it Endian
> aware rather then go with a more complex definition.  The prior patch
> has been updated with this new functionality.
> 
> This patch adds support for the 128-bit extension instruction and
> corresponding builtin support for the various sign extensions.
> 
> This was originally part of the Add 128-bit Integer operations patch
> series.  The patch logically goes with the earlier 5 patch series.

But it has nothing to do with patch 5/5, so it confused me that you
posted it as a reply to that.

> +(define_expand "vsignextend_v2di_v1ti"
> +  [(set (match_operand:V1TI 0 "vsx_register_operand" "=v")
> +     (unspec:V1TI [(match_operand:V2DI 1 "vsx_register_operand" "v")]
> +                  UNSPEC_VSX_SIGN_EXTEND))]
> +  "TARGET_POWER10"
> +{
> +  if (BYTES_BIG_ENDIAN)
> +    {
> +       rtx tmp = gen_reg_rtx (V2DImode);
> +
> +       emit_insn (gen_altivec_vrevev2di2(tmp, operands[1]));
> +       emit_insn (gen_vsx_sign_extend_v2di_v1ti(operands[0], tmp));
> +     }
> +  else
> +    emit_insn (gen_vsx_sign_extend_v2di_v1ti(operands[0], operands[1]));
> +  DONE;
> +})

The indentation is broken -- everything should go by two columns.

In cases like this where the pattern in the expand agrees with a
define_insn you have, you can write just

{
  if (BYTES_BIG_ENDIAN)
    {
      ...
      DONE;
    }
}

This is the normal way of writing it.  When a define_expand does not
call DONE or FAIL, the pattern is inserted in the insn stream.


Okay for trunk with the indents fixed, and maybe the expand thing.  Also
okay for GCC 11 if it survived testing on all targets and OSes.  That
goes for all patches in this series btw.  Thanks!


Segher

Reply via email to