[PATCH] D56789: FileCheck: Allow CHECK-{SAME, NEXT, EMPTY} after initial CHECK-DAG

Fri Jan 25 14:40:31 PST 2019

jdenny added a comment.

In D56789#1371821 <https://reviews.llvm.org/D56789#1371821>, @thopre wrote:

> Fair enough, any clobber can happen between the two, forgot about that. Note that I can see several existing tests in the testsuite that have CHECK-NEXT right after CHECK-DAG which are probably not as strict in terms of testing as their author were expecting, e.g.
>
> test/Analysis/LazyValueAnalysis/lvi-after-jumpthreading.ll
>  test/Bitcode/thinlto-function-summary-originalnames.ll
>  test/CodeGen/AArch64/arm64-ccmp.ll
>  test/CodeGen/AArch64/arm64-indexed-vector-ldst.ll (though it's a single DAG between a CHECK and CHECK-NEXT so it's a special case)
>  test/CodeGen/AArch64/f16-instructions.ll
>
> and many more.

Not surprising.  I remember now that I've put a CHECK-SAME after a CHECK-DAG because I wanted to look for items that could occur in any order on a single line.  I don't remember now if I did that as a best approximation of what I wanted or if I mistakenly thought it already achieved what I wanted.

>> A realistic use case would probably involve something like a `CHECK-DAG-NEXT` directive that doesn't permit intervening lines anywhere in the block.  Or should the above example actually indicate exactly that?
>> 
>> Do you have a use case from a real test to inform this discussion?
> 
> Agreed, something like CHECK-DAG-NEXT would be better suited. The requirement of CHECK-DAG with CHECK-NEXT was mentioned to me by a coworker and I noticed the weird inconsistency regarding whether CHECK-DAG is the first check directives or not and thought I'd fix it. The behaviour we need is indeed reordering without any line not checked for in between using CHECK-DAG is not the right solution. So I guess the correct approach would be to error out on CHECK-DAG with CHECK-NEXT, CHECK-SAME or CHECK-EMPTY, probably with some flag to enable the old behavior while tests are updated.

Possibly so.  Given that many of those existing cases were probably intending the stricter behavior, let's first create a better solution for the stricter behavior.

> Regarding CHECK-DAG-NEXT, I wonder how it would interact with CHECK-DAG?

I think that any kind of CHECK-DAG* following a different kind of CHECK-DAG* would start a new block such that there's no reordering among blocks.  Otherwise, the semantics become very unclear to me.

It seems intuitive that the first match in a CHECK-DAG-NEXT block would have to appear on the line after whatever match came before the block.  However, if you don't want that behavior, then I'm not sure how you would express that.

> Perhaps we should introduce some sort of only-tested region (e.g. CHECK-ONLY-START and CHECK-ONLY-END with better name) where CHECK-DAG inside would still be able to be reordered but only what is tested by a CHECK-DAG is allowed. It could also allow CHECK directive inside which would behave as CHECK-NEXT. How does that sound?

Or would that CHECK inside behave as a CHECK-SAME?  In general, how would whitespace be handled?  Error or ignored?

What you propose sounds very similar to something I suggested at one point: CHECK-NOT-PUSH  and CHECK-NOT-POP.  The idea is you would push a pattern into `-implicit-check-not` so the pattern would be rejected among matches between the push and the pop.  This would be useful in any context where you want `-implicit-check-not` but don't want it for the entire input.

To achieve something like CHECK-DAG-SAME, the pattern would specify a single newline, and you'd put CHECK-DAG directives in between the push and pop.  To achieve something like CHECK-DAG-NEXT, you'd do the same but each CHECK-DAG's pattern would be specified to gobble up the next newline.   Then again, those two cases might be easier to write and diagnostics might be clearer if we created special-purpose directives, but they could probably be built on the same infrastructure.

If all that makes sense to you, there are some details we could discuss about how CHECK-NOT-PUSH and CHECK-NOT-POP would interact with neighboring CHECK-NOT directives and neighboring matches.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56789/new/

https://reviews.llvm.org/D56789