[PATCH] Enable partial update depency breaking for A15

[Silviu Baranga]

Hi Renato,

Thanks for the review. My comments are inlined.

> From: Renato Golin [renato.golin at linaro.org]
> Sent: 26 March 2013 21:09
> To: Silviu Baranga
> Cc: Commit Messages and Patches for LLVM
> Subject: Re: [PATCH] Enable partial update depency breaking for A15
>
> Hi Silviu,
>
> +; CHECK: t1:
> +define <2 x float> @t1(float* %A, <2 x float> %B) {
> +; CHECK-NOT: vmov.f64
> +  %tmp2 = load float* %A, align 4
> +  %tmp3 = insertelement <2 x float> %B, float %tmp2, i32 1
> +  ret <2 x float> %tmp3
>
> What are you expecting here?

I should have added some comments to these tests (will do so tomorrow).
The generated code uses a vld1.32 instruction to write the lane 1 of a D register
containing the value of <2 x float> %B. Since the D register is defined, it would
be incorrect to fully write it (with a vmov.f64) before the vld1.32 instruction.
Test checks that a vmov.f64 was not generated.

>
> +; CHECK: t2:
> +define void @t2(<4 x i8> *%in, <4 x i8> *%out, i32 %n) {
> +entry:
> +  br label %loop
> +loop:
> +; CHECK: vmov.f64
>
> I'm lost, why f64?
It could be anything that writes the D register, it doesn't matter. The
existing code uses a FCONSTD (vmov.f64) to write the D register before the
vld1.32 instruction (or other instructions that partially update D registers)
in order to break the existing dependencies on the D register.

> -    if ((DisableA15SDOptimization || !getARMSubtarget().isCortexA15()) &&
> -      getARMSubtarget().hasNEON())
> +    if (getARMSubtarget().hasNEON())
>
> Now it always run? Any strong reason for dropping the A15 / opt constraints?

We need this pass to run since it performs the dependency breaking for partial updates.
Before this change, ExeDepsFix wasn't doing anything for A15 (unless the A15 SD optimization
pass was disabled). Now it always does something so it should always run.

>
>    // A9-like cores are particularly picky about mixing the two and want these
>    // converted.
> -  if (Subtarget.isLikeA9() && !isPredicated(MI) &&
> +  if (Subtarget.isCortexA9() && !isPredicated(MI) &&
>
> Would be good to update the comment, too.
Will also update the comment for the next iteration of the patch.

>
> cheers,
> --renato

Cheers,
Silviu


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.