[PATCH] ARM testcase improvements

Matthias Braun mbraun at apple.com
Thu Oct 10 15:33:11 PDT 2013


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
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131010/d11b5fa7/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Tests-Be-less-dependent-on-a-specific-schedule-regal.patch
Type: application/octet-stream
Size: 13579 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131010/d11b5fa7/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131010/d11b5fa7/attachment-0001.html>


More information about the llvm-commits mailing list