[PATCH] D47106: [FileCheck] Make CHECK-DAG non-overlapping

[Joel E. Denny via Phabricator via llvm-commits]

jdenny added inline comments.


================
Comment at: test/FileCheck/check-dag-overlap.txt:12
+
+; IdentPat:     {{^}}__IdentPat
+; IdentPat-DAG: {{^}}add [[REG1:r[0-9]+]], {{r[0-9]+}}, {{r[0-9]+}}
----------------
probinson wrote:
> Ordinarily these delimiters would be using -LABEL; is there some reason not to?
I was mimicking check-dag-xfails.txt.  I was thinking -LABEL wouldn't work well because each RUN would see only one -LABEL, but I suppose all RUNs could share a common CHECK prefix for -LABEL.  I'll work on that.  Thanks.


================
Comment at: test/FileCheck/check-dag-overlap.txt:34
+; IdentPatNot-NOT: {{^}}xor
+; IdentPatNot-DAG: {{^}}mul r5, r10, r11
+; IdentPatNot:     {{^}}__IdentPatNot
----------------
probinson wrote:
> Normally I'd say a single DAG after a NOT is more misleading than helpful; unless you changed some behavior here?
I was just trying to prove that this case is handled sanely when overlapping matches must be skipped.  I don't mean to imply anything about whether it's a good way to write test cases.


================
Comment at: test/FileCheck/check-dag-overlap.txt:165
+; RUN:               -check-prefix=Torture
+; RUN: FileCheck -input-file %s %s -check-prefix=Torture
+
----------------
probinson wrote:
> Please move the Torture  part into its own test file.  I'm finding it hard to keep track well enough to review.
Will do.


================
Comment at: test/FileCheck/check-dag-overlap.txt:467
+; DagNotDag:     {{^}}__DagNotDag
\ No newline at end of file

----------------
probinson wrote:
> Reading this in the overlapping mode:  The X set of DAGs will all match on line 424, the Y set will all match on line 425, therefore the NOTs for rule 1 will correctly not match anything.  The Z set of DAGs will all match on line 428, therefore the NOTs for rule 2 will correctly not match anything.  Then the NOTs for rule 3 will match, making the run fail.
> 
> Is that what you intended?  Naively I would have thought you'd try to break each of the rules, but that's not what the test does.
> Reading this in the overlapping mode: The X set of DAGs will all match on line 424,

Yes.

> the Y set will all match on line 425

No.  y also matches on line 424 because overlapping is permitted, so we get a reordering complaint.  You can confirm this with the -vv option.

> Is that what you intended? Naively I would have thought you'd try to break each of the rules, but that's not what the test does.

It breaks the first rule because of -allow-deprecated-dag-overlap.

With non-overlapping matches, it breaks the second rule if I remove the following line from CheckString::CheckDag:

      Matches.push_back(Match{MatchPos, MatchPos + MatchLen})

(I had that bug at one point.)

While rule 3 is exercised, I don't know how to break it without breaking rule 1 or 2 because it's a consequence of rules 1 and 2.


https://reviews.llvm.org/D47106