llvm dev - Sep 2012 - [LLVMdev] teaching FileCheck to handle variations in order

Just adding to the clamor for FileChecks ability to pattern match out-of-order
(match for mere presence); Just in the last 2 weeks, I have come across at least
a couple instances when I was unable to add small unit tests to the testsuite
because of this deficiency in FileCheck. Also, I agree with Krzysztof about the
lack of any real recurring overhead.

 

Can this feature please be added to FileCheck ?

 

Pranav

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation

 

From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On
Behalf Of Sergei Larin
Sent: Monday, September 10, 2012 1:16 PM
To: 'Matthew Curtis'; 'Chandler Carruth'
Cc: llvmdev at cs.uiuc.edu
Subject: Re: [LLVMdev] teaching FileCheck to handle variations in order

 

My 2c:

 

I think that ability to pattern match “out-of-order” in tests is critical. I
currently have to disable instruction scheduling on several Hexagon tests in
order to preserve “expected” order of patterns, which has nothing to do with
actual correctness of the test… So if nothing else, scheduler is left untested
for those cases. Needless to say, this feature must maintain status quo for all
existing tests, and only introduce “new” functionality for future, or selected
existing tests.

 

Sergei Larin

 

---

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation

 

From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On
Behalf Of Matthew Curtis
Sent: Monday, September 10, 2012 9:28 AM
To: Chandler Carruth
Cc: llvmdev at cs.uiuc.edu
Subject: Re: [LLVMdev] teaching FileCheck to handle variations in order

 

On 9/7/2012 4:08 PM, Chandler Carruth wrote:

On Fri, Sep 7, 2012 at 8:20 AM, Matthew Curtis <mcurtis at codeaurora.org>
wrote:

Hello all,

For the hexagon target, we have a couple of tests that are failing due to
variations in the order of checked text. In these cases the ordering is not
directly relevant to the functionality being tested.

For example:

; CHECK: memw(##a)
; CHECK: memw(##b)

%0 = load i32* @a, align 4
%1 = load i32* @b, align 4

requires that the compiler emit the memory operations for 'a' and
'b' in that order, even though the intent of the test might simply be to
ensure that each 'load' results in a memory read.

I'd like to teach FileCheck to allow tests to be more tolerant of these
incidental variations.

 

So, I'm not a huge fan of this, but for a strange reason.

 

Fundamentally, I agree with you that these ordering dependencies are unfortunate
in tests. However, I would make a few observations:

 

1) The order of instructions, unlike some things such as register
allocation's selection of registers, should ideally not vary much. Currently
it does more that we would like due to the inability to unit test single pieces
of the backend, but I don't think we should make FileCheck better to cope
with that, I think we should fix the underlying problem.

 

2) It is usually fairly obvious when two such checks don't actually have an
ordering constraint that is important, and where it isn't, a comment makes
the intent clear.

 

3) By checking the ordering, we gain at least some early detection of
non-determinism in the code generator. I definitely caught several bugs in the
block placement pass while I was working on it due to this very
"feature" of the test suite.

 

 

Given these points, the value tradeoff here of making FileCheck *significantly*
more complicated (and the tests significantly more complex as well) versus the
time savings updating test cases when the ordering changes...

 


To be honest, I'm fairly new to LLVM and Clang so I don't have a good
feel for how often this will come up. I've encountered two tests over the
past month and a half that have regressed due to ordering changes. In one case
the order varies based on target, so I would have to duplicate the test and
enable/disable one or the other based on target. Not great, but if it's just
one test probably acceptable.

As far as complexity, I didn't feel like the complexity added to FileCheck
was that significant, though relative to how often the feature would (or should)
get used perhaps it is. I was more concerned about adding 2-3 new directives to
the user interface. In the end, that acceptable given that users would only see
the additional complexity if they needed it.

If no one else feels like they would benefit much from this, I'll probably
just wait and see how much of a maintenance burden this really is. If it
continues to be an issue I can bring it back up to the community.



But maybe others who have spent more time hammering against these problems in
the backend have a different sense of the magnitude of the problem?

 


