Re: [PATCH] ARM/NEON: vld1q_dup_s64 builtin

[Ramana Radhakrishnan <ramana dot radhakrishnan at linaro dot org>]

>
>
>> You also don't mention how this patch was tested.
>
> I used the testsuite I developed some time ago to test all the Neon
> builtins, which I posted last year on the qemu mailing-list. With the
> current GCCs, this bug is the only remaining one I could detect.
>

Fair enough.


>
>> ?Alternatively it might be worth splitting the
>> vld1q_*64 case into a 64 bit load into a (subreg:DI (V2DI reg) ?0 )
>> followed by a subreg to subreg move which should end up having the
>> same effect . That splitting would allow for better instruction
>> scheduling.
>
> Are you aware of examples of similar cases I could use as a model?

I would change the iterator from VQX to VQ in the pattern above (you
can also simplify the setting of neon_type in that case as well as
change that to be a vec_duplicate as below and get rid of any
lingering definitions of UNSPEC_VLD1_DUP if they exist), define a
separate pattern that expressed this as a define_insn_and_split as
below.

 (define_insn_and_split "neon_vld1_dupv2di"
   [(set (match_operand:V2DI 0 "s_register_operand" "=w")
     (vec_duplicate:V2DI (match_operand:DI 1 "neon_struct_operand" "Um")))]
   "TARGET_NEON"
   "#"
   "&& reload_completed"
   [(const_int 0)]
   {
    rtx tmprtx = gen_lowpart (DImode, operands[0]);
    emit_insn (gen_neon_vld1_dupdi (tmprtx, operands[1]));
    emit_move_insn (gen_highpart (DImode, operands[0]), tmprtx );
    DONE;
    }
(set_attr "length" "8")
(set_attr "neon_type" "<fromearlierpattern">)
)

Do you want to try this and see what you get ?

>
>
>> ?In addition it would be nice to have a testcase in
>> gcc.target/arm .
>
> Well. Prior to sending my patch I did look at that directory, but I supposed
> that such a test ought to belong to the neon/ subdir where the tests are
> described as autogenerated. Any doc on how to do that?

 I'd rather have an extra regression test in gcc.target/arm that was a
run time test. for e.g. take a look at gcc.target/arm/neon-vadds64.c .

Ramana

>
> Thanks,
>
> Christophe.
>