llvm dev - Mar 2019 - Writing unit tests - how to test re-orderable blocks...

We have a test that looks like this…

define void @array16_store() {
; CHECK-LABEL: array16_store:

; CHECK: ldi [[REG1:r[0-9]+]], 204
; CHECK: ldi [[REG2:r[0-9]+]], 170
; CHECK: sts int.array+3, [[REG2]]
; CHECK: sts int.array+2, [[REG1]]

; CHECK: ldi [[REG1:r[0-9]+]], 187
; CHECK: ldi [[REG2:r[0-9]+]], 170
; CHECK: sts int.array+1, [[REG2]]
; CHECK: sts int.array, [[REG1]]


; CHECK: ldi [[REG1:r[0-9]+]], 221
; CHECK: ldi [[REG2:r[0-9]+]], 170
; CHECK: sts int.array+5, [[REG2]]
; CHECK: sts int.array+4, [[REG1]]
  store i16 43707, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
@int.array, i32 0, i64 0)
  store i16 43724, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
@int.array, i32 0, i64 1)
  store i16 43741, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
@int.array, i32 0, i64 2)
  ret void
}



The issue I have is this test is unnecessarily fragile because the compiler can
(and recently has) decided to reorder the blocks in a perfectly valid way.  For
example:

; %bb.0:
	ldi	r24, 221
	ldi	r25, 170
	sts	int.array+5, r25
	sts	int.array+4, r24
	ldi	r24, 204
	ldi	r25, 170
	sts	int.array+3, r25
	sts	int.array+2, r24
	ldi	r24, 187
	ldi	r25, 170
	sts	int.array+1, r25
	sts	int.array, r24
	ret


The three blocks should be independent, it doesn’t matter whether it does the
store to array+5 and array+4 first or last. What matters is that each block
stays together, these instructions must stay together for example:
	ldi	r24, 221
	ldi	r25, 170
	sts	int.array+5, r25
	sts	int.array+4, r24

 But that could come after the other two blocks or before them.



How can I write FileCheck conditions to test this? If they were single
re-orderable lines, I could use CHECK-DAG for each, but I need the four lines of
each block to stay together.

Can anyone offer advice?

Carl

We have a test that looks like this…

define void @array16_store() {
; CHECK-LABEL: array16_store:

; CHECK: ldi [[REG1:r[0-9]+]], 204
; CHECK: ldi [[REG2:r[0-9]+]], 170
; CHECK: sts int.array+3, [[REG2]]
; CHECK: sts int.array+2, [[REG1]]

; CHECK: ldi [[REG1:r[0-9]+]], 187
; CHECK: ldi [[REG2:r[0-9]+]], 170
; CHECK: sts int.array+1, [[REG2]]
; CHECK: sts int.array, [[REG1]]


; CHECK: ldi [[REG1:r[0-9]+]], 221
; CHECK: ldi [[REG2:r[0-9]+]], 170
; CHECK: sts int.array+5, [[REG2]]
; CHECK: sts int.array+4, [[REG1]]
 store i16 43707, i16* getelementptr inbounds ([3 x i16], [3 x i16]* @int.array,
i32 0, i64 0)
 store i16 43724, i16* getelementptr inbounds ([3 x i16], [3 x i16]* @int.array,
i32 0, i64 1)
 store i16 43741, i16* getelementptr inbounds ([3 x i16], [3 x i16]* @int.array,
i32 0, i64 2)
 ret void
}



The issue I have is this test is unnecessarily fragile because the compiler can
(and recently has) decided to reorder the blocks in a perfectly valid way.  For
example:

; %bb.0:
	ldi	r24, 221
	ldi	r25, 170
	sts	int.array+5, r25
	sts	int.array+4, r24
	ldi	r24, 204
	ldi	r25, 170
	sts	int.array+3, r25
	sts	int.array+2, r24
	ldi	r24, 187
	ldi	r25, 170
	sts	int.array+1, r25
	sts	int.array, r24
	ret


The three blocks should be independent, it doesn’t matter whether it does the
store to array+5 and array+4 first or last. What matters is that each block
stays together, these instructions must stay together for example:
	ldi	r24, 221
	ldi	r25, 170
	sts	int.array+5, r25
	sts	int.array+4, r24

But that could come after the other two blocks or before them.



How can I write FileCheck conditions to test this? If they were single
re-orderable lines, I could use CHECK-DAG for each, but I need the four lines of
each block to stay together.

Can anyone offer advice?

Carl

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190307/3f5355c7/attachment.html>