The attached patch implements one possible solution. It introduces a position
stack and a couple of directives:

*	'CHECK-PUSH:' pushes the current match position onto the stack.
*	'CHECK-POP:' pops the top value off of the stack and uses it to set
the current match position.

The above test can now be re-written as:

; CHECK-PUSH:
; CHECK: memw(##a)
; CHECK-POP:
; CHECK: memw(##b)

%0 = load i32* @a, align 4
%1 = load i32* @b, align 4

which handles either ordering of memory reads for 'a' and 'b'.

Thoughts?

Thanks,
Matthew Curtis

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation


_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

 





-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120913/dc00d44d/attachment.html>

On Thu, Sep 13, 2012 at 1:14 PM, Pranav Bhandarkar
<pranavb at codeaurora.org>wrote:
> Just adding to the clamor for FileChecks ability to pattern match
> out-of-order (match for mere presence); Just in the last 2 weeks, I have
> come across at least a couple instances when I was unable to add small unit
> tests to the testsuite because of this deficiency in FileCheck. Also, I
> agree with Krzysztof about the lack of any real recurring overhead.
>
Can you explain why you were unable to add a small unit test to the
testsuite?

All we have here are claims that missing this feature is blocking something
from happening, but neither myself nor several other developers have ever
hit a situation where they were unable to test because of missing this.

> ****
>
> ** **
>
> Can this feature please be added to FileCheck ?****
>
> ** **
>
> Pranav****
>
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by The Linux Foundation****
>
> ** **
>
> *From:* llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at
cs.uiuc.edu] *On
> Behalf Of *Sergei Larin
> *Sent:* Monday, September 10, 2012 1:16 PM
> *To:* 'Matthew Curtis'; 'Chandler Carruth'
>
> *Cc:* llvmdev at cs.uiuc.edu
> *Subject:* Re: [LLVMdev] teaching FileCheck to handle variations in order*
> ***
>
> ** **
>
> My 2c:****
>
> ** **
>
> I think that ability to pattern match “out-of-order” in tests is critical.
> I currently have to disable instruction scheduling on several Hexagon tests
> in order to preserve “expected” order of patterns, which has nothing to do
> with actual correctness of the test… So if nothing else, scheduler is left
> untested for those cases. Needless to say, this feature must maintain
> status quo for all existing tests, and only introduce “new” functionality
> for future, or selected existing tests.****
>
> ** **
>
> Sergei Larin****
>
> ** **
>
> ---****
>
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by The Linux Foundation****
>
> ** **
>
> *From:* llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at
cs.uiuc.edu] *On
> Behalf Of *Matthew Curtis
> *Sent:* Monday, September 10, 2012 9:28 AM
> *To:* Chandler Carruth
> *Cc:* llvmdev at cs.uiuc.edu
> *Subject:* Re: [LLVMdev] teaching FileCheck to handle variations in order*
> ***
>
> ** **
>
> On 9/7/2012 4:08 PM, Chandler Carruth wrote:****
>
> On Fri, Sep 7, 2012 at 8:20 AM, Matthew Curtis <mcurtis at
codeaurora.org>
> wrote:****
>
> Hello all,
>
> For the hexagon target, we have a couple of tests that are failing due to
> variations in the order of checked text. In these cases the ordering is not
> directly relevant to the functionality being tested.
>
> For example:****
>
> ; CHECK: memw(##a)
> ; CHECK: memw(##b)
>
> %0 = load i32* @a, align 4
> %1 = load i32* @b, align 4****
>
> requires that the compiler emit the memory operations for 'a' and
'b' in
> that order, even though the intent of the test might simply be to ensure
> that each 'load' results in a memory read.
>
> I'd like to teach FileCheck to allow tests to be more tolerant of these
> incidental variations.****
>
> ** **
>
> So, I'm not a huge fan of this, but for a strange reason.****
>
> ** **
>
> Fundamentally, I agree with you that these ordering dependencies are
> unfortunate in tests. However, I would make a few observations:****
>
> ** **
>
> 1) The order of instructions, unlike some things such as register
> allocation's selection of registers, should ideally not vary much.
> Currently it does more that we would like due to the inability to unit test
> single pieces of the backend, but I don't think we should make
FileCheck
> better to cope with that, I think we should fix the underlying problem.***
> *
>
> ** **
>
> 2) It is usually fairly obvious when two such checks don't actually
have
> an ordering constraint that is important, and where it isn't, a comment
> makes the intent clear.****
>
> ** **
>
> 3) By checking the ordering, we gain at least some early detection of
> non-determinism in the code generator. I definitely caught several bugs in
> the block placement pass while I was working on it due to this very
> "feature" of the test suite.****
>
> ** **
>
> ** **
>
> Given these points, the value tradeoff here of making FileCheck
> *significantly* more complicated (and the tests significantly more complex
> as well) versus the time savings updating test cases when the ordering
> changes...****
>
> ** **
>
>
> To be honest, I'm fairly new to LLVM and Clang so I don't have a
good feel
> for how often this will come up. I've encountered two tests over the
past
> month and a half that have regressed due to ordering changes. In one case
> the order varies based on target, so I would have to duplicate the test and
> enable/disable one or the other based on target. Not great, but if it's
> just one test probably acceptable.
>
> As far as complexity, I didn't feel like the complexity added to
FileCheck
> was that significant, though relative to how often the feature would (or
> should) get used perhaps it is. I was more concerned about adding 2-3 new
> directives to the user interface. In the end, that acceptable given that
> users would only see the additional complexity if they needed it.
>
> If no one else feels like they would benefit much from this, I'll
probably
> just wait and see how much of a maintenance burden this really is. If it
> continues to be an issue I can bring it back up to the community.
>
> ****
>
> But maybe others who have spent more time hammering against these problems
> in the backend have a different sense of the magnitude of the problem?****
>
> ** **
>
>
> The attached patch implements one possible solution. It introduces a
> position stack and a couple of directives:****
>
>    - 'CHECK-PUSH:' pushes the current match position onto the
stack.****
>    - 'CHECK-POP:' pops the top value off of the stack and uses it
to set
>    the current match position.****
>
> The above test can now be re-written as:****
>
> ; CHECK-PUSH:
> ; CHECK: memw(##a)
> ; CHECK-POP:
> ; CHECK: memw(##b)
>
> %0 = load i32* @a, align 4
> %1 = load i32* @b, align 4****
>
> which handles either ordering of memory reads for 'a' and
'b'.
>
> Thoughts?
>
> Thanks,
> Matthew Curtis****
>
> -- ****
>
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation****
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev****
>
> ** **
>
>
>
> ****
>
> -- ****
>
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation****
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120913/8f7a0063/attachment.html>

> Can you explain why you were unable to add a small unit test to the
testsuite?
This is the case I have.

******
       r3 = memb(r1+#1)
       r4 = memb(r1+ #0)
      ..
      ..
      {
        p0 = r3  /* Should almost never emit this. */
        p1 = r4  /* Should almost never emit this. */
     ..
    ..
       p0 = or(p1, p0)

*****
The CHECK tags I have are
; CHECK: [[IntReg0:r[0-9]+]] = memb     //IntReg0 is r3
; CHECK: [[IntReg1:r[0-9]+]] = memb    //IntReg1 is r4
; CHECK-NOT: {{r[0-9]+}} = zxtb([[IntReg1]])   //Ensure no zxtb instructions
exist between the loads and their uses.
; CHECK-NOT: {{r[0-9]+}} = zxtb([[IntReg0]])
; CHECK: [[PredReg0:p[0-3]+]] = [[IntReg0]]  // <--- [1]
; CHECK: [[PredReg1:p[0-3]+]] = [[IntReg1]] // <---  [2]
; CHECK: or([[PredReg1]], [[PredReg0]])

When I was trying to write this testcase I had thought that if the order of [1]
and [2] changed because of some change in the scheduler, the optimization
(removal of zxtb) would still hold, but the changed order would cause a test
failure.

I do understand your point about using such things as checks for non-determinism
in the code generator, but I would argue that this should be best left to the
judgement of the test case writer.

Pranav
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation