Re: [PATCH 2/2] rs6000: Refine integer comparison handlings in rs6000_emit_vector_compare

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Wed, Nov 16, 2022 at 02:51:04PM +0800, Kewen.Lin wrote:
> The current handlings in rs6000_emit_vector_compare is a bit
> complicated to me, especially after we emit vector float
> comparison insn with the given code directly.  This patch is
> to refine the handlings for vector integer comparison operators,
> it becomes not recursive, and we don't need the helper function
> rs6000_emit_vector_compare_inner any more.

That sounds nice.

>    /* In vector.md, we support all kinds of vector float point
>       comparison operators in a comparison rtl pattern, we can
>       just emit the comparison rtx insn directly here.  Besides,
>       we should have a centralized place to handle the possibility
> -     of raising invalid exception.  */
> -  if (GET_MODE_CLASS (dmode) == MODE_VECTOR_FLOAT)
> +     of raising invalid exception.  Also emit directly for vector
> +     integer comparison operators EQ/GT/GTU.  */
> +  if (GET_MODE_CLASS (dmode) == MODE_VECTOR_FLOAT
> +      || rcode == EQ
> +      || rcode == GT
> +      || rcode == GTU)

The comment still says it handles FP only.  That would be best to keep
imo: add a separate block of code to handle the integer stuff you want
to add.  You get the same or better generated code, the compiler is
smart enough.  Code is for the user to read, and C is not a portable
assembler language.

This whole series needs to be factored better, it does way too many
things, and only marginally related things, at every step.  Or I don't
see it anyway :-)


Segher