[AArch64] Fix vcvt_high_f64_f32 and vcvt_figh_f32_f64 intrinsics.

Hi,

This patch clears up some remaining confusion in the vector lane orderings
for the two intrinsics mentioned in the title.

Bootstrapped on aarch64-none-linux-gnu and regression tested for
aarch64_be-none-elf with no issues.

OK?

Thanks,
James

---
2015-09-09  James Greenhalgh  <james.greenhalgh@arm.com>

	* config/aarch64/aarch64-simd.md (vec_unpacks_lo_v4sf): Rewrite
	as an expand.
	(vec_unpacks_hi_v4sf):  Likewise.
	(aarch64_float_extend_lo_v2df): Rename to...
	(aarch64_fcvtl_v4sf): This.
	(aarch64_fcvtl2_v4sf): New.
	(aarch64_float_truncate_hi_v4sf): Rewrite as an expand.
	(aarch64_float_truncate_hi_v4sf_le): New.
	(aarch64_float_truncate_hi_v4sf_be): Likewise.

On 9 September 2015 at 10:31, James Greenhalgh <james.greenhalgh@arm.com> wrote:
>
> Hi,
>
> This patch clears up some remaining confusion in the vector lane orderings
> for the two intrinsics mentioned in the title.
>
> Bootstrapped on aarch64-none-linux-gnu and regression tested for
> aarch64_be-none-elf with no issues.
>

Does this actually fix an existing testcase?


> OK?
>
> Thanks,
> James
>
> ---
> 2015-09-09  James Greenhalgh  <james.greenhalgh@arm.com>
>
>         * config/aarch64/aarch64-simd.md (vec_unpacks_lo_v4sf): Rewrite
>         as an expand.
>         (vec_unpacks_hi_v4sf):  Likewise.
>         (aarch64_float_extend_lo_v2df): Rename to...
>         (aarch64_fcvtl_v4sf): This.
>         (aarch64_fcvtl2_v4sf): New.
>         (aarch64_float_truncate_hi_v4sf): Rewrite as an expand.
>         (aarch64_float_truncate_hi_v4sf_le): New.
>         (aarch64_float_truncate_hi_v4sf_be): Likewise.
>

Hmmm, hang on. I'm not quite sure what the actual issue/bug is here, but is this 
the same issue as my patch 12 "with BE RTL fix"? 
(https://gcc.gnu.org/ml/gcc-patches/2015-08/msg01482.html, explanation last at 
https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02365.html) I pushed this as 
r227551 last night and since this reparameterizes the patterns I don't think 
your patch will apply to current HEAD.

If my patch is wrong...well, that may be, I haven't understood the issue yet. 
But it sounds like the first thing we need is a decent testcase? (Or is the 
confusion just in the RTL representation, so a testcase would require getting 
constant-folding to happen in RTL, which I tried but failed to make that happen 
myself?)

--Alan

On 09/09/15 09:31, James Greenhalgh wrote:
>
> Hi,
>
> This patch clears up some remaining confusion in the vector lane orderings
> for the two intrinsics mentioned in the title.
>
> Bootstrapped on aarch64-none-linux-gnu and regression tested for
> aarch64_be-none-elf with no issues.
>
> OK?
>
> Thanks,
> James
>
> ---
> 2015-09-09  James Greenhalgh  <james.greenhalgh@arm.com>
>
> 	* config/aarch64/aarch64-simd.md (vec_unpacks_lo_v4sf): Rewrite
> 	as an expand.
> 	(vec_unpacks_hi_v4sf):  Likewise.
> 	(aarch64_float_extend_lo_v2df): Rename to...
> 	(aarch64_fcvtl_v4sf): This.
> 	(aarch64_fcvtl2_v4sf): New.
> 	(aarch64_float_truncate_hi_v4sf): Rewrite as an expand.
> 	(aarch64_float_truncate_hi_v4sf_le): New.
> 	(aarch64_float_truncate_hi_v4sf_be): Likewise.
>

On 09/09/15 11:31, Alan Lawrence wrote:
> Hmmm, hang on. I'm not quite sure what the actual issue/bug is here, but is this
> the same issue as my patch 12 "with BE RTL fix"?
> (https://gcc.gnu.org/ml/gcc-patches/2015-08/msg01482.html, explanation last at
> https://gcc.gnu.org/ml/gcc-patches/2015-07/msg02365.html) I pushed this as
> r227551 last night and since this reparameterizes the patterns I don't think
> your patch will apply to current HEAD.
>
> If my patch is wrong...well, that may be, I haven't understood the issue yet.

In particular, we should expect the vec_unpacks standard pattern to have 
different behaviour (from a tree POV), as this is what I find searching for 
VEC_UNPACK in tree-vect-stmts.c:


bool
supportable_widening_operation
{
...
    switch (code)
      {
...
      CASE_CONVERT:
        c1 = VEC_UNPACK_LO_EXPR;
        c2 = VEC_UNPACK_HI_EXPR;
        break;

      case FLOAT_EXPR:
        c1 = VEC_UNPACK_FLOAT_LO_EXPR;
        c2 = VEC_UNPACK_FLOAT_HI_EXPR;
        break;

      case FIX_TRUNC_EXPR:
        /* ??? Not yet implemented due to missing VEC_UNPACK_FIX_TRUNC_HI_EXPR/
           VEC_UNPACK_FIX_TRUNC_LO_EXPR tree codes and optabs used for
           computing the operation.  */
        return false;

      default:
        gcc_unreachable ();
      }

    if (BYTES_BIG_ENDIAN && c1 != VEC_WIDEN_MULT_EVEN_EXPR)
      std::swap (c1, c2);



Yes, IIUC this goes against the principle of tree being the same regardless of 
underlying endianness.

--Alan