On Fri, Mar 8, 2019 at 1:45 AM Carl Peto via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> We have a test that looks like this…
>
> define void @array16_store() {
> ; CHECK-LABEL: array16_store:
>
> ; CHECK: ldi [[REG1:r[0-9]+]], 204
> ; CHECK: ldi [[REG2:r[0-9]+]], 170
> ; CHECK: sts int.array+3, [[REG2]]
> ; CHECK: sts int.array+2, [[REG1]]
>
> ; CHECK: ldi [[REG1:r[0-9]+]], 187
> ; CHECK: ldi [[REG2:r[0-9]+]], 170
> ; CHECK: sts int.array+1, [[REG2]]
> ; CHECK: sts int.array, [[REG1]]
>
>
> ; CHECK: ldi [[REG1:r[0-9]+]], 221
> ; CHECK: ldi [[REG2:r[0-9]+]], 170
> ; CHECK: sts int.array+5, [[REG2]]
> ; CHECK: sts int.array+4, [[REG1]]
All these CHECK except the first one should be CHECK-NEXT.
>   store i16 43707, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
@int.array, i32 0, i64 0)
>   store i16 43724, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
@int.array, i32 0, i64 1)
>   store i16 43741, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
@int.array, i32 0, i64 2)
>   ret void
> }
>
>
>
> The issue I have is this test is unnecessarily fragile because the compiler
can (and recently has) decided to reorder the blocks in a perfectly valid way. 
For example:
> ; %bb.0:
>         ldi     r24, 221
>         ldi     r25, 170
>         sts     int.array+5, r25
>         sts     int.array+4, r24
>         ldi     r24, 204
>         ldi     r25, 170
>         sts     int.array+3, r25
>         sts     int.array+2, r24
>         ldi     r24, 187
>         ldi     r25, 170
>         sts     int.array+1, r25
>         sts     int.array, r24
>         ret
>
>
> The three blocks should be independent, it doesn’t matter whether it does
the store to array+5 and array+4 first or last. What matters is that each block
stays together, these instructions must stay together for example:
>         ldi     r24, 221
>         ldi     r25, 170
>         sts     int.array+5, r25
>         sts     int.array+4, r24
>
>  But that could come after the other two blocks or before them.
>
>
>
> How can I write FileCheck conditions to test this? If they were single
re-orderable lines, I could use CHECK-DAG for each, but I need the four lines of
each block to stay together.
Is the order deterministic, or does it reorder blocks in the same test
differently each time?
If it is *NOT* deterministic, then it is most certainly a bug that
should be reported.
If it is deterministic, (answer without backend-specific knowledge) it
might be best not to worry about this,
and just use the llvm/utils/update_llc_test_checks.py to simplify
updating of the tests.
> Can anyone offer advice?
>
> Carl
Roman.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

I’m not sure if it’s truly deterministic. It always gives the same results (so
far) on my machine but I’m not sure that’s enough.

My guess is it’s probably going to be deterministic on one machine but might
well not be deterministic across environments. Like it might give varying
results if cross compiled on different hosts, macOS vs intel Linux vs arm vs
s390. (Obviously AVR is always a cross compiler as it’s embedded!)

The patch that “broke” the unit test on my timeline is e2baccfd07. This was a
commit that improved parallelism on the compile. In this case the blocks are
independent. It’s valid to set them in any order.

If there’s no way to get FileCheck to recognise this, the test should probably
be weakened or it will keep breaking I guess?

Carl 

> On 8 Mar 2019, at 07:13, Roman Lebedev <lebedev.ri at gmail.com>
wrote:
> 
> On Fri, Mar 8, 2019 at 1:45 AM Carl Peto via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> 
>> We have a test that looks like this…
>> 
>> define void @array16_store() {
>> ; CHECK-LABEL: array16_store:
>> 
>> ; CHECK: ldi [[REG1:r[0-9]+]], 204
>> ; CHECK: ldi [[REG2:r[0-9]+]], 170
>> ; CHECK: sts int.array+3, [[REG2]]
>> ; CHECK: sts int.array+2, [[REG1]]
>> 
>> ; CHECK: ldi [[REG1:r[0-9]+]], 187
>> ; CHECK: ldi [[REG2:r[0-9]+]], 170
>> ; CHECK: sts int.array+1, [[REG2]]
>> ; CHECK: sts int.array, [[REG1]]
>> 
>> 
>> ; CHECK: ldi [[REG1:r[0-9]+]], 221
>> ; CHECK: ldi [[REG2:r[0-9]+]], 170
>> ; CHECK: sts int.array+5, [[REG2]]
>> ; CHECK: sts int.array+4, [[REG1]]
> 
> All these CHECK except the first one should be CHECK-NEXT.
> 
>>  store i16 43707, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
@int.array, i32 0, i64 0)
>>  store i16 43724, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
@int.array, i32 0, i64 1)
>>  store i16 43741, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
@int.array, i32 0, i64 2)
>>  ret void
>> }
>> 
>> 
>> 
>> The issue I have is this test is unnecessarily fragile because the
compiler can (and recently has) decided to reorder the blocks in a perfectly
valid way.  For example:
>> ; %bb.0:
>>        ldi     r24, 221
>>        ldi     r25, 170
>>        sts     int.array+5, r25
>>        sts     int.array+4, r24
>>        ldi     r24, 204
>>        ldi     r25, 170
>>        sts     int.array+3, r25
>>        sts     int.array+2, r24
>>        ldi     r24, 187
>>        ldi     r25, 170
>>        sts     int.array+1, r25
>>        sts     int.array, r24
>>        ret
>> 
>> 
>> The three blocks should be independent, it doesn’t matter whether it
does the store to array+5 and array+4 first or last. What matters is that each
block stays together, these instructions must stay together for example:
>>        ldi     r24, 221
>>        ldi     r25, 170
>>        sts     int.array+5, r25
>>        sts     int.array+4, r24
>> 
>> But that could come after the other two blocks or before them.
>> 
>> 
>> 
>> How can I write FileCheck conditions to test this? If they were single
re-orderable lines, I could use CHECK-DAG for each, but I need the four lines of
each block to stay together.
> Is the order deterministic, or does it reorder blocks in the same test
> differently each time?
> If it is *NOT* deterministic, then it is most certainly a bug that
> should be reported.
> If it is deterministic, (answer without backend-specific knowledge) it
> might be best not to worry about this,
> and just use the llvm/utils/update_llc_test_checks.py to simplify
> updating of the tests.
> 
>> Can anyone offer advice?
>> 
>> Carl
> Roman.
> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

> -----Original Message-----
> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of
Carl
> Peto via llvm-dev
> Sent: Thursday, March 07, 2019 5:44 PM
> To: llvm-dev
> Subject: [llvm-dev] Writing unit tests - how to test re-orderable
> blocks...
> 
> We have a test that looks like this…
> 
> define void @array16_store() {
> ; CHECK-LABEL: array16_store:
> 
> ; CHECK: ldi [[REG1:r[0-9]+]], 204
> ; CHECK: ldi [[REG2:r[0-9]+]], 170
> ; CHECK: sts int.array+3, [[REG2]]
> ; CHECK: sts int.array+2, [[REG1]]
> 
> ; CHECK: ldi [[REG1:r[0-9]+]], 187
> ; CHECK: ldi [[REG2:r[0-9]+]], 170
> ; CHECK: sts int.array+1, [[REG2]]
> ; CHECK: sts int.array, [[REG1]]
> 
> 
> ; CHECK: ldi [[REG1:r[0-9]+]], 221
> ; CHECK: ldi [[REG2:r[0-9]+]], 170
> ; CHECK: sts int.array+5, [[REG2]]
> ; CHECK: sts int.array+4, [[REG1]]
>   store i16 43707, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
> @int.array, i32 0, i64 0)
>   store i16 43724, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
> @int.array, i32 0, i64 1)
>   store i16 43741, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
> @int.array, i32 0, i64 2)
>   ret void
> }
> 
> 
> 
> The issue I have is this test is unnecessarily fragile because the
> compiler can (and recently has) decided to reorder the blocks in a
> perfectly valid way.  For example:
> 
> ; %bb.0:
> 	ldi	r24, 221
> 	ldi	r25, 170
> 	sts	int.array+5, r25
> 	sts	int.array+4, r24
> 	ldi	r24, 204
> 	ldi	r25, 170
> 	sts	int.array+3, r25
> 	sts	int.array+2, r24
> 	ldi	r24, 187
> 	ldi	r25, 170
> 	sts	int.array+1, r25
> 	sts	int.array, r24
> 	ret
> 
> 
> The three blocks should be independent, it doesn’t matter whether it does
> the store to array+5 and array+4 first or last. What matters is that each
> block stays together, these instructions must stay together for example:
> 	ldi	r24, 221
> 	ldi	r25, 170
> 	sts	int.array+5, r25
> 	sts	int.array+4, r24
> 
>  But that could come after the other two blocks or before them.
> 
> 
> 
> How can I write FileCheck conditions to test this? If they were single re-
> orderable lines, I could use CHECK-DAG for each, but I need the four lines
> of each block to stay together.
I would capture the text output to a file, and run FileCheck multiple times
on the same file, once for each group of instructions. You would need to give 
each block of CHECK lines its own prefix, but that's a small thing.

But I would second the observation that even if different orders are valid,
the compiler's behavior should be deterministic, aside from patches that
change the ordering for a specific reason. This kind of breakage ought to
be unusual, and if it's not, that's likely a bug that should be fixed
instead of you working around it in your test.
--paulr
> 
> Can anyone offer advice?
> 
> Carl
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

I’ve corrected the test to match current output in my patch. I think from what
you guys say that’s perfectly good enough.

I’ll submit to the AVR team via the maintainer.

Thanks for your help guys. :)

Carl
> On 8 Mar 2019, at 19:49, via llvm-dev <llvm-dev at lists.llvm.org>
wrote:
> 
> 
> 
>> -----Original Message-----
>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org
<mailto:llvm-dev-bounces at lists.llvm.org>] On Behalf Of Carl
>> Peto via llvm-dev
>> Sent: Thursday, March 07, 2019 5:44 PM
>> To: llvm-dev
>> Subject: [llvm-dev] Writing unit tests - how to test re-orderable
>> blocks...
>> 
>> We have a test that looks like this…
>> 
>> define void @array16_store() {
>> ; CHECK-LABEL: array16_store:
>> 
>> ; CHECK: ldi [[REG1:r[0-9]+]], 204
>> ; CHECK: ldi [[REG2:r[0-9]+]], 170
>> ; CHECK: sts int.array+3, [[REG2]]
>> ; CHECK: sts int.array+2, [[REG1]]
>> 
>> ; CHECK: ldi [[REG1:r[0-9]+]], 187
>> ; CHECK: ldi [[REG2:r[0-9]+]], 170
>> ; CHECK: sts int.array+1, [[REG2]]
>> ; CHECK: sts int.array, [[REG1]]
>> 
>> 
>> ; CHECK: ldi [[REG1:r[0-9]+]], 221
>> ; CHECK: ldi [[REG2:r[0-9]+]], 170
>> ; CHECK: sts int.array+5, [[REG2]]
>> ; CHECK: sts int.array+4, [[REG1]]
>>  store i16 43707, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
>> @int.array, i32 0, i64 0)
>>  store i16 43724, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
>> @int.array, i32 0, i64 1)
>>  store i16 43741, i16* getelementptr inbounds ([3 x i16], [3 x i16]*
>> @int.array, i32 0, i64 2)
>>  ret void
>> }
>> 
>> 
>> 
>> The issue I have is this test is unnecessarily fragile because the
>> compiler can (and recently has) decided to reorder the blocks in a
>> perfectly valid way.  For example:
>> 
>> ; %bb.0:
>> 	ldi	r24, 221
>> 	ldi	r25, 170
>> 	sts	int.array+5, r25
>> 	sts	int.array+4, r24
>> 	ldi	r24, 204
>> 	ldi	r25, 170
>> 	sts	int.array+3, r25
>> 	sts	int.array+2, r24
>> 	ldi	r24, 187
>> 	ldi	r25, 170
>> 	sts	int.array+1, r25
>> 	sts	int.array, r24
>> 	ret
>> 
>> 
>> The three blocks should be independent, it doesn’t matter whether it
does
>> the store to array+5 and array+4 first or last. What matters is that
each
>> block stays together, these instructions must stay together for
example:
>> 	ldi	r24, 221
>> 	ldi	r25, 170
>> 	sts	int.array+5, r25
>> 	sts	int.array+4, r24
>> 
>> But that could come after the other two blocks or before them.
>> 
>> 
>> 
>> How can I write FileCheck conditions to test this? If they were single
re-
>> orderable lines, I could use CHECK-DAG for each, but I need the four
lines
>> of each block to stay together.
> 
> I would capture the text output to a file, and run FileCheck multiple times
> on the same file, once for each group of instructions. You would need to
give
> each block of CHECK lines its own prefix, but that's a small thing.
> 
> But I would second the observation that even if different orders are valid,
> the compiler's behavior should be deterministic, aside from patches
that
> change the ordering for a specific reason. This kind of breakage ought to
> be unusual, and if it's not, that's likely a bug that should be
fixed
> instead of you working around it in your test.
> --paulr
> 
>> 
>> Can anyone offer advice?
>> 
>> Carl
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20190309/b71c0a0b/attachment.html>