[PATCH] ARM testcase improvements

[Renato Golin]

LGTM.


On 10 October 2013 23:33, Matthias Braun <mbraun at apple.com> wrote:

>
> On Oct 7, 2013, at 3:42 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>
> Hi Matthias,
>
> 0001:
> LGTM.
>
> 0002:
> Unless I am missing the semantic of CHECK-DAG, this seems wrong:
> +;CHECK-DAG: str r0, [sp, #8]
> +;CHECK-DAG: add r0, sp, #8
> How could a str-add sequence with a write-after-read dependency be
> semantically equivalent to a add-str with a read-after-write dependency?
>
> Yes that was too much -DAG adding on my part, I revised the patch. This
> place looks like this now:
> +;CHECK-DAG:    str     [[R0:r0]], [sp, #8]
> +;CHECK-DAG:    add     [[R0]], sp, #8
> +;CHECK-DAG:    str     r2, [sp, #12]
> so the 3rd instruction can still move anywhere in between but the 2nd
> should have a dependency on the first one now.
>
>
> At several places, you’ve replaced:
> r0, r1, etc.
> by
> {{r[0-9]+}}
> I didn’t check all the places, but in some of them, r0, r1, etc. directly
> come from the ABI constraints, so we want to explicitly check for them.
> Please make sure, you are not obfuscated ABI constraints as it may
> emphasize we are doing something wrong!
> One example is
> -;CHECK: vst1.16 {d16[2]}, [r0:16]
> +;CHECK: vst1.16 {{{d[0-9]+}}[2]}, [{{r[0-9]+}}:16]
> r0 comes from the ABI.
>
> yes right all the places with r0 accessed paramters, I reverted this. All
> the _update tests that mentioned r1 did not access an ABI parameter and I
> left them as is.
>
>
> Or here
> -;CHECK: vst2.16 {d16[1], d17[1]}, [r0:32]
> +;CHECK: vst2.16 {d16[1], d17[1]}, [{{r[0-9]+}}:32]
> This one is even worse, because r0 comes from the ABI but not d16 and d17,
> thus they should have been “regexped” but not r0.
>
> I wasn’t entirely sure if d16+d17 has to be in order or not, so I left
> them intact, I did not replace r0 with a regex in the updated version
> anymore.
>
>
> The last one should use CHECK and not CHECK-DAG:
> +; CHECK-DAG:  add.w r[[ADDR:[0-9]+]], r[[SOURCE]], {{r[0-9]+}}, lsl #2
> +; CHECK-DAG:  vld1.32 {[[DREG:d[0-9]+]][], [[DREG2:d[0-9]+]][]},
> [r[[ADDR]]:32]       <— ADDR, read-after-write dependencies.
>
> FileCheck is smart enough to compute dependencies between [[Variables]]
> and enforce a correct order, but for this case it didn’t change anything
> compared to just CHECK: so I reverted that part.
>
> Thanks for the review,
> Matthias
>
>
>
>
> 0003:
> LGTM.
>
> Thanks,
> -Quentin
>
> On Oct 7, 2013, at 2:08 PM, Matthias Braun <mbraun at apple.com> wrote:
>
> Attached are a few improvements to make some (mostly arm) testcases less
> dependent on a particular schedule/register allocation. Maybe you want to
> have these changes in the repository. Descriptions:
>
> * Tests: Use CHECK-LABEL where possible
> * Tests: Be less dependent on a specific schedule/regalloc
> * Tests: Do not unnecessarily depend on kill comments
>
> Greetings
> Matthias
>
> <0001-Tests-Use-CHECK-LABEL-where-possible.patch>
> <0002-Tests-Be-less-dependent-on-a-specific-schedule-regal.patch>
> <0003-Tests-Do-not-unnecessarily-depend-on-kill-comments.patch>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131011/b985b760/attachment.html>